All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Hugh Dickins <hughd@google.com>,
	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 20:14:37 -0700 (PDT)	[thread overview]
Message-ID: <e3d322e4-57e6-61d0-23e7-11f96f87415d@google.com> (raw)
In-Reply-To: <YMTdtRZG+7q8OtkK@casper.infradead.org>

On Sat, 12 Jun 2021, Matthew Wilcox wrote:
> 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.

Okay.  I did not mean to imply, by removing those TODOs, that they
should not be done: just that they were a developer's notes to self,
that I found distracting there.

I've restored both TODOs (but changed the second to say
"hugetlb pages" explicitly, rather than the ambiguous "hugepage").

> > --- 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

I don't see what's wrong with what's there, myself.  Unfamiliar territory
to me, but mem_map_next() appears to have the same MAX_ORDER expectation.
Or perhaps you're just suggesting an optimization.

If it were obvious to me, I'd have gladly folded it in; but no,
please send your own patch for that, running it by Mike Kravetz
and Mike Rapoport and David Hildenbrand, I think.

Thanks, v2 follows,
Hugh

  reply	other threads:[~2021-06-13  3:15 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
2021-06-13  3:14   ` Hugh Dickins [this message]
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=e3d322e4-57e6-61d0-23e7-11f96f87415d@google.com \
    --to=hughd@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --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 \
    --cc=willy@infradead.org \
    /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.