On Sat, Nov 07, 2020 at 12:08:13PM -0500, Kent Overstreet wrote:
> On Fri, Nov 06, 2020 at 12:30:37PM +0000, Matthew Wilcox (Oracle) wrote:
> > struct pagevec {
> > - unsigned char nr;
> > - bool percpu_pvec_drained;
> > - struct page *pages[PAGEVEC_SIZE];
> > + union {
> > + struct {
> > + unsigned char sz;
> > + unsigned char nr;
> > + bool percpu_pvec_drained;
> This should probably be removed, it's only used by the swap code and I don't
> think it belongs in the generic data structure. That would mean nr and size (and
> let's please use more standard naming...) can be u32, not u8s.
Nevertheless, that's a very important user of pagevecs! You and I have no
need for it in the fs code, but it doesn't hurt to leave it here.
I really don't think that increasing the size above 255 is worth anything.
See my earlir analysis of the effect of increasing the batch size.
> > + struct page *pages[PAGEVEC_SIZE];
> > + };
> > + void *__p[PAGEVEC_SIZE + 1];
>
> What's up with this union?
*blink*. That was supposed to be 'struct page *pages[];'
And the reason to do it that way is to avoid overly-clever instrumentation
like kmsan from saying "Hey, nr is bigger than 16, this is a buffer
overrun". Clearly I didn't build a kernel with the various sanitisers
enabled, but I'll do that now.
