[PATCH v6 1/5] PID namespace translation support

[Dmitry V. Levin]

On Fri, Aug 07, 2020 at 09:34:22PM +0200, Ákos Uzonyi wrote:
> On Fri, 7 Aug 2020 at 20:23, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > On Thu, Aug 06, 2020 at 09:02:07PM +0200, Ákos Uzonyi wrote:
[...]
> > > + * large, since there can be large holes between PIDs, and it would be just a
> > > + * waste of memory having large nodes with lot of NULL pointers in them.
> > > + */
> > > +static struct trie *
> > > +create_trie_4(uint8_t key_size, uint8_t item_size_lg, uint64_t empty_value)
> > > +{
> > > +     struct trie *t = trie_create(key_size, item_size_lg, 4, 4, empty_value);
> > > +     if (!t)
> > > +             error_msg_and_die("creating trie failed");
> > > +
> > > +     return t;
> > > +}
> >
> > I believe empty_value is always 0, so why bother with the argument?
> 
> I'm working on a performance improvement: translating pids by unix
> sockets. In that code I use a trie where empty_value is -1 (keys are
> file descriptors), so I would like to keep that argument.

OK

[...]
> > > +struct trie *
> > > +trie_create(uint8_t key_size, uint8_t item_size_lg, uint8_t node_key_bits,
> > > +            uint8_t data_block_key_bits, uint64_t empty_value)
> > > +{
> > > +     if (item_size_lg > 6)
> > > +             return NULL;
> > > +     if (key_size > 64)
> > > +             return NULL;
> > > +     if (node_key_bits < 1)
> > > +             return NULL;
> > > +     if (data_block_key_bits < 1 || data_block_key_bits > key_size)
> > > +             return NULL;
> >
> > Should there be a check that node_key_bits isn't too big, for consistency?
> 
> Actually it's not a problem if node_key_bits is big. If node_key_bits
> is larger than key_size, then all the key bits (except the bits for
> the data block) are part of the "<remainder>" node level.

Unless we care about integer overflows due to node_key_bits being too
large.  Well, this definitely can wait as we are not going to have such
large node_key_bits any time soon.

[...]
> > > +     /**
> > > +      * Number of bits in key that makes a symbol for a node.
> > > +      * (equals to log2 of the child count of the node)
> > > +      */
> > > +     uint8_t node_key_bits;
> > > +
> > > +     /**
> > > +      * Number of bits in key that make a symbol for the data block (leaf).
> >
> > Typo?
> 
> Sorry I can't find the typo you are referring to.

It's grammar than. :)

The first comments says: Number of bits in key that makes.
The second comments says: Number of bits in key that make.

By the way, it should be "in the key" in both cases.


-- 
ldv