| Message ID | 20200212041845.25879-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Large pages in the page cache | expand |
On Tue, Feb 11, 2020 at 08:18:22PM -0800, Matthew Wilcox wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > If THP is disabled, find_subpage can become a no-op. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > include/linux/pagemap.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index 75bdfec49710..0842622cca90 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -340,7 +340,7 @@ static inline struct page *find_subpage(struct page *page, pgoff_t offset) > > VM_BUG_ON_PAGE(PageTail(page), page); > > - return page + (offset & (compound_nr(page) - 1)); > + return page + (offset & (hpage_nr_pages(page) - 1)); > } So just above the quoted code is a if (PageHuge(page)) return page; So how can we get into a compound page that is not a huge page, but only if THP is enabled? (Yes, maybe I'm a little rusty on VM internals). Can you add comments describing the use case of this function and why it does all these checks? It looks like black magic to me.
On Tue, Feb 11, 2020 at 11:41:05PM -0800, Christoph Hellwig wrote: > On Tue, Feb 11, 2020 at 08:18:22PM -0800, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > > > If THP is disabled, find_subpage can become a no-op. > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > include/linux/pagemap.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 75bdfec49710..0842622cca90 100644 > > --- a/include/linux/pagemap.h > > +++ b/include/linux/pagemap.h > > @@ -340,7 +340,7 @@ static inline struct page *find_subpage(struct page *page, pgoff_t offset) > > > > VM_BUG_ON_PAGE(PageTail(page), page); > > > > - return page + (offset & (compound_nr(page) - 1)); > > + return page + (offset & (hpage_nr_pages(page) - 1)); > > } > > So just above the quoted code is a > > if (PageHuge(page)) > return page; > > So how can we get into a compound page that is not a huge page, but > only if THP is enabled? (Yes, maybe I'm a little rusty on VM > internals). That's for hugetlbfs: if (!PageCompound(page)) return 0; page = compound_head(page); return page[1].compound_dtor == HUGETLB_PAGE_DTOR; > Can you add comments describing the use case of this function and why > it does all these checks? It looks like black magic to me. Would this help? -static inline struct page *find_subpage(struct page *page, pgoff_t offset) +/* + * Given the page we found in the page cache, return the page corresponding + * to this offset in the file + */ +static inline struct page *find_subpage(struct page *head, pgoff_t offset) { - if (PageHuge(page)) - return page; + /* HugeTLBfs wants the head page regardless */ + if (PageHuge(head)) + return head; - VM_BUG_ON_PAGE(PageTail(page), page); + VM_BUG_ON_PAGE(PageTail(head), head); - return page + (offset & (hpage_nr_pages(page) - 1)); + return head + (offset & (hpage_nr_pages(head) - 1)); }
On Wed, Feb 12, 2020 at 05:02:00AM -0800, Matthew Wilcox wrote: > > Can you add comments describing the use case of this function and why > > it does all these checks? It looks like black magic to me. > > Would this help? > > -static inline struct page *find_subpage(struct page *page, pgoff_t offset) > +/* > + * Given the page we found in the page cache, return the page corresponding > + * to this offset in the file > + */ > +static inline struct page *find_subpage(struct page *head, pgoff_t offset) > { > - if (PageHuge(page)) > - return page; > + /* HugeTLBfs wants the head page regardless */ > + if (PageHuge(head)) > + return head; > > - VM_BUG_ON_PAGE(PageTail(page), page); > + VM_BUG_ON_PAGE(PageTail(head), head); > > - return page + (offset & (hpage_nr_pages(page) - 1)); > + return head + (offset & (hpage_nr_pages(head) - 1)); Much better.
On Wed, Feb 12, 2020 at 05:02:00AM -0800, Matthew Wilcox wrote: > Would this help? LGTM: Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 75bdfec49710..0842622cca90 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -340,7 +340,7 @@ static inline struct page *find_subpage(struct page *page, pgoff_t offset) VM_BUG_ON_PAGE(PageTail(page), page); - return page + (offset & (compound_nr(page) - 1)); + return page + (offset & (hpage_nr_pages(page) - 1)); } struct page *find_get_entry(struct address_space *mapping, pgoff_t offset);