All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Zhang Yi <wetpzy@gmail.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Darren Hart <dvhart@infradead.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Neel Natu <neelnatu@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm, futex: Fix shared futex pgoff on shmem huge page
Date: Sat, 12 Jun 2021 17:15:49 +0100	[thread overview]
Message-ID: <YMTdtRZG+7q8OtkK@casper.infradead.org> (raw)
In-Reply-To: <45e8fd67-51fd-7828-fe43-d261d6c33727@google.com>

On Fri, Jun 11, 2021 at 09:31:16PM -0700, Hugh Dickins wrote:
> +++ linux/include/linux/pagemap.h	2021-06-11 17:30:28.726720252 -0700
> @@ -516,8 +516,7 @@ static inline struct page *read_mapping_
>  }
>  
>  /*
> - * Get index of the page with in radix-tree
> - * (TODO: remove once hugetlb pages will have ->index in PAGE_SIZE)
> + * Get index of the page within radix-tree (but not for hugetlb pages).
>   */

I think the TODO should be retained.  It's still something that I
intend to do.

>  static inline pgoff_t page_to_pgoff(struct page *page)
>  {
> -	if (unlikely(PageHeadHuge(page)))
> -		return page->index << compound_order(page);
> -
> +	if (unlikely(PageHuge(page))) {
> +		extern pgoff_t hugetlb_basepage_index(struct page *page);
> +		return hugetlb_basepage_index(page);
> +	}
>  	return page_to_index(page);
>  }

Yes, this change absolutely makes sense.  It's just a landmine if it
returns the right answer for some tail pages but garbage for other
kinds of tail pages.

> --- 5.13-rc5/mm/hugetlb.c	2021-06-06 16:57:26.263006733 -0700
> +++ linux/mm/hugetlb.c	2021-06-11 17:30:28.730720276 -0700
> @@ -1588,15 +1588,12 @@ struct address_space *hugetlb_page_mappi
>  	return NULL;
>  }
>  
> -pgoff_t __basepage_index(struct page *page)
> +pgoff_t hugetlb_basepage_index(struct page *page)
>  {
>  	struct page *page_head = compound_head(page);
>  	pgoff_t index = page_index(page_head);
>  	unsigned long compound_idx;
>  
> -	if (!PageHuge(page_head))
> -		return page_index(page);
> -
>  	if (compound_order(page_head) >= MAX_ORDER)
>  		compound_idx = page_to_pfn(page) - page_to_pfn(page_head);
>  	else
> 

urgh.  this trailing bit should be:

#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
	compound_idx = page_to_pfn(page) - page_to_pfn(page_head);
#else
	compound_idx = page - page_head;
#endif

  reply	other threads:[~2021-06-12 16:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-12  4:31 [PATCH] mm, futex: Fix shared futex pgoff on shmem huge page Hugh Dickins
2021-06-12 16:15 ` Matthew Wilcox [this message]
2021-06-13  3:14   ` Hugh Dickins
2021-06-13  3:16   ` [PATCH v2] " Hugh Dickins
2021-06-13 10:27     ` Matthew Wilcox
2021-06-15 14:53     ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YMTdtRZG+7q8OtkK@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=hughd@google.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mike.kravetz@oracle.com \
    --cc=mingo@redhat.com \
    --cc=neelnatu@google.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=wetpzy@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.