| Message ID | 20220102215729.2943705-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Convert GUP to folios | expand |
On Sun, Jan 02, 2022 at 09:57:14PM +0000, Matthew Wilcox (Oracle) wrote: > -static inline bool hpage_pincount_available(struct page *page) > +static inline bool folio_pincount_available(struct folio *folio) > { > /* > - * Can the page->hpage_pinned_refcount field be used? That field is in > + * Can the folio->hpage_pinned_refcount field be used? That field is in > * the 3rd page of the compound page, so the smallest (2-page) compound > * pages cannot support it. > */ > + return folio_order(folio) > 1; > +} Maybe convert the comment into a kerneldoc one? Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Jan 04, 2022 at 12:01:45AM -0800, Christoph Hellwig wrote: > On Sun, Jan 02, 2022 at 09:57:14PM +0000, Matthew Wilcox (Oracle) wrote: > > -static inline bool hpage_pincount_available(struct page *page) > > +static inline bool folio_pincount_available(struct folio *folio) > > { > > /* > > - * Can the page->hpage_pinned_refcount field be used? That field is in > > + * Can the folio->hpage_pinned_refcount field be used? That field is in > > * the 3rd page of the compound page, so the smallest (2-page) compound > > * pages cannot support it. > > */ > > + return folio_order(folio) > 1; > > +} > > Maybe convert the comment into a kerneldoc one? I'm actually thinking about getting rid of it by moving the hpage_pinned_refcount into page[1]: diff --git a/include/linux/mm.h b/include/linux/mm.h index 4e763a590c9c..07fa8af564ed 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -950,7 +950,9 @@ static inline int head_compound_pincount(struct page *head) static inline void set_compound_order(struct page *page, unsigned int order) { page[1].compound_order = order; +#ifdef CONFIG_64BIT page[1].compound_nr = 1U << order; +#endif } /* Returns the number of pages in this potentially compound page. */ @@ -958,7 +960,11 @@ static inline unsigned long compound_nr(struct page *page) { if (!PageHead(page)) return 1; +#ifdef CONFIG_64BIT return page[1].compound_nr; +#else + return 1UL << compound_order(page); +#endif } /* Returns the number of bytes in this potentially compound page. */ diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 09d9e2c4a2c5..e19af4a90a6c 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -150,11 +150,14 @@ struct page { unsigned char compound_dtor; unsigned char compound_order; atomic_t compound_mapcount; + atomic_t hpage_pinned_refcount; +#ifdef CONFIG_64BIT unsigned int compound_nr; /* 1 << compound_order */ +#endif }; struct { /* Second tail page of compound page */ unsigned long _compound_pad_1; /* compound_head */ - atomic_t hpage_pinned_refcount; + unsigned long _compound_pad_2; /* For both global and memcg */ struct list_head deferred_list; }; Now it's always available for every compound page.
On 1/2/22 13:57, Matthew Wilcox (Oracle) wrote: > Convert hpage_pincount_available() into folio_pincount_available() and > turn hpage_pincount_available() into a wrapper. We don't need to > check folio_test_large() before checking folio_order() as > folio_order() includes a check of folio_test_large(). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/mm.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 98a10412d581..269b5484d66e 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -927,15 +927,19 @@ static inline void destroy_compound_page(struct page *page) > compound_page_dtors[page[1].compound_dtor](page); > } > > -static inline bool hpage_pincount_available(struct page *page) > +static inline bool folio_pincount_available(struct folio *folio) > { > /* > - * Can the page->hpage_pinned_refcount field be used? That field is in > + * Can the folio->hpage_pinned_refcount field be used? That field is in > * the 3rd page of the compound page, so the smallest (2-page) compound > * pages cannot support it. > */ > - page = compound_head(page); > - return PageCompound(page) && compound_order(page) > 1; > + return folio_order(folio) > 1; I see, no need to look at the compound page head, because folio_order() returns zero for tail pages, and neatly avoids all of that. > +} > + > +static inline bool hpage_pincount_available(struct page *page) > +{ > + return folio_pincount_available(page_folio(page)); > } > > static inline int head_compound_pincount(struct page *head) Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
On Tue, Jan 04, 2022 at 01:40:36PM -0800, John Hubbard wrote: > > - page = compound_head(page); > > - return PageCompound(page) && compound_order(page) > 1; > > + return folio_order(folio) > 1; > > I see, no need to look at the compound page head, because folio_order() returns > zero for tail pages, and neatly avoids all of that. Did you mean base pages instead of tail pages? A folio can never be a tail page.
On 1/4/22 21:04, Matthew Wilcox wrote: > On Tue, Jan 04, 2022 at 01:40:36PM -0800, John Hubbard wrote: >>> - page = compound_head(page); >>> - return PageCompound(page) && compound_order(page) > 1; >>> + return folio_order(folio) > 1; >> >> I see, no need to look at the compound page head, because folio_order() returns >> zero for tail pages, and neatly avoids all of that. > > Did you mean base pages instead of tail pages? A folio can never be a > tail page. > Oh right. I was thinking (incorrectly) that there might be some cases during the page-to-folio conversion in which a tail page could sort of slip through, but good point. thanks,
diff --git a/include/linux/mm.h b/include/linux/mm.h index 98a10412d581..269b5484d66e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -927,15 +927,19 @@ static inline void destroy_compound_page(struct page *page) compound_page_dtors[page[1].compound_dtor](page); } -static inline bool hpage_pincount_available(struct page *page) +static inline bool folio_pincount_available(struct folio *folio) { /* - * Can the page->hpage_pinned_refcount field be used? That field is in + * Can the folio->hpage_pinned_refcount field be used? That field is in * the 3rd page of the compound page, so the smallest (2-page) compound * pages cannot support it. */ - page = compound_head(page); - return PageCompound(page) && compound_order(page) > 1; + return folio_order(folio) > 1; +} + +static inline bool hpage_pincount_available(struct page *page) +{ + return folio_pincount_available(page_folio(page)); } static inline int head_compound_pincount(struct page *head)
Convert hpage_pincount_available() into folio_pincount_available() and turn hpage_pincount_available() into a wrapper. We don't need to check folio_test_large() before checking folio_order() as folio_order() includes a check of folio_test_large(). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/mm.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)