public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: "David Hildenbrand (Arm)" <david@kernel.org>
To: Alexander Gordeev <agordeev@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Lorenzo Stoakes <ljs@kernel.org>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	linux-s390@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Ryan Roberts <Ryan.Roberts@arm.com>,
	"Christophe Leroy (CS GROUP)" <chleroy@kernel.org>
Subject: Re: [PATCH] mm/page_vma_mapped_walk: add missing pgtable entry accessors
Date: Mon, 27 Apr 2026 11:24:31 +0200	[thread overview]
Message-ID: <08ecabe9-0664-4aea-82fb-f9cb1739f762@kernel.org> (raw)
In-Reply-To: <69c4c12c-7cdc-4a17-9eb7-6fb1a61f9834@kernel.org>

On 4/27/26 11:02, David Hildenbrand (Arm) wrote:
> On 4/27/26 07:20, Alexander Gordeev wrote:
>> Convert pgtable direct entry dereferences to the corresponding
>> pXdp_get() accessors. Use ptep_get_lockless() variant for PTE
>> reads when no lock is taken.
>>
>> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
>> ---
>>  mm/page_vma_mapped.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index b38a1d00c971..a4520bb10d2a 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -41,7 +41,7 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw, pmd_t *pmdvalp,
>>  	if (!pvmw->pte)
>>  		return false;
>>  
>> -	ptent = ptep_get(pvmw->pte);
>> +	ptent = ptep_get_lockless(pvmw->pte);
>>  
>>  	if (pte_none(ptent)) {
>>  		return false;
>> @@ -219,17 +219,17 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>>  restart:
>>  	do {
>>  		pgd = pgd_offset(mm, pvmw->address);
>> -		if (!pgd_present(*pgd)) {
>> +		if (!pgd_present(pgdp_get(pgd))) {
>>  			step_forward(pvmw, PGDIR_SIZE);
>>  			continue;
>>  		}
>>  		p4d = p4d_offset(pgd, pvmw->address);
>> -		if (!p4d_present(*p4d)) {
>> +		if (!p4d_present(p4dp_get(p4d))) {
>>  			step_forward(pvmw, P4D_SIZE);
>>  			continue;
>>  		}
>>  		pud = pud_offset(p4d, pvmw->address);
>> -		if (!pud_present(*pud)) {
>> +		if (!pud_present(pudp_get(pud))) {
>>  			step_forward(pvmw, PUD_SIZE);
>>  			continue;
> 
> Wasn't there a problem with folded page tables, where we would no longer be able
> to optimize out the folded page table accesses?
> 
> I thought we discussed ways to resolve that. Let me dig ...

Here is what I have after the discussion following
https://lore.kernel.org/all/0019d675-ce3d-4a5c-89ed-f126c45145c9@kernel.org/.

Completely untested of course:

From b4715d49feb588032c6bb44bd97cf06739b5c99f Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Mon, 27 Apr 2026 11:16:21 +0200
Subject: [PATCH] mm: optimize pmdp_get() and friends for folded pagetable
 levels

Using pmdp_get() and friends in common code on a kernel config with
folded page tables is suboptimal: pdmp_get() and friends defaults to a
READ_ONCE(), forcing the compiler to actually read that value even though
it will not actually be used afterwards.

This was recently reported by Christophe Leroy [1].

Once we realize that the output of pdmp_get() on these kernel configs is
entirely ignored, as pmd_present()==1 and pmd_leaf()==0 are just
hard-coded, we can just make it return some dummy value.

pmd_offset() is expected to be called with the pudp afterwards, simply
performing a typecast of the pudp pointer to a pmdp pointer.

Let's introduce a pmd_offset_lockless() that does exactly the same:
perform a typecast.

[1] https://lore.kernel.org/all/0019d675-ce3d-4a5c-89ed-f126c45145c9@kernel.org/

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 include/asm-generic/pgtable-nop4d.h |  8 ++++++++
 include/asm-generic/pgtable-nopmd.h | 19 +++++++++++++++++++
 include/asm-generic/pgtable-nopud.h |  8 ++++++++
 3 files changed, 35 insertions(+)

diff --git a/include/asm-generic/pgtable-nop4d.h b/include/asm-generic/pgtable-nop4d.h
index 03b7dae47dd4..cfe87036d61c 100644
--- a/include/asm-generic/pgtable-nop4d.h
+++ b/include/asm-generic/pgtable-nop4d.h
@@ -32,6 +32,14 @@ static inline void pgd_clear(pgd_t *pgd)	{ }
  */
 #define set_pgd(pgdptr, pgdval)	set_p4d((p4d_t *)(pgdptr), (p4d_t) { pgdval })
 
+static inline pgd_t pgdp_get(pgd_t *p4dp)
+{
+	pgd_t dummy = { 0 };
+
+	return dummy;
+}
+#define pgdp_get pgdp_get
+
 static inline p4d_t *p4d_offset(pgd_t *pgd, unsigned long address)
 {
 	return (p4d_t *)pgd;
diff --git a/include/asm-generic/pgtable-nopmd.h b/include/asm-generic/pgtable-nopmd.h
index 8ffd64e7a24c..e87a42407563 100644
--- a/include/asm-generic/pgtable-nopmd.h
+++ b/include/asm-generic/pgtable-nopmd.h
@@ -43,12 +43,31 @@ static inline void pud_clear(pud_t *pud)	{ }
  */
 #define set_pud(pudptr, pudval)			set_pmd((pmd_t *)(pudptr), (pmd_t) { pudval })
 
+static inline pud_t pudp_get(pud_t *pudp)
+{
+	pud_t dummy = { 0 };
+
+	/*
+	 * Given that pud_present()==1 and pud_leaf==0, page table walking code
+	 * treats this like a page table and calls pmd_offset() /
+	 * pmd_offset_lockless() with pudp, ignoring the returned value.
+	 */
+	return dummy;
+}
+#define pudp_get pudp_get
+
 static inline pmd_t * pmd_offset(pud_t * pud, unsigned long address)
 {
 	return (pmd_t *)pud;
 }
 #define pmd_offset pmd_offset
 
+static inline pmd_t * pmd_offset_lockless(pud_t *pud, puf_t pud, unsigned long address)
+{
+	return (pmd_t *)pud;
+}
+#define pmd_offset_lockless pmd_offset_lockless
+
 #define pmd_val(x)				(pud_val((x).pud))
 #define __pmd(x)				((pmd_t) { __pud(x) } )
 
diff --git a/include/asm-generic/pgtable-nopud.h b/include/asm-generic/pgtable-nopud.h
index eb70c6d7ceff..2cacd9571b2f 100644
--- a/include/asm-generic/pgtable-nopud.h
+++ b/include/asm-generic/pgtable-nopud.h
@@ -39,6 +39,14 @@ static inline void p4d_clear(p4d_t *p4d)	{ }
  */
 #define set_p4d(p4dptr, p4dval)	set_pud((pud_t *)(p4dptr), (pud_t) { p4dval })
 
+static inline p4d_t p4dp_get(p4d_t *p4dp)
+{
+	p4d_t dummy = { 0 };
+
+	return dummy;
+}
+#define p4dp_get p4dp_get
+
 static inline pud_t *pud_offset(p4d_t *p4d, unsigned long address)
 {
 	return (pud_t *)p4d;
-- 
2.43.0


-- 
Cheers,

David


      reply	other threads:[~2026-04-27  9:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27  5:20 [PATCH] mm/page_vma_mapped_walk: add missing pgtable entry accessors Alexander Gordeev
2026-04-27  5:47 ` Anshuman Khandual
2026-04-27  8:40 ` Oscar Salvador
2026-04-27  8:49   ` Anshuman Khandual
2026-04-27  9:01   ` Alexander Gordeev
2026-04-27  9:02 ` David Hildenbrand (Arm)
2026-04-27  9:24   ` David Hildenbrand (Arm) [this message]

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=08ecabe9-0664-4aea-82fb-f9cb1739f762@kernel.org \
    --to=david@kernel.org \
    --cc=Ryan.Roberts@arm.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=chleroy@kernel.org \
    --cc=gerald.schaefer@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=ljs@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox