arm64: Restrict pagetable teardown to avoid false warning

PATCH Applied Dev Jain 5 participants 8 messages May 27, 2025 – Jun 12, 2025

Subsystems (2)

Stable branch arm64

lore

Share & tools

lore

Dev Jain

author

2025-05-27 08:26 UTC #0 lore raw

PATCH v3 1/1 arm64: Restrict pagetable teardown to avoid false warning

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().

This problem was found by code inspection.

Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())

Cc <[email protected]>

Reported-by Ryan Roberts <[email protected]>

Acked-by David Hildenbrand <[email protected]>

Signed-off-by Dev Jain <[email protected]>

---
This patch is based on 6.15-rc6.

v2->v3:
- Use pmdp_get()

v1->v2:
- Enforce check in caller

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

arch/arm64/mm/mmu.c +2 -1

index ea6695d53fb9..5a9bf291c649 100644
@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-		pmd_free_pte_page(pmdp, next);
+		if (pmd_present(pmdp_get(pmdp)))
+			pmd_free_pte_page(pmdp, next);
 	} while (pmdp++, next += PMD_SIZE, next != end);
 
 	pud_clear(pudp);

Signature · 1 line

2.30.2

Anshuman Khandual

2025-05-29 09:02 UTC #1 lore raw

Quoted 31 lines · Dev Jain

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())
Cc <[email protected]>
Reported-by Ryan Roberts <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
This patch is based on 6.15-rc6.
v2->v3:
- Use pmdp_get()
v1->v2:
- Enforce check in caller
 arch/arm64/mm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +0 -0
index ea6695d53fb9..5a9bf291c649 100644

arch/arm64/mm/mmu.c int pud_free_pmd_page(pud_t *pudp, unsigned long addr) · line 1289

Quoted 5 lines

arch/arm64/mm/mmu.c +1 -1

@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-		pmd_free_pte_page(pmdp, next);
+		if (pmd_present(pmdp_get(pmdp)))

This code path is only called for the kernel mapping. Hence should pmd_valid() be used instead of pmd_present() which also checks for present invalid scenarios as well ?

Quoted 4 lines

arch/arm64/mm/mmu.c +1 -0

+			pmd_free_pte_page(pmdp, next);
 	} while (pmdp++, next += PMD_SIZE, next != end);
 
 	pud_clear(pudp);

Ryan Roberts

Reviewed-by

2025-05-29 09:13 UTC #2 lore raw

Quoted 10 lines · Anshuman Khandual

Quoted 7 lines · Dev Jain
Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().

Quoted 7 lines

Quoted 7 lines
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())
Cc <[email protected]>
Reported-by Ryan Roberts <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Dev Jain <[email protected]>

LGTM!

Reviewed-by Ryan Roberts <[email protected]>

Quoted 12 lines

Quoted 12 lines
---
This patch is based on 6.15-rc6.
v2->v3:
- Use pmdp_get()
v1->v2:
- Enforce check in caller
 arch/arm64/mm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 14 lines

arch/arm64/mm/mmu.c int pud_free_pmd_page(pud_t *pudp, unsigned long addr) · line 1289
Quoted 10 lines
arch/arm64/mm/mmu.c +1 -1
index ea6695d53fb9..5a9bf291c649 100644
@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-		pmd_free_pte_page(pmdp, next);
+		if (pmd_present(pmdp_get(pmdp)))
This code path is only called for the kernel mapping. Hence should pmd_valid() be used instead of pmd_present() which also checks for present invalid scenarios as well ?

I think a similar question came up in a previous round, where we concluded that it's better to be consistent with what vmalloc is already doing. So personally I'd leave it as pmd_present():

	if (pmd_present(*pmd) && !pmd_free_pte_page(pmd, addr))
		return 0;

Quoted 5 lines

arch/arm64/mm/mmu.c +1 -0

+			pmd_free_pte_page(pmdp, next);
 	} while (pmdp++, next += PMD_SIZE, next != end);
 
 	pud_clear(pudp);

Anshuman Khandual

Reviewed-by on v3 1/1

2025-05-29 09:23 UTC #3 lore raw

In reply to Dev Jain

Quoted 31 lines · Dev Jain

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())
Cc <[email protected]>
Reported-by Ryan Roberts <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
This patch is based on 6.15-rc6.
v2->v3:
- Use pmdp_get()
v1->v2:
- Enforce check in caller
 arch/arm64/mm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +0 -0
index ea6695d53fb9..5a9bf291c649 100644

arch/arm64/mm/mmu.c int pud_free_pmd_page(pud_t *pudp, unsigned long addr) · line 1293

Quoted 9 lines

arch/arm64/mm/mmu.c +2 -1

@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	next = addr;
 	end = addr + PUD_SIZE;
 	do {
-		pmd_free_pte_page(pmdp, next);
+		if (pmd_present(pmdp_get(pmdp)))
+			pmd_free_pte_page(pmdp, next);
 	} while (pmdp++, next += PMD_SIZE, next != end);
 
 	pud_clear(pudp);

Agree with Ryan about keeping pmd_present() to be consistent.

Reviewed-by Anshuman Khandual <[email protected]>

Dev Jain

author

2025-05-30 03:56 UTC #4 lore raw

In reply to Dev Jain

Quoted 10 lines · Dev Jain

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())

I missed double quotes around the fixes commit message. Can Will or Catalin fix that, or shall I resend.

Quoted 30 lines

Cc <[email protected]>
Reported-by Ryan Roberts <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
This patch is based on 6.15-rc6.
v2->v3:
- Use pmdp_get()
v1->v2:
- Enforce check in caller
  arch/arm64/mm/mmu.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +2 -1
index ea6695d53fb9..5a9bf291c649 100644
@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
  	next = addr;
  	end = addr + PUD_SIZE;
  	do {
-		pmd_free_pte_page(pmdp, next);
+		if (pmd_present(pmdp_get(pmdp)))
+			pmd_free_pte_page(pmdp, next);
  	} while (pmdp++, next += PMD_SIZE, next != end);
 
  	pud_clear(pudp);

Ryan Roberts

2025-05-30 07:05 UTC #5 lore raw

Quoted 3 lines · Dev Jain

Quoted 1 line · Dev Jain
Commit 9c006972c3fe removes the pxd_present() checks because the caller

nit: "Commit 9c006972c3fe" should have actually been:

Commit 9c006972c3fe ("arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table()")

Quoted 6 lines

Quoted 6 lines
checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().

Quoted 8 lines

Quoted 4 lines
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from
pXd_free_pYd_table())
I missed double quotes around the fixes commit message. Can Will or Catalin fix that,
or shall I resend.

For future, I have the following in my ~/.gitconfig

"""
[pretty]

        fixes = Fixes: %h (\"%s\")
        commit = Commit %h (\"%s\")

"""

Then I can do:

$ git show --pretty=fixes <SHA> | head -n 1

$ git show --pretty=commit <SHA> | head -n 1

to get the correct format. Note that "Fixes:" is a tag and should all be on a single line. "Commit" is just a way to refer to other commits in prose and can be broken across lines at the usual character limit.

Perhaps there is an even easier way to do it, but this works for me.

Thanks,
Ryan

Quoted 30 lines

Quoted 30 lines
Cc <[email protected]>
Reported-by Ryan Roberts <[email protected]>
Acked-by David Hildenbrand <[email protected]>
Signed-off-by Dev Jain <[email protected]>
---
This patch is based on 6.15-rc6.
v2->v3:
- Use pmdp_get()
v1->v2:
- Enforce check in caller
arch/arm64/mm/mmu.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +2 -1
index ea6695d53fb9..5a9bf291c649 100644
@@ -1286,7 +1286,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
       next = addr;
       end = addr + PUD_SIZE;
       do {
-        pmd_free_pte_page(pmdp, next);
+        if (pmd_present(pmdp_get(pmdp)))
+            pmd_free_pte_page(pmdp, next);
       } while (pmdp++, next += PMD_SIZE, next != end);
         pud_clear(pudp);

Catalin Marinas

maintainer Reviewed-by on v3 1/1

2025-06-10 16:37 UTC #6 lore raw

In reply to Dev Jain

Quoted 11 lines · Dev Jain

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().
This problem was found by code inspection.
Fixes 9c006972c3fe (arm64: mmu: drop pXd_present() checks from pXd_free_pYd_table())
Cc <[email protected]>

Reviewed-by Catalin Marinas <[email protected]>

Will Deacon

maintainer Applied

2025-06-12 17:27 UTC #7 lore raw

In reply to Dev Jain

Quoted 8 lines · Dev Jain

Commit 9c006972c3fe removes the pxd_present() checks because the caller checks pxd_present(). But, in case of vmap_try_huge_pud(), the caller only checks pud_present(); pud_free_pmd_page() recurses on each pmd through pmd_free_pte_page(), wherein the pmd may be none. Thus it is possible to hit a warning in the latter, since pmd_none => !pmd_table(). Thus, add a pmd_present() check in pud_free_pmd_page().
[...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: Restrict pagetable teardown to avoid false warning

      https://git.kernel.org/arm64/c/650768c512fa

Cheers,

Signature · 5 lines

Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev