On Mon, Mar 2, 2020 at 8:11 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
>
> If THP is disabled, find_subpage can become a no-op.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> ---
> include/linux/pagemap.h | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index b4ea3a5d00e5..8785f60b05f8 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -333,14 +333,19 @@ static inline struct page *grab_cache_page_nowait(struct address_space *mapping,
> mapping_gfp_mask(mapping));
> }
>
> -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);
Is there any specific reason for renaming page to head? Just wondering
since it adds some noise to the patch that wasn't really called out in
the patch description. From what I can tell none of the changes above
this point have any explanation to them in the patch description, and
until I noticed the change below I thought maybe you had the wrong
patch description for this patch.
> - return page + (offset & (compound_nr(page) - 1));
> + return head + (offset & (hpage_nr_pages(head) - 1));
> }
So the patch description refers to this line here, correct?
One thing I am noticing is that it looks like the VM_BUG_ON_PAGE is
now redundant. If I am not not mistaken hpage_nr_pages will call
PageTransHuge which also performs the same check. So do you still need
the VM_BUG_ON_PAGE call in this function?
