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
