Ted Unangst wrote:
> Steven Chamberlain wrote:
> > This is a really, really long shot, but could the calculation here on
> > (unsigned int) pgsize underflow and return true when it shouldn't??
> > 
> > +    * Decide whether to put the page header off page to avoid
> > +    * wasting too large a part of the page. Off-page page headers
> > +    * go into an RB tree, so we can match a returned item with
> > +    * its header based on the page address.
> > +    */
> > +   if (pgsize - (size * items) > sizeof(struct pool_item_header)) {
> > +           flags |= PR_PHINPAGE;
> > +           off = pgsize - sizeof(struct pool_item_header);
> 
> That's interesting. I agree that test could perhaps be improved by writing it
>       if (pgsize > size * items + sizeof(struct pool_item_header))

Luckily it seems pgsize is implicitly always >= size, otherwise it would
have underflowed.

Nitpicking slightly - why wasn't the test greater-equal?  If there is
exactly enough space for a struct pool_item_header, why not use it?
(Unless the intent was to always leave slack space for cache coloring,
although I doubt it).

Next I'm curious about this, also in kern/subr_pool.c:

        unsigned int pgsize = PAGE_SIZE, items;
        ...
        if (palloc == NULL) {
                while (size > pgsize)
                        pgsize <<= 1;
        ...
        } else
                pgsize = palloc->pa_pagesz ? palloc->pa_pagesz : PAGE_SIZE;

I don't see code any more to set pa_pagesz?  So is it really safe to
assume pgsize = PAGE_SIZE?

In -CURRENT, this changed to allocate size*8 (which has been problematic
in the past), meaning pgsize is more likely to be larger than PAGE_SIZE.
If this function is re-entered, I think it would assume a different
pgsize and off (page header offset) as a result?

Thanks,
Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org

Reply via email to