| Message ID | 20201106123040.28451-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] pagevec: Allow pagevecs to be different sizes | expand |
On Fri, Nov 06, 2020 at 12:30:37PM +0000, Matthew Wilcox (Oracle) wrote: > Declaring a pagevec continues to create a pagevec which is the same size, > but functions which manipulate pagevecs no longer rely on this. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/pagevec.h | 20 ++++++++++++++++---- > mm/swap.c | 8 ++++++++ > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h > index 875a3f0d9dd2..ee5d3c4da8da 100644 > --- a/include/linux/pagevec.h > +++ b/include/linux/pagevec.h > @@ -18,9 +18,15 @@ struct page; > struct address_space; > > 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. > + struct page *pages[PAGEVEC_SIZE]; > + }; > + void *__p[PAGEVEC_SIZE + 1]; What's up with this union? > + }; > }; > > void __pagevec_release(struct pagevec *pvec); > @@ -41,6 +47,7 @@ static inline unsigned pagevec_lookup_tag(struct pagevec *pvec, > > static inline void pagevec_init(struct pagevec *pvec) > { > + pvec->sz = PAGEVEC_SIZE; > pvec->nr = 0; > pvec->percpu_pvec_drained = false; > } > @@ -50,6 +57,11 @@ static inline void pagevec_reinit(struct pagevec *pvec) > pvec->nr = 0; > } > > +static inline unsigned pagevec_size(struct pagevec *pvec) > +{ > + return pvec->sz; > +} > + > static inline unsigned pagevec_count(struct pagevec *pvec) > { > return pvec->nr; > @@ -57,7 +69,7 @@ static inline unsigned pagevec_count(struct pagevec *pvec) > > static inline unsigned pagevec_space(struct pagevec *pvec) > { > - return PAGEVEC_SIZE - pvec->nr; > + return pvec->sz - pvec->nr; > } > > /* > diff --git a/mm/swap.c b/mm/swap.c > index 2ee3522a7170..d093fb30f038 100644 > --- a/mm/swap.c > +++ b/mm/swap.c > @@ -52,6 +52,7 @@ struct lru_rotate { > }; > static DEFINE_PER_CPU(struct lru_rotate, lru_rotate) = { > .lock = INIT_LOCAL_LOCK(lock), > + .pvec.sz = PAGEVEC_SIZE, > }; > > /* > @@ -70,6 +71,13 @@ struct lru_pvecs { > }; > static DEFINE_PER_CPU(struct lru_pvecs, lru_pvecs) = { > .lock = INIT_LOCAL_LOCK(lock), > + .lru_add.sz = PAGEVEC_SIZE, > + .lru_deactivate_file.sz = PAGEVEC_SIZE, > + .lru_deactivate.sz = PAGEVEC_SIZE, > + .lru_lazyfree.sz = PAGEVEC_SIZE, > +#ifdef CONFIG_SMP > + .activate_page.sz = PAGEVEC_SIZE, > +#endif > }; > > /* > -- > 2.28.0 >
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.
diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h index 875a3f0d9dd2..ee5d3c4da8da 100644 --- a/include/linux/pagevec.h +++ b/include/linux/pagevec.h @@ -18,9 +18,15 @@ struct page; struct address_space; 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; + struct page *pages[PAGEVEC_SIZE]; + }; + void *__p[PAGEVEC_SIZE + 1]; + }; }; void __pagevec_release(struct pagevec *pvec); @@ -41,6 +47,7 @@ static inline unsigned pagevec_lookup_tag(struct pagevec *pvec, static inline void pagevec_init(struct pagevec *pvec) { + pvec->sz = PAGEVEC_SIZE; pvec->nr = 0; pvec->percpu_pvec_drained = false; } @@ -50,6 +57,11 @@ static inline void pagevec_reinit(struct pagevec *pvec) pvec->nr = 0; } +static inline unsigned pagevec_size(struct pagevec *pvec) +{ + return pvec->sz; +} + static inline unsigned pagevec_count(struct pagevec *pvec) { return pvec->nr; @@ -57,7 +69,7 @@ static inline unsigned pagevec_count(struct pagevec *pvec) static inline unsigned pagevec_space(struct pagevec *pvec) { - return PAGEVEC_SIZE - pvec->nr; + return pvec->sz - pvec->nr; } /* diff --git a/mm/swap.c b/mm/swap.c index 2ee3522a7170..d093fb30f038 100644 --- a/mm/swap.c +++ b/mm/swap.c @@ -52,6 +52,7 @@ struct lru_rotate { }; static DEFINE_PER_CPU(struct lru_rotate, lru_rotate) = { .lock = INIT_LOCAL_LOCK(lock), + .pvec.sz = PAGEVEC_SIZE, }; /* @@ -70,6 +71,13 @@ struct lru_pvecs { }; static DEFINE_PER_CPU(struct lru_pvecs, lru_pvecs) = { .lock = INIT_LOCAL_LOCK(lock), + .lru_add.sz = PAGEVEC_SIZE, + .lru_deactivate_file.sz = PAGEVEC_SIZE, + .lru_deactivate.sz = PAGEVEC_SIZE, + .lru_lazyfree.sz = PAGEVEC_SIZE, +#ifdef CONFIG_SMP + .activate_page.sz = PAGEVEC_SIZE, +#endif }; /*
Declaring a pagevec continues to create a pagevec which is the same size, but functions which manipulate pagevecs no longer rely on this. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/pagevec.h | 20 ++++++++++++++++---- mm/swap.c | 8 ++++++++ 2 files changed, 24 insertions(+), 4 deletions(-)