[PATCH 1/2] arm64/mm: Move __check_safe_pte_update()
Anshuman Khandual
anshuman.khandual at arm.com
Mon Sep 7 21:27:39 PDT 2026
On Mon, Sep 07, 2026 at 05:48:18PM +0200, David Hildenbrand (Arm) wrote:
> On 9/1/26 08:54, Anshuman Khandual wrote:
> > The page table entry print helpers and related macros which are defined in
> > <linux/pgtable.h> will not be accessible in platform <asm/pgtable.h> which
> > is basically caused by cycling dependency.
> >
> > Move __check_safe_pte_update() inside arch/arm64/mm/mmu.c as a preparation
> > for subsequent usage of the afore mentioned generic MM helpers.
> >
> > This does not cause any functional change.
> >
> > Cc: Catalin Marinas <catalin.marinas at arm.com>
> > Cc: Will Deacon <will at kernel.org>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Anshuman Khandual <anshuman.khandual at arm.com>
> > ---
> > Should __check_safe_pte_update() be wrapped in #ifdef CONFIG_DEBUG_VM
> > along with dropping off current IS_ENABLED() test. That would ensure
> > the function never gets called when CONFIG_DEBUG_VM is disabled. That
> > will preserve current behaviour as the inline function gets dropped
> > when CONFIG_DEBUG_VM remains disabled.
> >
> > arch/arm64/include/asm/pgtable.h | 47 +-------------------------------
> > arch/arm64/mm/mmu.c | 45 ++++++++++++++++++++++++++++++
> > 2 files changed, 46 insertions(+), 46 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index e89ec5f4787b..61fbfab3e06e 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -387,52 +387,7 @@ static inline pte_t __ptep_get(pte_t *ptep)
> > extern void __sync_icache_dcache(pte_t pteval);
> > bool pgattr_change_is_safe(pteval_t old, pteval_t new);
> >
> > -/*
> > - * PTE bits configuration in the presence of hardware Dirty Bit Management
> > - * (PTE_WRITE == PTE_DBM):
> > - *
> > - * Dirty Writable | PTE_RDONLY PTE_WRITE PTE_DIRTY (sw)
> > - * 0 0 | 1 0 0
> > - * 0 1 | 1 1 0
> > - * 1 0 | 1 0 1
> > - * 1 1 | 0 1 x
> > - *
> > - * When hardware DBM is not present, the software PTE_DIRTY bit is updated via
> > - * the page fault mechanism. Checking the dirty status of a pte becomes:
> > - *
> > - * PTE_DIRTY || (PTE_WRITE && !PTE_RDONLY)
> > - */
> > -
> > -static inline void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep,
> > - pte_t pte)
> > -{
> > - pte_t old_pte;
> > -
> > - if (!IS_ENABLED(CONFIG_DEBUG_VM))
> > - return;
> > -
> > - old_pte = __ptep_get(ptep);
> > -
> > - if (!pte_valid(old_pte) || !pte_valid(pte))
> > - return;
> > - if (mm != current->active_mm && atomic_read(&mm->mm_users) <= 1)
> > - return;
> > -
> > - /*
> > - * Check for potential race with hardware updates of the pte
> > - * (__ptep_set_access_flags safely changes valid ptes without going
> > - * through an invalid entry).
> > - */
> > - VM_WARN_ONCE(!pte_young(pte),
> > - "%s: racy access flag clearing: 0x%016llx -> 0x%016llx",
> > - __func__, pte_val(old_pte), pte_val(pte));
> > - VM_WARN_ONCE(pte_write(old_pte) && !pte_dirty(pte),
> > - "%s: racy dirty state clearing: 0x%016llx -> 0x%016llx",
> > - __func__, pte_val(old_pte), pte_val(pte));
> > - VM_WARN_ONCE(!pgattr_change_is_safe(pte_val(old_pte), pte_val(pte)),
> > - "%s: unsafe attribute change: 0x%016llx -> 0x%016llx",
> > - __func__, pte_val(old_pte), pte_val(pte));
> > -}
> > +void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte);
> >
>
> That implies that any __set_ptes_anysz() will invoke another function call that
> will not be optimized out.
Right.
>
> You should likely keep most of __check_safe_pte_update() in the header as an
> inline function (or at least the CONFIG_DEBUG_VM check), and only move the "slow
> path" code.
>
> Alternatively, make the whole thing #ifdef CONFIG_DEBUG_VM and provide an empty
> inline helper for !CONFIG_DEBUG_VM.
>
> That's probably the cleanest, because performance with CONFIG_DEBUG_VM is not
> really relevant.
Agreed, I had asked exact same question earlier as well.
> > Should __check_safe_pte_update() be wrapped in #ifdef CONFIG_DEBUG_VM
> > along with dropping off current IS_ENABLED() test. That would ensure
> > the function never gets called when CONFIG_DEBUG_VM is disabled. That
> > will preserve current behaviour as the inline function gets dropped
> > when CONFIG_DEBUG_VM remains disabled.
Will fold in the following changes.
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 61fbfab3e06e..281a512f1fc6 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -387,7 +387,13 @@ static inline pte_t __ptep_get(pte_t *ptep)
extern void __sync_icache_dcache(pte_t pteval);
bool pgattr_change_is_safe(pteval_t old, pteval_t new);
+#ifdef CONFIG_DEBUG_VM
void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte);
+#else
+static inline void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
+{
+}
+#endif
static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
{
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index b5de4095d650..26f4cf7b753e 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -2408,15 +2408,13 @@ int arch_set_user_pkey_access(int pkey, unsigned long init_val)
*
* PTE_DIRTY || (PTE_WRITE && !PTE_RDONLY)
*/
+#ifdef CONFIG_DEBUG_VM
void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
{
char pte_str_old[PTVAL_STR_MAX];
char pte_str[PTVAL_STR_MAX];
pte_t old_pte;
- if (!IS_ENABLED(CONFIG_DEBUG_VM))
- return;
-
old_pte = __ptep_get(ptep);
if (!pte_valid(old_pte) || !pte_valid(pte))
@@ -2442,3 +2440,4 @@ void __check_safe_pte_update(struct mm_struct *mm, pte_t *ptep, pte_t pte)
__func__, pte_str_old, pte_str);
}
#endif
+#endif
>
>
> --
> Cheers,
>
> David
More information about the linux-arm-kernel
mailing list