From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-03 16:44:45
Since forever, MM code has thrown pte_t * around with no concern for const
safety, or typesafety of any kind. This is confusing. Attempt to address it
by:
1) Making sure pte_get*() helpers can cope with const pte_t * arguments
2) Constifying the pte_offset_map_ro_nolock() return type, which by definition
already pledges that users will not write to it.
These two simple steps were already able to uncover code smell from
khugepaged + do_swap_page().
Separate steps could include introducing pte_offset_map_ro_lock() for more
widespread usage of this.
Benefits of this include less confusion and better type-safety. It could also
futurely aid in efforts such as [0] which may want semantic annotation of these
accesses.
Based on mm-unstable and compile-tested on a handful of architectures.
No functional changes intended.
[CC list editorially trimmed for brevity reasons; apologies if you're not on it]
Link: https://lore.kernel.org/linux-mm/20260526-kpkeys-v8-0-eaaacdacc67c@arm.com/#t [0]
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: "Liam R. Howlett" <liam@infradead.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Usama Arif <usama.arif@linux.dev>
Cc: Kevin Brodsky <redacted>
Cc: Muhammad Usama Anjum <redacted>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org
v2:
- Small fixups on the arm64 side
- Re-order patches in a way such that bisection is preserved
- Pick up Helge's patch dropping parisc ptep_get()
- Constify s390's ptep_get() as well
Helge Deller (1):
parisc: Drop own implementations for ptep_get() and
ptep_test_and_clear_young()
Pedro Falcato (5):
mm/arm64: constify pte_get*() and contpte get logic
mm/powerpc/8xx: constify ptep_get() argument
mm/s390: constify ptep_get() argument
mm: constify generic pte_get*()
mm: constify the pte_offset_map_ro_nolock() return value
arch/arm64/include/asm/pgtable.h | 10 +++++-----
arch/arm64/mm/contpte.c | 11 ++++++++---
arch/parisc/include/asm/pgtable.h | 20 --------------------
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 2 +-
arch/powerpc/mm/pgtable.c | 2 +-
arch/s390/include/asm/pgtable.h | 2 +-
include/linux/mm.h | 4 ++--
include/linux/pgtable.h | 8 ++++----
mm/filemap.c | 2 +-
mm/khugepaged.c | 2 +-
mm/pgtable-generic.c | 4 ++--
11 files changed, 26 insertions(+), 41 deletions(-)
--
2.55.0
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-03 16:44:29
There is no need for write access to the PTE.
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-03 16:44:35
None of the helpers need write access to the PTE. Constifying the param
allows for const typesafety.
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
include/linux/pgtable.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-03 16:44:40
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
Suggested-by: Usama Arif <usama.arif@linux.dev>
Suggested-by: John David Anglin <redacted>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
arch/parisc/include/asm/pgtable.h | 20 --------------------
1 file changed, 20 deletions(-)
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-03 16:44:44
Constify the pte_t * retval from pte_offset_map_ro_nolock(), for which it is
already pledged that accesses must be read-only. With it, convert the three
treewide users to use const pte_t *.
khugepaged passes the result right down to fault code (do_swap_page()). This
leads to a complicated set of conditions that, in order to be correct, must
not install anything into *vmf->pte. This is not trivial to work around in
fault code, and as such just trivially cast to non-const pte_t* in the
meantime.
The other users are far more trivial and the conversion is equally
trivially simple.
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
arch/powerpc/mm/pgtable.c | 2 +-
include/linux/mm.h | 4 ++--
include/linux/pgtable.h | 2 +-
mm/filemap.c | 2 +-
mm/khugepaged.c | 2 +-
mm/pgtable-generic.c | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
From: Muhammad Usama Anjum <hidden> Date: 2026-08-03 18:39:10
On 03/08/2026 5:43 pm, Pedro Falcato wrote:
Since forever, MM code has thrown pte_t * around with no concern for const
safety, or typesafety of any kind. This is confusing. Attempt to address it
by:
1) Making sure pte_get*() helpers can cope with const pte_t * arguments
2) Constifying the pte_offset_map_ro_nolock() return type, which by definition
already pledges that users will not write to it.
These two simple steps were already able to uncover code smell from
khugepaged + do_swap_page().
Separate steps could include introducing pte_offset_map_ro_lock() for more
widespread usage of this.
Benefits of this include less confusion and better type-safety. It could also
futurely aid in efforts such as [0] which may want semantic annotation of these
accesses.
Based on mm-unstable and compile-tested on a handful of architectures.
No functional changes intended.
I've reviewed the entire series. Hence:
Reviewed-by: Muhammad Usama Anjum <redacted>
[CC list editorially trimmed for brevity reasons; apologies if you're not on it]
Link: https://lore.kernel.org/linux-mm/20260526-kpkeys-v8-0-eaaacdacc67c@arm.com/#t [0]
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: "Liam R. Howlett" <liam@infradead.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Usama Arif <usama.arif@linux.dev>
Cc: Kevin Brodsky <redacted>
Cc: Muhammad Usama Anjum <redacted>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org
v2:
- Small fixups on the arm64 side
- Re-order patches in a way such that bisection is preserved
- Pick up Helge's patch dropping parisc ptep_get()
- Constify s390's ptep_get() as well
Helge Deller (1):
parisc: Drop own implementations for ptep_get() and
ptep_test_and_clear_young()
Pedro Falcato (5):
mm/arm64: constify pte_get*() and contpte get logic
mm/powerpc/8xx: constify ptep_get() argument
mm/s390: constify ptep_get() argument
mm: constify generic pte_get*()
mm: constify the pte_offset_map_ro_nolock() return value
arch/arm64/include/asm/pgtable.h | 10 +++++-----
arch/arm64/mm/contpte.c | 11 ++++++++---
arch/parisc/include/asm/pgtable.h | 20 --------------------
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 2 +-
arch/powerpc/mm/pgtable.c | 2 +-
arch/s390/include/asm/pgtable.h | 2 +-
include/linux/mm.h | 4 ++--
include/linux/pgtable.h | 8 ++++----
mm/filemap.c | 2 +-
mm/khugepaged.c | 2 +-
mm/pgtable-generic.c | 4 ++--
11 files changed, 26 insertions(+), 41 deletions(-)
Since forever, MM code has thrown pte_t * around with no concern for const
safety, or typesafety of any kind. This is confusing. Attempt to address it
by:
What do you mean by "typesafety of any kind" ?
On powerpc64, pte_t is a struct so you can't play-up too much with it.
On powerpc32, pte_t is a long int because having it as a struct is counter-performant, but we have it as a struct when __CHECKER__ is defined, ie when doing a sparse check with 'make C=2'.
1) Making sure pte_get*() helpers can cope with const pte_t * arguments
2) Constifying the pte_offset_map_ro_nolock() return type, which by definition
already pledges that users will not write to it.
These two simple steps were already able to uncover code smell from
khugepaged + do_swap_page().
Separate steps could include introducing pte_offset_map_ro_lock() for more
widespread usage of this.
Benefits of this include less confusion and better type-safety. It could also
futurely aid in efforts such as [0] which may want semantic annotation of these
accesses.
Based on mm-unstable and compile-tested on a handful of architectures.
No functional changes intended.
For the series,
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
[CC list editorially trimmed for brevity reasons; apologies if you're not on it]
Link: https://lore.kernel.org/linux-mm/20260526-kpkeys-v8-0-eaaacdacc67c@arm.com/#t [0]
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: "Liam R. Howlett" <liam@infradead.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Usama Arif <usama.arif@linux.dev>
Cc: Kevin Brodsky <redacted>
Cc: Muhammad Usama Anjum <redacted>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org
v2:
- Small fixups on the arm64 side
- Re-order patches in a way such that bisection is preserved
- Pick up Helge's patch dropping parisc ptep_get()
- Constify s390's ptep_get() as well
Helge Deller (1):
parisc: Drop own implementations for ptep_get() and
ptep_test_and_clear_young()
Pedro Falcato (5):
mm/arm64: constify pte_get*() and contpte get logic
mm/powerpc/8xx: constify ptep_get() argument
mm/s390: constify ptep_get() argument
mm: constify generic pte_get*()
mm: constify the pte_offset_map_ro_nolock() return value
arch/arm64/include/asm/pgtable.h | 10 +++++-----
arch/arm64/mm/contpte.c | 11 ++++++++---
arch/parisc/include/asm/pgtable.h | 20 --------------------
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 2 +-
arch/powerpc/mm/pgtable.c | 2 +-
arch/s390/include/asm/pgtable.h | 2 +-
include/linux/mm.h | 4 ++--
include/linux/pgtable.h | 8 ++++----
mm/filemap.c | 2 +-
mm/khugepaged.c | 2 +-
mm/pgtable-generic.c | 4 ++--
11 files changed, 26 insertions(+), 41 deletions(-)
On Mon, Aug 03, 2026 at 05:43:55PM +0100, Pedro Falcato wrote:
None of the contpte code needs write access to the PTEs.
This seems like a very broad statement? Is that actually true? I see a bunch of
ptep's that aren't const-ified, so you should explain why those couldn't be
converted.
Also you add a new contpte_align_down() macro, you should mention that it the
commit message, explain why it was needed.
In general more needed here :) it'd be ok if it was a truly trivial change that
was all obvious but you're changing some pte_t *'s and not others so it's
clearly not.
This is (very) nitty but - not sure on the policy on extern's (Will/Catalin?) -
but in mm we drop them when we touch the code since you don't need them these
days :)
quoted hunk
extern void contpte_set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned int nr);
extern void contpte_clear_full_ptes(struct mm_struct *mm, unsigned long addr,
I really hate these _Generic() helper things. So ugly. And it's a pretty horrid
cast now :(
Was it not possible to const-ify further to just be able to constify
contpte_align_down itself?
It also seems to contradict the claim that contpte doesn't need write-access to
pte's since you're going to lengths to allow non-const pte_t * here.
You should cover off why this was necessary in the commit message as above.
Anyway PTR_ALIGN_DOWN() is already const-safe so couldn't you anyway just
collapse this to:
#define contpte_align_down(ptep) \
PTR_ALIGN_DOWN(ptep, sizeof(*(ptep)) * CONT_PTES)
Then describe in the commit message why you need to handle both cases?
quoted hunk
+ static inline pte_t *contpte_align_addr_ptep(unsigned long *start, unsigned long *end, pte_t *ptep, unsigned int nr)
@@ -310,7 +315,7 @@ void __contpte_try_unfold(struct mm_struct *mm, unsigned long addr, } EXPORT_SYMBOL_GPL(__contpte_try_unfold);-pte_t contpte_ptep_get(pte_t *ptep, pte_t orig_pte)+pte_t contpte_ptep_get(const pte_t *ptep, pte_t orig_pte) { /* * Gather access/dirty bits, which may be populated in any of the ptes
@@ -367,7 +372,7 @@ static inline bool contpte_is_consistent(pte_t pte, unsigned long pfn, pgprot_val(prot) == pgprot_val(orig_prot); }-pte_t contpte_ptep_get_lockless(pte_t *orig_ptep)+pte_t contpte_ptep_get_lockless(const pte_t *orig_ptep) { /* * The ptep_get_lockless() API requires us to read and return *orig_ptep
@@ -386,10 +391,10 @@ pte_t contpte_ptep_get_lockless(pte_t *orig_ptep) * because it is not part of a contpte range. */+ const pte_t *ptep;
Nit but this is breaking the reverse xmas tree isn't it?
pgprot_t orig_prot;
unsigned long pfn;
pte_t orig_pte;
- pte_t *ptep;
pte_t pte;
int i;
--
2.55.0
On Mon, Aug 03, 2026 at 05:43:56PM +0100, Pedro Falcato wrote:
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
You sure do like succinct commit messages :)
Maybe worth saying by dropping the __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG and
ptep_get defines you get the generic versions from include/pgtable.h which are
functionally identical.
(Being pedantic, they're not quite strictly identical as the
ptep_test_and_clear_young() generic function does some weird unnecessary
indirection with a local variable and the single {} is dropped etc.)
Suggested-by: Usama Arif <usama.arif@linux.dev>
Suggested-by: John David Anglin <redacted>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
Nits above notwithstanding, LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
On Mon, Aug 03, 2026 at 05:44:00PM +0100, Pedro Falcato wrote:
Constify the pte_t * retval from pte_offset_map_ro_nolock(), for which it is
already pledged that accesses must be read-only. With it, convert the three
treewide users to use const pte_t *.
khugepaged passes the result right down to fault code (do_swap_page()). This
leads to a complicated set of conditions that, in order to be correct, must
not install anything into *vmf->pte. This is not trivial to work around in
fault code, and as such just trivially cast to non-const pte_t* in the
meantime.
The other users are far more trivial and the conversion is equally
trivially simple.
Ah finally more words! :)
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
With comment updated as below and nits addressed, LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
I was going to question this based on whether the contract holds for
CONFIG_HIGHPTE but actually:
#define pte_unmap(pte) do { \
kunmap_local((pte)); \
rcu_read_unlock(); \
} while (0)
#define kunmap_local(__addr) \
do { \
BUILD_BUG_ON(__same_type((__addr), struct page *)); \
__kunmap_local(__addr); \
} while (0)
static inline void __kunmap_local(const void *vaddr) <-- const!
{
kunmap_local_indexed(vaddr);
}
So nice (CONFIG_HIGHPTE is going to go away at some point though, right? I
hope... :)
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-04 12:31:18
On Tue, Aug 04, 2026 at 11:55:09AM +0100, Lorenzo Stoakes (ARM) wrote:
On Mon, Aug 03, 2026 at 05:43:55PM +0100, Pedro Falcato wrote:
quoted
None of the contpte code needs write access to the PTEs.
This seems like a very broad statement? Is that actually true? I see a bunch of
ptep's that aren't const-ified, so you should explain why those couldn't be
converted.
ACK. FTR, I think it would've been far clearer with "None of the contpte get
code". There is of course contpte code that needs write access (e.g
contpte_clear_full_ptes).
Also you add a new contpte_align_down() macro, you should mention that it the
commit message, explain why it was needed.
In general more needed here :) it'd be ok if it was a truly trivial change that
was all obvious but you're changing some pte_t *'s and not others so it's
clearly not.
This is (very) nitty but - not sure on the policy on extern's (Will/Catalin?) -
but in mm we drop them when we touch the code since you don't need them these
days :)
*nods*. For what it's worth, this is new code that never needed extern.
quoted
extern void contpte_set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned int nr);
extern void contpte_clear_full_ptes(struct mm_struct *mm, unsigned long addr,
I really hate these _Generic() helper things. So ugly. And it's a pretty horrid
cast now :(
Me too!
Was it not possible to const-ify further to just be able to constify
contpte_align_down itself?
You can't do that because some callers want a pte_t* out of align_down,
others want a const pte_t* out of align_down, depending on the param.
In A More Civilized Language(TM):
template <typename T>
T contpte_align_down(T ptr);
:P
It also seems to contradict the claim that contpte doesn't need write-access to
pte's since you're going to lengths to allow non-const pte_t * here.
Yep, I'll admit the commit message is confusing and crap. I'll flesh it out
here.
You should cover off why this was necessary in the commit message as above.
Anyway PTR_ALIGN_DOWN() is already const-safe so couldn't you anyway just
collapse this to:
#define contpte_align_down(ptep) \
PTR_ALIGN_DOWN(ptep, sizeof(*(ptep)) * CONT_PTES)
Then describe in the commit message why you need to handle both cases?
quoted
+ static inline pte_t *contpte_align_addr_ptep(unsigned long *start, unsigned long *end, pte_t *ptep, unsigned int nr)
@@ -310,7 +315,7 @@ void __contpte_try_unfold(struct mm_struct *mm, unsigned long addr, } EXPORT_SYMBOL_GPL(__contpte_try_unfold);-pte_t contpte_ptep_get(pte_t *ptep, pte_t orig_pte)+pte_t contpte_ptep_get(const pte_t *ptep, pte_t orig_pte) { /* * Gather access/dirty bits, which may be populated in any of the ptes
@@ -367,7 +372,7 @@ static inline bool contpte_is_consistent(pte_t pte, unsigned long pfn, pgprot_val(prot) == pgprot_val(orig_prot); }-pte_t contpte_ptep_get_lockless(pte_t *orig_ptep)+pte_t contpte_ptep_get_lockless(const pte_t *orig_ptep) { /* * The ptep_get_lockless() API requires us to read and return *orig_ptep
@@ -386,10 +391,10 @@ pte_t contpte_ptep_get_lockless(pte_t *orig_ptep) * because it is not part of a contpte range. */+ const pte_t *ptep;
Nit but this is breaking the reverse xmas tree isn't it?
Yep, seems like I mistakenly broke the coding style here and in the
contpte_align_down macro above (the \ is misaligned). I'll fix it up.
(I think Andrew isn't taking more material for next cycle, and while
this should have no functional effect, it is very late and you have
pushback, so probably no rush here...)
--
Pedro
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-04 12:34:54
On Tue, Aug 04, 2026 at 12:11:08PM +0100, Lorenzo Stoakes (ARM) wrote:
On Mon, Aug 03, 2026 at 05:43:56PM +0100, Pedro Falcato wrote:
quoted
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
You sure do like succinct commit messages :)
I didn't even write this one! See the From: :))
Maybe worth saying by dropping the __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG and
ptep_get defines you get the generic versions from include/pgtable.h which are
functionally identical.
I can touch it up though, if you insist.
(Being pedantic, they're not quite strictly identical as the
ptep_test_and_clear_young() generic function does some weird unnecessary
indirection with a local variable and the single {} is dropped etc.)
quoted
Suggested-by: Usama Arif <usama.arif@linux.dev>
Suggested-by: John David Anglin <redacted>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
Nits above notwithstanding, LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
On Tue, Aug 04, 2026 at 01:31:04PM +0100, Pedro Falcato wrote:
On Tue, Aug 04, 2026 at 11:55:09AM +0100, Lorenzo Stoakes (ARM) wrote:
quoted
On Mon, Aug 03, 2026 at 05:43:55PM +0100, Pedro Falcato wrote:
quoted
None of the contpte code needs write access to the PTEs.
This seems like a very broad statement? Is that actually true? I see a bunch of
ptep's that aren't const-ified, so you should explain why those couldn't be
converted.
ACK. FTR, I think it would've been far clearer with "None of the contpte get
code". There is of course contpte code that needs write access (e.g
contpte_clear_full_ptes).
yeah that sounds better. Maybe just something like 'specify const for read-only
users of pte *' or similar?
quoted
Also you add a new contpte_align_down() macro, you should mention that it the
commit message, explain why it was needed.
In general more needed here :) it'd be ok if it was a truly trivial change that
was all obvious but you're changing some pte_t *'s and not others so it's
clearly not.
This is (very) nitty but - not sure on the policy on extern's (Will/Catalin?) -
but in mm we drop them when we touch the code since you don't need them these
days :)
*nods*. For what it's worth, this is new code that never needed extern.
Yup, this isn't a big deal anyway :)
quoted
quoted
extern void contpte_set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned int nr);
extern void contpte_clear_full_ptes(struct mm_struct *mm, unsigned long addr,
I really hate these _Generic() helper things. So ugly. And it's a pretty horrid
cast now :(
Me too!
quoted
Was it not possible to const-ify further to just be able to constify
contpte_align_down itself?
You can't do that because some callers want a pte_t* out of align_down,
others want a const pte_t* out of align_down, depending on the param.
In A More Civilized Language(TM):
template <typename T>
T contpte_align_down(T ptr);
Now do it in rust ;)
:P
Well as per below you can just avoid the whole issue with:
#define contpte_align_down(ptep) \
PTR_ALIGN_DOWN(ptep, sizeof(*(ptep)) * CONT_PTES)
And avoid this mess? You're ultimately calling into a macro anyway.
The generic thing Seems like a lot of hassle for an align-down, and then you're
forcing a macro and nasty casts anyway so I don't really see the downside of
just making it a macro in general?
quoted
It also seems to contradict the claim that contpte doesn't need write-access to
pte's since you're going to lengths to allow non-const pte_t * here.
Yep, I'll admit the commit message is confusing and crap. I'll flesh it out
here.
Thanks!
quoted
You should cover off why this was necessary in the commit message as above.
Anyway PTR_ALIGN_DOWN() is already const-safe so couldn't you anyway just
collapse this to:
#define contpte_align_down(ptep) \
PTR_ALIGN_DOWN(ptep, sizeof(*(ptep)) * CONT_PTES)
Then describe in the commit message why you need to handle both cases?
quoted
+ static inline pte_t *contpte_align_addr_ptep(unsigned long *start, unsigned long *end, pte_t *ptep, unsigned int nr)
@@ -310,7 +315,7 @@ void __contpte_try_unfold(struct mm_struct *mm, unsigned long addr, } EXPORT_SYMBOL_GPL(__contpte_try_unfold);-pte_t contpte_ptep_get(pte_t *ptep, pte_t orig_pte)+pte_t contpte_ptep_get(const pte_t *ptep, pte_t orig_pte) { /* * Gather access/dirty bits, which may be populated in any of the ptes
@@ -367,7 +372,7 @@ static inline bool contpte_is_consistent(pte_t pte, unsigned long pfn, pgprot_val(prot) == pgprot_val(orig_prot); }-pte_t contpte_ptep_get_lockless(pte_t *orig_ptep)+pte_t contpte_ptep_get_lockless(const pte_t *orig_ptep) { /* * The ptep_get_lockless() API requires us to read and return *orig_ptep
@@ -386,10 +391,10 @@ pte_t contpte_ptep_get_lockless(pte_t *orig_ptep) * because it is not part of a contpte range. */+ const pte_t *ptep;
Nit but this is breaking the reverse xmas tree isn't it?
Yep, seems like I mistakenly broke the coding style here and in the
contpte_align_down macro above (the \ is misaligned). I'll fix it up.
(I think Andrew isn't taking more material for next cycle, and while
this should have no functional effect, it is very late and you have
pushback, so probably no rush here...)
Yeah I assumed this was for 7.4 :) I mean the series is fine for 7.3 too AFAIC
(with feedback addressed obviously).
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
This is PPC code, so I don't know if they have any particular opinion here,
but I could definitely do this.
(FWIW, it seems we're more aggressive in MM in doing this than other places
in the kernel?)
--
Pedro
On Tue, Aug 04, 2026 at 01:34:41PM +0100, Pedro Falcato wrote:
On Tue, Aug 04, 2026 at 12:11:08PM +0100, Lorenzo Stoakes (ARM) wrote:
quoted
On Mon, Aug 03, 2026 at 05:43:56PM +0100, Pedro Falcato wrote:
quoted
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
You sure do like succinct commit messages :)
I didn't even write this one! See the From: :))
Lol well then feedback addressed to Helge ;)
quoted
Maybe worth saying by dropping the __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG and
ptep_get defines you get the generic versions from include/pgtable.h which are
functionally identical.
I can touch it up though, if you insist.
Yeah, I'm being pedantic here it's not vital to have this change :) but be nice
if you could, I'm sure Helge probably wouldn't mind too much? :)
quoted
(Being pedantic, they're not quite strictly identical as the
ptep_test_and_clear_young() generic function does some weird unnecessary
indirection with a local variable and the single {} is dropped etc.)
quoted
Suggested-by: Usama Arif <usama.arif@linux.dev>
Suggested-by: John David Anglin <redacted>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
Nits above notwithstanding, LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
This is PPC code, so I don't know if they have any particular opinion here,
but I could definitely do this.
(FWIW, it seems we're more aggressive in MM in doing this than other places
in the kernel?)
I mean I can't see why there'd be any objection given you're already const-ing
here :)
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
Any suggestion welcome.
powerpc 8xx page table is independant on page size. When you use 16k pages, depending on the address you hit the page for the first time, the HW assist page table walk will fetch one of four 4k cells in page table that need to be identical as they all four define the same 16k page. Not sure I'm clear.
Christophe
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
Any suggestion welcome.
I'm being super nitty, all I mean is:
- pte_basic_t val = READ_ONCE(ptep->pte);
- pte_t pte = {val, val, val, val};
+ const pte_basic_t val = READ_ONCE(ptep->pte);
+ const pte_t pte = {val, val, val, val};
:)
powerpc 8xx page table is independant on page size. When you use 16k pages,
depending on the address you hit the page for the first time, the HW assist
page table walk will fetch one of four 4k cells in page table that need to
be identical as they all four define the same 16k page. Not sure I'm clear.
No that's clear, thanks!
(I say 'horrifying' because I am looking into RCU page table freeing which this
may complicate, though perhaps not in practice, to be continued :)
On Tue, Aug 04, 2026 at 01:34:41PM +0100, Pedro Falcato wrote:
quoted
On Tue, Aug 04, 2026 at 12:11:08PM +0100, Lorenzo Stoakes (ARM) wrote:
quoted
On Mon, Aug 03, 2026 at 05:43:56PM +0100, Pedro Falcato wrote:
quoted
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
You sure do like succinct commit messages :)
I didn't even write this one! See the From: :))
Lol well then feedback addressed to Helge ;)
Noted :-)
quoted
quoted
Maybe worth saying by dropping the __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG and
ptep_get defines you get the generic versions from include/pgtable.h which are
functionally identical.
Isn't that basically the same as:
"Switch to the generic implementations, which are identical."
;-)
quoted
I can touch it up though, if you insist.
Yeah, I'm being pedantic here it's not vital to have this change :) but be nice
if you could, I'm sure Helge probably wouldn't mind too much? :)
Yes, I'm fine with any cleanup/rephrasing of the commit message.
Thanks!
Helge
quoted
quoted
(Being pedantic, they're not quite strictly identical as the
ptep_test_and_clear_young() generic function does some weird unnecessary
indirection with a local variable and the single {} is dropped etc.)
On Tue, Aug 04, 2026 at 03:00:43PM +0200, Helge Deller wrote:
On 8/4/26 14:42, Lorenzo Stoakes (ARM) wrote:
quoted
On Tue, Aug 04, 2026 at 01:34:41PM +0100, Pedro Falcato wrote:
quoted
On Tue, Aug 04, 2026 at 12:11:08PM +0100, Lorenzo Stoakes (ARM) wrote:
quoted
On Mon, Aug 03, 2026 at 05:43:56PM +0100, Pedro Falcato wrote:
quoted
From: Helge Deller <deller@gmx.de>
Switch to the generic implementations, which are identical.
You sure do like succinct commit messages :)
I didn't even write this one! See the From: :))
Lol well then feedback addressed to Helge ;)
Noted :-)
quoted
quoted
quoted
Maybe worth saying by dropping the __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG and
ptep_get defines you get the generic versions from include/pgtable.h which are
functionally identical.
Isn't that basically the same as:
"Switch to the generic implementations, which are identical."
;-)
I am being _exceedingly_, possibly outrageously pedantic here :>)
quoted
quoted
I can touch it up though, if you insist.
Yeah, I'm being pedantic here it's not vital to have this change :) but be nice
if you could, I'm sure Helge probably wouldn't mind too much? :)
Yes, I'm fine with any cleanup/rephrasing of the commit message.
Thanks :)
Thanks!
Helge
quoted
quoted
quoted
(Being pedantic, they're not quite strictly identical as the
ptep_test_and_clear_young() generic function does some weird unnecessary
indirection with a local variable and the single {} is dropped etc.)
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
Any suggestion welcome.
I'm being super nitty, all I mean is:
- pte_basic_t val = READ_ONCE(ptep->pte);
- pte_t pte = {val, val, val, val};
+ const pte_basic_t val = READ_ONCE(ptep->pte);
+ const pte_t pte = {val, val, val, val};
:)
I'm fine with that, I was reacting on the "horrifying".
quoted
powerpc 8xx page table is independant on page size. When you use 16k pages,
depending on the address you hit the page for the first time, the HW assist
page table walk will fetch one of four 4k cells in page table that need to
be identical as they all four define the same 16k page. Not sure I'm clear.
No that's clear, thanks!
(I say 'horrifying' because I am looking into RCU page table freeing which this
may complicate, though perhaps not in practice, to be continued :)
A few more details here if needed: 55c8fc3f4930 ("powerpc/8xx:
reintroduce 16K pages with HW assistance")
Christophe
Worth making these const too (that {val, val, val, val} horrifies me btw :)?
Any suggestion welcome.
I'm being super nitty, all I mean is:
- pte_basic_t val = READ_ONCE(ptep->pte);
- pte_t pte = {val, val, val, val};
+ const pte_basic_t val = READ_ONCE(ptep->pte);
+ const pte_t pte = {val, val, val, val};
:)
I'm fine with that, I was reacting on the "horrifying".
quoted
powerpc 8xx page table is independant on page size. When you use 16k pages,
depending on the address you hit the page for the first time, the HW assist
page table walk will fetch one of four 4k cells in page table that need to
be identical as they all four define the same 16k page. Not sure I'm clear.
No that's clear, thanks!
(I say 'horrifying' because I am looking into RCU page table freeing which this
may complicate, though perhaps not in practice, to be continued :)
A few more details here if needed: 55c8fc3f4930 ("powerpc/8xx: reintroduce 16K pages with HW assistance")
Christophe
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-04 19:23:36
On Tue, Aug 04, 2026 at 12:22:19PM +0100, Lorenzo Stoakes (ARM) wrote:
On Mon, Aug 03, 2026 at 05:44:00PM +0100, Pedro Falcato wrote:
quoted
Constify the pte_t * retval from pte_offset_map_ro_nolock(), for which it is
already pledged that accesses must be read-only. With it, convert the three
treewide users to use const pte_t *.
khugepaged passes the result right down to fault code (do_swap_page()). This
leads to a complicated set of conditions that, in order to be correct, must
not install anything into *vmf->pte. This is not trivial to work around in
fault code, and as such just trivially cast to non-const pte_t* in the
meantime.
The other users are far more trivial and the conversion is equally
trivially simple.
Ah finally more words! :)
:)
quoted
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
With comment updated as below and nits addressed, LGTM so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
I was going to question this based on whether the contract holds for
CONFIG_HIGHPTE but actually:
#define pte_unmap(pte) do { \
kunmap_local((pte)); \
rcu_read_unlock(); \
} while (0)
#define kunmap_local(__addr) \
do { \
BUILD_BUG_ON(__same_type((__addr), struct page *)); \
__kunmap_local(__addr); \
} while (0)
static inline void __kunmap_local(const void *vaddr) <-- const!
{
kunmap_local_indexed(vaddr);
}
So nice (CONFIG_HIGHPTE is going to go away at some point though, right? I
hope... :)
I was going to say "yes but then pmdp_get() also needs to be constfified" but
actually no, it can't:
pte_t *__pte_offset_map(pmd_t *pmd, unsigned long addr, pmd_t *pmdvalp) {
if (unlikely(pmd_bad(pmdval))) {
pmd_clear_bad(pmd);
goto nomap;
}
}
so PTE mapping actually needs to write to the pmdp if the pmd looks bad.
Tricky stuff :)
--
Pedro
I was going to say "yes but then pmdp_get() also needs to be constfified" but
actually no, it can't:
pte_t *__pte_offset_map(pmd_t *pmd, unsigned long addr, pmd_t *pmdvalp) {
if (unlikely(pmd_bad(pmdval))) {
pmd_clear_bad(pmd);
goto nomap;
}
}
so PTE mapping actually needs to write to the pmdp if the pmd looks bad.
Tricky stuff :)
But if pmd is const, can it be bad at all ?
Christophe
I was going to say "yes but then pmdp_get() also needs to be constfified" but
actually no, it can't:
pte_t *__pte_offset_map(pmd_t *pmd, unsigned long addr, pmd_t *pmdvalp) {
if (unlikely(pmd_bad(pmdval))) {
pmd_clear_bad(pmd);
goto nomap;
}
}
so PTE mapping actually needs to write to the pmdp if the pmd looks bad.
Tricky stuff :)
But if pmd is const, can it be bad at all ?
Yes, you just need a stray write or a bit of memory corruption and it can
go bad. And then we need to do clear_bad() :)
(it's unclear to me whether this is actually common or useful enough these
days; the way this was explained to me, page tables can be best-effort
redundant; but it's not like we know clearing the whole range is ok, and
the way pmd_ERROR, etc work they don't even communicate to userspace what
happened, unlike normal hwpoison mechanisms)
--
Pedro
None of the helpers need write access to the PTE. Constifying the param
allows for const typesafety.
Signed-off-by: Pedro Falcato <pfalcato@suse.de>
---
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
On Mon, Aug 03, 2026 at 05:43:54PM +0100, Pedro Falcato wrote:
Since forever, MM code has thrown pte_t * around with no concern for const
safety, or typesafety of any kind. This is confusing. Attempt to address it
by:
1) Making sure pte_get*() helpers can cope with const pte_t * arguments
2) Constifying the pte_offset_map_ro_nolock() return type, which by definition
already pledges that users will not write to it.
These two simple steps were already able to uncover code smell from
khugepaged + do_swap_page().
Separate steps could include introducing pte_offset_map_ro_lock() for more
widespread usage of this.
Benefits of this include less confusion and better type-safety. It could also
futurely aid in efforts such as [0] which may want semantic annotation of these
accesses.
Based on mm-unstable and compile-tested on a handful of architectures.
No functional changes intended.
Even though the pte accesses should always be type-safe, this series does
not really address the problem completely and instead changes things only
for a small set of pte access sites. So just wondering how much beneficial
this series really is ?
[CC list editorially trimmed for brevity reasons; apologies if you're not on it]
Link: https://lore.kernel.org/linux-mm/20260526-kpkeys-v8-0-eaaacdacc67c@arm.com/#t [0]
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: "Liam R. Howlett" <liam@infradead.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Lance Yang <lance.yang@linux.dev>
Cc: Usama Arif <usama.arif@linux.dev>
Cc: Kevin Brodsky <redacted>
Cc: Muhammad Usama Anjum <redacted>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org
v2:
- Small fixups on the arm64 side
- Re-order patches in a way such that bisection is preserved
- Pick up Helge's patch dropping parisc ptep_get()
- Constify s390's ptep_get() as well
Helge Deller (1):
parisc: Drop own implementations for ptep_get() and
ptep_test_and_clear_young()
Pedro Falcato (5):
mm/arm64: constify pte_get*() and contpte get logic
mm/powerpc/8xx: constify ptep_get() argument
mm/s390: constify ptep_get() argument
mm: constify generic pte_get*()
mm: constify the pte_offset_map_ro_nolock() return value
arch/arm64/include/asm/pgtable.h | 10 +++++-----
arch/arm64/mm/contpte.c | 11 ++++++++---
arch/parisc/include/asm/pgtable.h | 20 --------------------
arch/powerpc/include/asm/nohash/32/pte-8xx.h | 2 +-
arch/powerpc/mm/pgtable.c | 2 +-
arch/s390/include/asm/pgtable.h | 2 +-
include/linux/mm.h | 4 ++--
include/linux/pgtable.h | 8 ++++----
mm/filemap.c | 2 +-
mm/khugepaged.c | 2 +-
mm/pgtable-generic.c | 4 ++--
11 files changed, 26 insertions(+), 41 deletions(-)
--
2.55.0
From: Pedro Falcato <pfalcato@suse.de> Date: 2026-08-05 12:40:07
On Wed, Aug 05, 2026 at 04:12:21PM +0530, Anshuman Khandual wrote:
On Mon, Aug 03, 2026 at 05:43:54PM +0100, Pedro Falcato wrote:
quoted
Since forever, MM code has thrown pte_t * around with no concern for const
safety, or typesafety of any kind. This is confusing. Attempt to address it
by:
1) Making sure pte_get*() helpers can cope with const pte_t * arguments
2) Constifying the pte_offset_map_ro_nolock() return type, which by definition
already pledges that users will not write to it.
These two simple steps were already able to uncover code smell from
khugepaged + do_swap_page().
Separate steps could include introducing pte_offset_map_ro_lock() for more
widespread usage of this.
Benefits of this include less confusion and better type-safety. It could also
futurely aid in efforts such as [0] which may want semantic annotation of these
accesses.
Based on mm-unstable and compile-tested on a handful of architectures.
No functional changes intended.
Even though the pte accesses should always be type-safe, this series does
not really address the problem completely and instead changes things only
for a small set of pte access sites. So just wondering how much beneficial
this series really is ?
This series is meant to be a small step in the right direction (while
feeling out what the community thinks, which seems to be receptive). The
obvious next steps would be to introduce some sort of
const pte_t *pte_offset_map_ro_lock(struct mm_struct *mm, pmd_t *pmd,
unsigned long addr, spinlock_t **ptlp);
and use it in more places. That will obviously involve a bit of churn.
--
Pedro