| Message ID | 20211208042256.1923824-6-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Folios for 5.17 | expand |
On Wed, Dec 08, 2021 at 04:22:13AM +0000, Matthew Wilcox (Oracle) wrote: > +static inline void folio_batch_release(struct folio_batch *fbatch) > +{ > + pagevec_release((struct pagevec *)fbatch); > +} > + > +static inline void folio_batch_remove_exceptionals(struct folio_batch *fbatch) > +{ > + pagevec_remove_exceptionals((struct pagevec *)fbatch); > +} I think these casts need documentation, both here and at the struct folio_batch and struct pagevec definitions. Alternatively I wonder if a union in stuct pagevec so that it can store folios or pages might be the better option.
On Wed, Dec 22, 2021 at 10:54:27PM -0800, Christoph Hellwig wrote: > On Wed, Dec 08, 2021 at 04:22:13AM +0000, Matthew Wilcox (Oracle) wrote: > > +static inline void folio_batch_release(struct folio_batch *fbatch) > > +{ > > + pagevec_release((struct pagevec *)fbatch); > > +} > > + > > +static inline void folio_batch_remove_exceptionals(struct folio_batch *fbatch) > > +{ > > + pagevec_remove_exceptionals((struct pagevec *)fbatch); > > +} > > I think these casts need documentation, both here and at the > struct folio_batch and struct pagevec definitions. > > Alternatively I wonder if a union in stuct pagevec so that it can store > folios or pages might be the better option. I tried that way first, but then the caller & callee need to agree whether they're storing folios or pages in the pagevec. And that's kind of why we have types. pagevec_remove_exceptionals() goes away by the end of this series. pagevec_release() will take longer to remove. What documentation do you want to see?
On Thu, Dec 23, 2021 at 02:18:40PM +0000, Matthew Wilcox wrote: > > I think these casts need documentation, both here and at the > > struct folio_batch and struct pagevec definitions. > > > > Alternatively I wonder if a union in stuct pagevec so that it can store > > folios or pages might be the better option. > > I tried that way first, but then the caller & callee need to agree > whether they're storing folios or pages in the pagevec. And that's kind > of why we have types. > > pagevec_remove_exceptionals() goes away by the end of this series. > pagevec_release() will take longer to remove. What documentation > do you want to see? Mostly comments at the pagevec and folio_batch definitions that they need to match because of these two functions, and then maybe a backreference from the casts to the definitions.
diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h index 7f3f19065a9f..4483e6ad7607 100644 --- a/include/linux/pagevec.h +++ b/include/linux/pagevec.h @@ -15,6 +15,7 @@ #define PAGEVEC_SIZE 15 struct page; +struct folio; struct address_space; struct pagevec { @@ -81,4 +82,66 @@ static inline void pagevec_release(struct pagevec *pvec) __pagevec_release(pvec); } +/** + * struct folio_batch - A collection of folios. + * + * The folio_batch is used to amortise the cost of retrieving and + * operating on a set of folios. The order of folios in the batch is + * not considered important. Some users of the folio_batch store + * "exceptional" entries in it which can be removed by calling + * folio_batch_remove_exceptionals(). + */ +struct folio_batch { + unsigned char nr; + unsigned char aux[3]; + struct folio *folios[PAGEVEC_SIZE]; +}; + +/** + * folio_batch_init() - Initialise a batch of folios + * @fbatch: The folio batch. + * + * A freshly initialised folio_batch contains zero folios. + */ +static inline void folio_batch_init(struct folio_batch *fbatch) +{ + fbatch->nr = 0; +} + +static inline unsigned int folio_batch_count(struct folio_batch *fbatch) +{ + return fbatch->nr; +} + +static inline unsigned int fbatch_space(struct folio_batch *fbatch) +{ + return PAGEVEC_SIZE - fbatch->nr; +} + +/** + * folio_batch_add() - Add a folio to a batch. + * @fbatch: The folio batch. + * @folio: The folio to add. + * + * The folio is added to the end of the batch. + * The batch must have previously been initialised using folio_batch_init(). + * + * Return: The number of slots still available. + */ +static inline unsigned folio_batch_add(struct folio_batch *fbatch, + struct folio *folio) +{ + fbatch->folios[fbatch->nr++] = folio; + return fbatch_space(fbatch); +} + +static inline void folio_batch_release(struct folio_batch *fbatch) +{ + pagevec_release((struct pagevec *)fbatch); +} + +static inline void folio_batch_remove_exceptionals(struct folio_batch *fbatch) +{ + pagevec_remove_exceptionals((struct pagevec *)fbatch); +} #endif /* _LINUX_PAGEVEC_H */
The folio_batch is the same as the pagevec, except that it is typed to contain folios and not pages. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/pagevec.h | 63 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+)