Re: [PATCH v4] mm/memory: use pmdp_get_lockless() for the pmd read in do_fault_around()
From: Hui Zhu
Date: Mon Nov 09 2026 - 00:51:07 EST
2026/11/6 20:08, Greg Ungerer:
Hi Greg,
Hi Hui,
On 6/11/26 13:03, Hui Zhu wrote:
2026/11/5 20:00, David Hildenbrand (Arm) :The m5475evb_defconfig is supposed to have MMU enabled:
On 11/4/26 10:10, Hui Zhu wrote:Hi David,
From: Hui Zhu <zhuhui@xxxxxxxxxx>Fun, this breaks some odd m68k configs because they do some legacy weird stuff with pmd_none().
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@xxxxxxxxxx # KCSAN annotation only, no functional change
Signed-off-by: Hui Zhu <zhuhui@xxxxxxxxxx>
Acked-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
---
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@xxxxxxxxx/
Shorten the comment to one line (David Hildenbrand).
v2:
https://lore.kernel.org/all/20261102093120.92654-1-hui.zhu@xxxxxxxxx/
Use pmdp_get_lockless() instead of pmdp_get() (David Hildenbrand).
v1:
https://lore.kernel.org/all/20261102031308.66177-1-hui.zhu@xxxxxxxxx/
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;
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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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; }
/*
Thanks for picking this up, and for the m68k cleanup!
Tested-by: Hui Zhu <zhuhui@xxxxxxxxxx>
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.
$ ARCH=m68k CROSS_COMPILE=m68k-linux-uclibc- make m5475evb_defconfig
HOSTCC scripts/basic/fixdep
[snip]
HOSTLD scripts/kconfig/conf
#
# configuration written to .config
#
$
$ grep CONFIG_MMU .config
CONFIG_MMU=y
CONFIG_MMU_COLDFIRE=y
CONFIG_MMU_GATHER_NO_RANGE=y
CONFIG_MMU_GATHER_MERGE_VMAS=y
CONFIG_MMU_LAZY_TLB_REFCOUNT=y
But something ends up disabling it when you compile.
I'll fix this.
I also checked whether your patch changes code generation, by building theYou should not be able to set MMU=y if configured for the M5208.
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.
It doesn't have an MMU. You want the 5475, 5485 or 54418 to test
valid MMU=y configurations.
Regards
Greg
You are right on both counts, and that mail of mine had three things
wrong.
1. "No m52xx/m54xx defconfig enables CONFIG_MMU" - wrong. I grepped the
defconfig fragments for CONFIG_MMU=y, but MMU is 'default y', so an
enabled MMU never appears in the fragment. m5475evb and m54418evb
both give CONFIG_MMU=y, as you show.
2. My "m5208evb + CONFIG_MMU=y" test config was not a valid config.
M520x is 'depends on !MMU', so forcing MMU on leaves the SoC choice
unsatisfiable and olddefconfig silently drops it:
before: # CONFIG_MMU is not set / CONFIG_M520x=y
after: CONFIG_MMU=y / CONFIG_MMU_COLDFIRE=y / CONFIG_M520x gone
Please disregard the m5208evb row in my codegen numbers.
3. The "red herring" I flagged was my own doing, not a pre-existing
problem. mm/Makefile has 'mmu-$(CONFIG_MMU) := ... memory.o', so a
NOMMU config never builds mm/memory.o at all; I asked for a target
that config does not have. Sorry for the noise on that one.
Retested with the configs you named (7.4-rc1 plus the do_fault_around
change plus David's cleanup, gcc-m68k-linux-gnu 13.3.0):
m5475evb_defconfig without David's patch: mm/memory.o fails at
mcf_pgtable.h:134 with it: full -j24 build ok, vmlinux 4215344
bytes, 0 warnings
m54418evb_defconfig without David's patch: same mcf_pgtable.h:134
failure
So the Tested-by I sent covers amiga, sun3 and m5475evb. It is
build-tested only - I have no m68k hardware here to boot.
On MMU being disabled when you compile: I cannot reproduce that. Here
m5475evb_defconfig gives CONFIG_MMU=y, olddefconfig reports no change,
two consecutive defconfig runs produce an identical .config, and a full
build finishes with CONFIG_MMU=y and CONFIG_M547x=y still set and
.config untouched. Two differences from your setup that might matter:
I am on a glibc toolchain rather than uclibc, and building out of tree
with O= rather than in tree.
One observation, which may well be unrelated. m5475evb_defconfig never
sets CONFIG_M547x - it only ends up with it because the "ColdFire SoC
type" choice defaults to M520x, which is unavailable under MMU=y, so
kconfig falls back to the first usable member. m54418evb_defconfig sets
CONFIG_M5441x=y explicitly. If that fallback is what breaks on your
side, making m5475evb explicit would sidestep it.
Best,
Hui