mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
@ 2026-11-04  9:10 Hui Zhu
  2026-11-05 10:12 ` David Hildenbrand
  2026-11-05 12:00 ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 4+ messages in thread
From: Hui Zhu @ 2026-11-04  9:10 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, linux-kernel
  Cc: Hui Zhu, stable+noautosel

From: Hui Zhu <zhuhui@kylinos.cn>

KCSAN reports a data-race in do_read_fault / pmd_install; the same code
is in mainline:

  write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
   native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
   pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
   pmd_install+0xe0/0x120 mm/memory.c:444
   finish_fault+0x5bc/0xb00 mm/memory.c:5204
   do_cow_fault mm/memory.c:5432 [inline]
   do_fault+0x5d0/0x6e0 mm/memory.c:5526
   handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
   __mm_populate+0x144/0x270 mm/gup.c:2207
   __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764

  read to 0xffff8d52a75fb810 of 8 bytes by task 27527 on cpu 9:
   do_fault_around mm/memory.c:5341 [inline]
   do_read_fault+0x162/0x670 mm/memory.c:5381
   do_fault+0x31a/0x6e0 mm/memory.c:5524
   handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
   faultin_page_range+0x227/0x530 mm/gup.c:2156
   __x64_sys_madvise+0x67/0x80 mm/madvise.c:1479

  value changed: 0x0000000000000000 -> 0x000000001386c067

Both paths only hold mmap_lock for read, so do_fault_around() tests
vmf->pmd while pmd_install() populates it under the pmd lock.  The race is
benign: a stale pmd_none() only wastes the prealloc_pte, which do_fault()
frees again, and a pmd seen as populated only makes ->map_pages() find no
PTE and do_read_fault() fall back to __do_fault().

The load still has to be lockless-safe.  The store side already is: on
i386-PAE, pmd_install() publishes both halves of the pmd with a single
cmpxchg8b.  The load side is not.  pmdp_get() is a plain READ_ONCE() over
a pmd_t that is wider than a word there, so it becomes two 32-bit loads
and can return the low half of one pmd together with the high half of
another.  Use pmdp_get_lockless(), the accessor for pmd reads that do not
hold the pmd lock: it re-reads until both halves come from the same pmd.
It is also what __pte_offset_map() already uses on this very pmd a few
instructions later.

Only X86_32+X86_PAE and SH+X2TLB compile the retry loop, these being the
GUP_GET_PXX_LOW_HIGH selectors that have more than two page table levels.
mips32 with 64-bit physical addresses selects GUP_GET_PXX_LOW_HIGH too,
but is 2-level and so falls back to pmdp_get().  On every other
configuration, x86-64 included, pmdp_get_lockless() is pmdp_get(): the
pmd is loaded in one go, the READ_ONCE() silences the report the same
way, and there is no functional change.

Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
Cc: stable+noautosel@kernel.org # KCSAN annotation only, no functional change
Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
Acked-by: Lorenzo Stoakes <ljs@kernel.org>
---
Changelog:
v4:
Commit message only, the diff is identical to v3.  Spell out the affected
arches and add the lore links for v1-v3 (Lorenzo Stoakes).  Reword the
cmpxchg8b sentence: the store is already atomic, it is the load that tears.
Add the Fixes: tag (David Hildenbrand).
v3:
https://lore.kernel.org/all/20261103014749.96764-1-hui.zhu@linux.dev/
Shorten the comment to one line (David Hildenbrand).
v2:
https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@linux.dev/
Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
v1:
https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@linux.dev/

 mm/memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index 0fb676dd2bd63..f25c2ce0ef153 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6017,7 +6017,8 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
 	to_pte = min3(from_pte + nr_pages, (pgoff_t)PTRS_PER_PTE,
 		      pte_off + vma_pages(vmf->vma) - vma_off) - 1;
 
-	if (pmd_none(*vmf->pmd)) {
+	/* pmd_install() rechecks pmd_none() under the pmd lock. */
+	if (pmd_none(pmdp_get_lockless(vmf->pmd))) {
 		vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm);
 		if (!vmf->prealloc_pte)
 			return VM_FAULT_OOM;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
  2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
@ 2026-11-05 10:12 ` David Hildenbrand
  2026-11-05 12:00 ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 4+ messages in thread
From: David Hildenbrand @ 2026-11-05 10:12 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
	Suren Baghdasaryan, Michal Hocko, linux-mm, linux-kernel,
	Hui Zhu
  Cc: Hui Zhu, stable+noautosel

On Wed, 04 Nov 2026 17:10:50 +0800, Hui Zhu wrote:
> KCSAN reports a data-race in do_read_fault / pmd_install; the same code
> is in mainline:
> 
>   write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
>    native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
>    pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
>    pmd_install+0xe0/0x120 mm/memory.c:444
>    finish_fault+0x5bc/0xb00 mm/memory.c:5204
>    do_cow_fault mm/memory.c:5432 [inline]
>    do_fault+0x5d0/0x6e0 mm/memory.c:5526
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    __mm_populate+0x144/0x270 mm/gup.c:2207
>    __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764
> 
> [...]

Applied, thanks!

tree: mm/core.git
branch: core-7.5.misc

Applied patches will appear soon in mm-next, followed by linux-next.

Please report any outstanding bugs that were missed during review, so we
can either drop the patch series or decide how to fix it up.

It is encouraged to provide Acked-bys, Reviewed-bys, and Tested-bys even
though the patches have already been applied. If possible, patch trailers
will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. Further, this is not a guarantee that the
patches will go upstream. If in doubt, please check the listed branch.

[1/1] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
      https://git.kernel.org/mm/core/c/6ab55cfd

-- 
Cheers,

David


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
  2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
  2026-11-05 10:12 ` David Hildenbrand
@ 2026-11-05 12:00 ` David Hildenbrand (Arm)
  2026-11-06  3:03   ` Hui Zhu
  1 sibling, 1 reply; 4+ messages in thread
From: David Hildenbrand (Arm) @ 2026-11-05 12:00 UTC (permalink / raw)
  To: Hui Zhu, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	linux-mm, linux-kernel, Geert Uytterhoeven, Sam Creasey
  Cc: Hui Zhu, stable+noautosel, linux-m68k

On 11/4/26 10:10, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> KCSAN reports a data-race in do_read_fault / pmd_install; the same code
> is in mainline:
> 
>   write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
>    native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
>    pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
>    pmd_install+0xe0/0x120 mm/memory.c:444
>    finish_fault+0x5bc/0xb00 mm/memory.c:5204
>    do_cow_fault mm/memory.c:5432 [inline]
>    do_fault+0x5d0/0x6e0 mm/memory.c:5526
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    __mm_populate+0x144/0x270 mm/gup.c:2207
>    __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764
> 
>   read to 0xffff8d52a75fb810 of 8 bytes by task 27527 on cpu 9:
>    do_fault_around mm/memory.c:5341 [inline]
>    do_read_fault+0x162/0x670 mm/memory.c:5381
>    do_fault+0x31a/0x6e0 mm/memory.c:5524
>    handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>    faultin_page_range+0x227/0x530 mm/gup.c:2156
>    __x64_sys_madvise+0x67/0x80 mm/madvise.c:1479
> 
>   value changed: 0x0000000000000000 -> 0x000000001386c067
> 
> Both paths only hold mmap_lock for read, so do_fault_around() tests
> vmf->pmd while pmd_install() populates it under the pmd lock.  The race is
> benign: a stale pmd_none() only wastes the prealloc_pte, which do_fault()
> frees again, and a pmd seen as populated only makes ->map_pages() find no
> PTE and do_read_fault() fall back to __do_fault().
> 
> The load still has to be lockless-safe.  The store side already is: on
> i386-PAE, pmd_install() publishes both halves of the pmd with a single
> cmpxchg8b.  The load side is not.  pmdp_get() is a plain READ_ONCE() over
> a pmd_t that is wider than a word there, so it becomes two 32-bit loads
> and can return the low half of one pmd together with the high half of
> another.  Use pmdp_get_lockless(), the accessor for pmd reads that do not
> hold the pmd lock: it re-reads until both halves come from the same pmd.
> It is also what __pte_offset_map() already uses on this very pmd a few
> instructions later.
> 
> Only X86_32+X86_PAE and SH+X2TLB compile the retry loop, these being the
> GUP_GET_PXX_LOW_HIGH selectors that have more than two page table levels.
> mips32 with 64-bit physical addresses selects GUP_GET_PXX_LOW_HIGH too,
> but is 2-level and so falls back to pmdp_get().  On every other
> configuration, x86-64 included, pmdp_get_lockless() is pmdp_get(): the
> pmd is loaded in one go, the READ_ONCE() silences the report the same
> way, and there is no functional change.
> 
> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
> Cc: stable+noautosel@kernel.org # KCSAN annotation only, no functional change
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> Changelog:
> v4:
> Commit message only, the diff is identical to v3.  Spell out the affected
> arches and add the lore links for v1-v3 (Lorenzo Stoakes).  Reword the
> cmpxchg8b sentence: the store is already atomic, it is the load that tears.
> Add the Fixes: tag (David Hildenbrand).
> v3:
> https://lore.kernel.org/all/20261103014749.96764-1-hui.zhu@linux.dev/
> Shorten the comment to one line (David Hildenbrand).
> v2:
> https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@linux.dev/
> Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
> v1:
> https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@linux.dev/
> 
>  mm/memory.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 0fb676dd2bd63..f25c2ce0ef153 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6017,7 +6017,8 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
>  	to_pte = min3(from_pte + nr_pages, (pgoff_t)PTRS_PER_PTE,
>  		      pte_off + vma_pages(vmf->vma) - vma_off) - 1;
>  
> -	if (pmd_none(*vmf->pmd)) {
> +	/* pmd_install() rechecks pmd_none() under the pmd lock. */
> +	if (pmd_none(pmdp_get_lockless(vmf->pmd))) {
>  		vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm);
>  		if (!vmf->prealloc_pte)
>  			return VM_FAULT_OOM;

Fun, this breaks some odd m68k configs because they do some legacy weird stuff with pmd_none().

I'll throw in the following cleanup. CCing m68k maintainers: I would drag
this along with this fix here unless you disagree.

From e3c739933a04743e3923f6f39a603e93f2700304 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Thu, 5 Nov 2026 12:49:38 +0100
Subject: [PATCH] m68k/mm: simplify
 pmd_none()/pmd_val()/pmd_present()/pmd_bad()

pmd_t on m68k used to contain 16 words. Commit ef22d8abd876 ("m68k:
mm: Restructure Motorola MMU page-table layout") reduced Motorola PMDs
to one word; Sun3 and ColdFire use word-sized folded PMDs.

Use direct member access in pmd_val() and replace the Sun3 and ColdFire
pointer-taking predicate wrappers with inline functions taking values.

This allows us to write something like:

  if (pmd_none(pmdp_get(pmdp)))

While at it remove unused pmd_present2() and commented-out pmd_bad()
on sun3.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 arch/m68k/include/asm/mcf_pgtable.h  | 20 +++++++++++++++-----
 arch/m68k/include/asm/page.h         |  2 +-
 arch/m68k/include/asm/sun3_pgtable.h | 23 +++++++++++++++--------
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/arch/m68k/include/asm/mcf_pgtable.h b/arch/m68k/include/asm/mcf_pgtable.h
index f45a882238dbb..4efa8e3e1e98a 100644
--- a/arch/m68k/include/asm/mcf_pgtable.h
+++ b/arch/m68k/include/asm/mcf_pgtable.h
@@ -130,11 +130,21 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr,
 
 #define pte_page(pte)	virt_to_page(__pte_page(pte))
 
-static inline int pmd_none2(pmd_t *pmd) { return !pmd_val(*pmd); }
-#define pmd_none(pmd) pmd_none2(&(pmd))
-static inline int pmd_bad2(pmd_t *pmd) { return 0; }
-#define pmd_bad(pmd) pmd_bad2(&(pmd))
-#define pmd_present(pmd) (!pmd_none2(&(pmd)))
+static inline int pmd_none(pmd_t pmd)
+{
+	return !pmd_val(pmd);
+}
+
+static inline int pmd_bad(pmd_t pmd)
+{
+	return 0;
+}
+
+static inline int pmd_present(pmd_t pmd)
+{
+	return !pmd_none(pmd);
+}
+
 static inline void pmd_clear(pmd_t *pmdp) { pmd_val(*pmdp) = 0; }
 
 
diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
index d30f8b2f15927..09a2c020ec992 100644
--- a/arch/m68k/include/asm/page.h
+++ b/arch/m68k/include/asm/page.h
@@ -17,7 +17,7 @@
  */
 #if !defined(CONFIG_MMU) || CONFIG_PGTABLE_LEVELS == 3
 typedef struct { unsigned long pmd; } pmd_t;
-#define pmd_val(x)	((&x)->pmd)
+#define pmd_val(x)	((x).pmd)
 #define __pmd(x)	((pmd_t) { (x) } )
 #endif
 
diff --git a/arch/m68k/include/asm/sun3_pgtable.h b/arch/m68k/include/asm/sun3_pgtable.h
index 704442a391fd7..5867d24032655 100644
--- a/arch/m68k/include/asm/sun3_pgtable.h
+++ b/arch/m68k/include/asm/sun3_pgtable.h
@@ -109,14 +109,21 @@ static inline void pte_clear (struct mm_struct *mm, unsigned long addr, pte_t *p
 #define pmd_page(pmd)		virt_to_page((void *)pmd_page_vaddr(pmd))
 
 
-static inline int pmd_none2 (pmd_t *pmd) { return !pmd_val (*pmd); }
-#define pmd_none(pmd) pmd_none2(&(pmd))
-//static inline int pmd_bad (pmd_t pmd) { return (pmd_val (pmd) & SUN3_PMD_MASK) != SUN3_PMD_MAGIC; }
-static inline int pmd_bad2 (pmd_t *pmd) { return 0; }
-#define pmd_bad(pmd) pmd_bad2(&(pmd))
-static inline int pmd_present2 (pmd_t *pmd) { return pmd_val (*pmd) & SUN3_PMD_VALID; }
-/* #define pmd_present(pmd) pmd_present2(&(pmd)) */
-#define pmd_present(pmd) (!pmd_none2(&(pmd)))
+static inline int pmd_none(pmd_t pmd)
+{
+	return !pmd_val(pmd);
+}
+
+static inline int pmd_bad(pmd_t pmd)
+{
+	return 0;
+}
+
+static inline int pmd_present(pmd_t pmd)
+{
+	return !pmd_none(pmd);
+}
+
 static inline void pmd_clear (pmd_t *pmdp) { pmd_val (*pmdp) = 0; }
 
 /*
-- 
2.43.0


-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
  2026-11-05 12:00 ` David Hildenbrand (Arm)
@ 2026-11-06  3:03   ` Hui Zhu
  0 siblings, 0 replies; 4+ messages in thread
From: Hui Zhu @ 2026-11-06  3:03 UTC (permalink / raw)
  To: David Hildenbrand (Arm),
	Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
	Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-mm,
	linux-kernel, Geert Uytterhoeven, Sam Creasey
  Cc: Hui Zhu, stable+noautosel, linux-m68k


2026/11/5 20:00, David Hildenbrand (Arm) :
> On 11/4/26 10:10, Hui Zhu wrote:
>> From: Hui Zhu <zhuhui@kylinos.cn>
>>
>> KCSAN reports a data-race in do_read_fault / pmd_install; the same code
>> is in mainline:
>>
>>    write (marked) to 0xffff8d52a75fb810 of 8 bytes by task 27487 on cpu 5:
>>     native_set_pmd arch/x86/include/asm/pgtable_64.h:83 [inline]
>>     pmd_populate arch/x86/include/asm/pgalloc.h:84 [inline]
>>     pmd_install+0xe0/0x120 mm/memory.c:444
>>     finish_fault+0x5bc/0xb00 mm/memory.c:5204
>>     do_cow_fault mm/memory.c:5432 [inline]
>>     do_fault+0x5d0/0x6e0 mm/memory.c:5526
>>     handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>>     __mm_populate+0x144/0x270 mm/gup.c:2207
>>     __do_sys_mlockall+0x2cd/0x380 mm/mlock.c:764
>>
>>    read to 0xffff8d52a75fb810 of 8 bytes by task 27527 on cpu 9:
>>     do_fault_around mm/memory.c:5341 [inline]
>>     do_read_fault+0x162/0x670 mm/memory.c:5381
>>     do_fault+0x31a/0x6e0 mm/memory.c:5524
>>     handle_mm_fault+0x211/0x4c0 mm/memory.c:6218
>>     faultin_page_range+0x227/0x530 mm/gup.c:2156
>>     __x64_sys_madvise+0x67/0x80 mm/madvise.c:1479
>>
>>    value changed: 0x0000000000000000 -> 0x000000001386c067
>>
>> Both paths only hold mmap_lock for read, so do_fault_around() tests
>> vmf->pmd while pmd_install() populates it under the pmd lock.  The race is
>> benign: a stale pmd_none() only wastes the prealloc_pte, which do_fault()
>> frees again, and a pmd seen as populated only makes ->map_pages() find no
>> PTE and do_read_fault() fall back to __do_fault().
>>
>> The load still has to be lockless-safe.  The store side already is: on
>> i386-PAE, pmd_install() publishes both halves of the pmd with a single
>> cmpxchg8b.  The load side is not.  pmdp_get() is a plain READ_ONCE() over
>> a pmd_t that is wider than a word there, so it becomes two 32-bit loads
>> and can return the low half of one pmd together with the high half of
>> another.  Use pmdp_get_lockless(), the accessor for pmd reads that do not
>> hold the pmd lock: it re-reads until both halves come from the same pmd.
>> It is also what __pte_offset_map() already uses on this very pmd a few
>> instructions later.
>>
>> Only X86_32+X86_PAE and SH+X2TLB compile the retry loop, these being the
>> GUP_GET_PXX_LOW_HIGH selectors that have more than two page table levels.
>> mips32 with 64-bit physical addresses selects GUP_GET_PXX_LOW_HIGH too,
>> but is 2-level and so falls back to pmdp_get().  On every other
>> configuration, x86-64 included, pmdp_get_lockless() is pmdp_get(): the
>> pmd is loaded in one go, the READ_ONCE() silences the report the same
>> way, and there is no functional change.
>>
>> Fixes: 7267ec008b5c ("mm: postpone page table allocation until we have page to map")
>> Cc: stable+noautosel@kernel.org # KCSAN annotation only, no functional change
>> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
>> Acked-by: Lorenzo Stoakes <ljs@kernel.org>
>> ---
>> Changelog:
>> v4:
>> Commit message only, the diff is identical to v3.  Spell out the affected
>> arches and add the lore links for v1-v3 (Lorenzo Stoakes).  Reword the
>> cmpxchg8b sentence: the store is already atomic, it is the load that tears.
>> Add the Fixes: tag (David Hildenbrand).
>> v3:
>> https://lore.kernel.org/all/20261103014749.96764-1-hui.zhu@linux.dev/
>> Shorten the comment to one line (David Hildenbrand).
>> v2:
>> https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@linux.dev/
>> Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
>> v1:
>> https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@linux.dev/
>>
>>   mm/memory.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 0fb676dd2bd63..f25c2ce0ef153 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -6017,7 +6017,8 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
>>   	to_pte = min3(from_pte + nr_pages, (pgoff_t)PTRS_PER_PTE,
>>   		      pte_off + vma_pages(vmf->vma) - vma_off) - 1;
>>   
>> -	if (pmd_none(*vmf->pmd)) {
>> +	/* pmd_install() rechecks pmd_none() under the pmd lock. */
>> +	if (pmd_none(pmdp_get_lockless(vmf->pmd))) {
>>   		vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm);
>>   		if (!vmf->prealloc_pte)
>>   			return VM_FAULT_OOM;
> Fun, this breaks some odd m68k configs because they do some legacy weird stuff with pmd_none().
>
> I'll throw in the following cleanup. CCing m68k maintainers: I would drag
> this along with this fix here unless you disagree.
>
>  From e3c739933a04743e3923f6f39a603e93f2700304 Mon Sep 17 00:00:00 2001
> From: "David Hildenbrand (Arm)" <david@kernel.org>
> Date: Thu, 5 Nov 2026 12:49:38 +0100
> Subject: [PATCH] m68k/mm: simplify
>   pmd_none()/pmd_val()/pmd_present()/pmd_bad()
>
> pmd_t on m68k used to contain 16 words. Commit ef22d8abd876 ("m68k:
> mm: Restructure Motorola MMU page-table layout") reduced Motorola PMDs
> to one word; Sun3 and ColdFire use word-sized folded PMDs.
>
> Use direct member access in pmd_val() and replace the Sun3 and ColdFire
> pointer-taking predicate wrappers with inline functions taking values.
>
> This allows us to write something like:
>
>    if (pmd_none(pmdp_get(pmdp)))
>
> While at it remove unused pmd_present2() and commented-out pmd_bad()
> on sun3.
>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
>   arch/m68k/include/asm/mcf_pgtable.h  | 20 +++++++++++++++-----
>   arch/m68k/include/asm/page.h         |  2 +-
>   arch/m68k/include/asm/sun3_pgtable.h | 23 +++++++++++++++--------
>   3 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/arch/m68k/include/asm/mcf_pgtable.h b/arch/m68k/include/asm/mcf_pgtable.h
> index f45a882238dbb..4efa8e3e1e98a 100644
> --- a/arch/m68k/include/asm/mcf_pgtable.h
> +++ b/arch/m68k/include/asm/mcf_pgtable.h
> @@ -130,11 +130,21 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr,
>   
>   #define pte_page(pte)	virt_to_page(__pte_page(pte))
>   
> -static inline int pmd_none2(pmd_t *pmd) { return !pmd_val(*pmd); }
> -#define pmd_none(pmd) pmd_none2(&(pmd))
> -static inline int pmd_bad2(pmd_t *pmd) { return 0; }
> -#define pmd_bad(pmd) pmd_bad2(&(pmd))
> -#define pmd_present(pmd) (!pmd_none2(&(pmd)))
> +static inline int pmd_none(pmd_t pmd)
> +{
> +	return !pmd_val(pmd);
> +}
> +
> +static inline int pmd_bad(pmd_t pmd)
> +{
> +	return 0;
> +}
> +
> +static inline int pmd_present(pmd_t pmd)
> +{
> +	return !pmd_none(pmd);
> +}
> +
>   static inline void pmd_clear(pmd_t *pmdp) { pmd_val(*pmdp) = 0; }
>   
>   
> diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
> index d30f8b2f15927..09a2c020ec992 100644
> --- a/arch/m68k/include/asm/page.h
> +++ b/arch/m68k/include/asm/page.h
> @@ -17,7 +17,7 @@
>    */
>   #if !defined(CONFIG_MMU) || CONFIG_PGTABLE_LEVELS == 3
>   typedef struct { unsigned long pmd; } pmd_t;
> -#define pmd_val(x)	((&x)->pmd)
> +#define pmd_val(x)	((x).pmd)
>   #define __pmd(x)	((pmd_t) { (x) } )
>   #endif
>   
> diff --git a/arch/m68k/include/asm/sun3_pgtable.h b/arch/m68k/include/asm/sun3_pgtable.h
> index 704442a391fd7..5867d24032655 100644
> --- a/arch/m68k/include/asm/sun3_pgtable.h
> +++ b/arch/m68k/include/asm/sun3_pgtable.h
> @@ -109,14 +109,21 @@ static inline void pte_clear (struct mm_struct *mm, unsigned long addr, pte_t *p
>   #define pmd_page(pmd)		virt_to_page((void *)pmd_page_vaddr(pmd))
>   
>   
> -static inline int pmd_none2 (pmd_t *pmd) { return !pmd_val (*pmd); }
> -#define pmd_none(pmd) pmd_none2(&(pmd))
> -//static inline int pmd_bad (pmd_t pmd) { return (pmd_val (pmd) & SUN3_PMD_MASK) != SUN3_PMD_MAGIC; }
> -static inline int pmd_bad2 (pmd_t *pmd) { return 0; }
> -#define pmd_bad(pmd) pmd_bad2(&(pmd))
> -static inline int pmd_present2 (pmd_t *pmd) { return pmd_val (*pmd) & SUN3_PMD_VALID; }
> -/* #define pmd_present(pmd) pmd_present2(&(pmd)) */
> -#define pmd_present(pmd) (!pmd_none2(&(pmd)))
> +static inline int pmd_none(pmd_t pmd)
> +{
> +	return !pmd_val(pmd);
> +}
> +
> +static inline int pmd_bad(pmd_t pmd)
> +{
> +	return 0;
> +}
> +
> +static inline int pmd_present(pmd_t pmd)
> +{
> +	return !pmd_none(pmd);
> +}
> +
>   static inline void pmd_clear (pmd_t *pmdp) { pmd_val (*pmdp) = 0; }
>   
>   /*

Hi David,

Thanks for picking this up, and for the m68k cleanup!

Tested-by: Hui Zhu <zhuhui@kylinos.cn>

Tested with gcc-m68k-linux-gnu 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04), on
three configs that between them cover all three m68k families:

   amiga_defconfig                    Motorola MMU, PGTABLE_LEVELS=3
   sun3_defconfig                     SUN3,  PGTABLE_LEVELS=2
   m5208evb_defconfig + CONFIG_MMU=y  ColdFire,  PGTABLE_LEVELS=2

Without your patch none of the three can build mm/memory.o, and each fails
at a different site:

   arch/m68k/include/asm/page.h:20:27: error: lvalue required as unary 
'&' operand
   arch/m68k/include/asm/sun3_pgtable.h:113:33: error: lvalue required 
as unary '&' operand
   arch/m68k/include/asm/mcf_pgtable.h:134:33: error: lvalue required as 
unary '&' operand

With it, all three build a full vmlinux with zero warnings.

Two things that might save you some time:

- m68k looks like the only arch with this pattern. Grepping arch/ and
   include/ for '(&x)->pmd' and '(&(pmd))' finds nothing outside arch/m68k/,
   and pmd_none2()/pmd_bad2()/pmd_present2() only exist in mcf_pgtable.h and
   sun3_pgtable.h. So your three hunks look complete to me.

- No m52xx/m54xx defconfig enables CONFIG_MMU, so the ColdFire path through
   mcf_pgtable.h has no defconfig coverage at all; you have to flip it on by
   hand (scripts/config -e MMU && make olddefconfig) to reach it. That is
   probably how this went unnoticed for so long - my patch has been breaking
   m68k since v1, because pmdp_get() returns by value just like
   pmdp_get_lockless() does.

I also checked whether your patch changes code generation, by building the
same tree with and without it and comparing every object with objdump -d
(plain cmp is useless here, timestamps and link layout shift everything).
Counting only objects whose disassembly actually differs:

   amiga      3733 objects, 0 changed
   sun3       3553 objects, 3 changed: mm/memory.o, mm/pgtable-generic.o,
                                       mm/vmalloc.o
   m5208evb    716 objects, 1 changed: mm/pgtable-generic.o

amiga is the interesting one. It is the only config where pmd_val() itself
changed, and it comes out bit-for-bit identical, which also says the build
is reproducible enough for this comparison to mean anything. sun3 and
ColdFire differ because pmd_none() stops taking the address of its
argument, and the direction is an improvement. In mm/vmalloc.o on sun3 one
function loses a forced spill:

   -  subql #4,%sp
   -  movel %d0,%sp@(12)     /* spill the pmd value */
   -  lea %sp@(12),%a1       /* &(pmd) */
   -  moveal %a1@,%a0        /* read it back */

and turns into a 13 instruction function with no stack slot at all.
Instruction counts are otherwise flat (sun3 mm/memory.o 7373 -> 7373,
mm/vmalloc.o 5514 -> 5515), so it is not a size regression.

Dragging the cleanup along with the fix sounds right to me. It is a hard
prerequisite, and sent on its own it would just look like a cleanup with no
urgency. I assume it goes in ahead of 6ab55cfd in core-7.5.misc, so that
linux-next never sees a broken m68k in between.

One red herring in case anyone else pokes at this: m5208evb with its stock
defconfig (CONFIG_MMU not set) fails to build mm/memory.o for reasons that
have nothing to do with any of this - implicit declarations of
pte_is_uffd_wp_marker() and pmd_pgtable(), plus a parse error in
include/linux/mm.h. Pre-existing, and it goes away once MMU is on.

Best,
Hui



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-06  3:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-04  9:10 [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around() Hui Zhu
2026-11-05 10:12 ` David Hildenbrand
2026-11-05 12:00 ` David Hildenbrand (Arm)
2026-11-06  3:03   ` Hui Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®