[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAfSe-vbvGQy9JozQY3vsqrrPrTaWYMcNw+NaDf3nReWz8ynZg@mail.gmail.com>
Date: Wed, 10 Sep 2025 16:25:28 +0800
From: Chunyan Zhang <zhang.lyra@...il.com>
To: David Hildenbrand <david@...hat.com>
Cc: Chunyan Zhang <zhangchunyan@...as.ac.cn>, linux-riscv@...ts.infradead.org,
linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>, Deepak Gupta <debug@...osinc.com>, Ved Shanbhogue <ved@...osinc.com>,
Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Andrew Morton <akpm@...ux-foundation.org>, Peter Xu <peterx@...hat.com>,
Arnd Bergmann <arnd@...db.de>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>,
Mike Rapoport <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Axel Rasmussen <axelrasmussen@...gle.com>, Yuanchu Xie <yuanchu@...gle.com>
Subject: Re: [PATCH V10 1/5] mm: softdirty: Add pte_soft_dirty_available()
Hi David,
On Tue, 9 Sept 2025 at 19:42, David Hildenbrand <david@...hat.com> wrote:
>
> On 09.09.25 11:56, Chunyan Zhang wrote:
> > Some platforms can customize the PTE soft dirty bit and make it unavailable
> > even if the architecture allows providing the PTE resource.
> >
> > Add an API which architectures can define their specific implementations
> > to detect if the PTE soft-dirty bit is available, on which the kernel
> > is running.
> >
> > Signed-off-by: Chunyan Zhang <zhangchunyan@...as.ac.cn>
> > ---
> > fs/proc/task_mmu.c | 17 ++++++++++++++++-
> > include/linux/pgtable.h | 10 ++++++++++
> > mm/debug_vm_pgtable.c | 9 +++++----
> > mm/huge_memory.c | 10 ++++++----
> > mm/internal.h | 2 +-
> > mm/mremap.c | 10 ++++++----
> > mm/userfaultfd.c | 6 ++++--
> > 7 files changed, 48 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index 29cca0e6d0ff..20a609ec1ba6 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -1058,7 +1058,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> > * -Werror=unterminated-string-initialization warning
> > * with GCC 15
> > */
> > - static const char mnemonics[BITS_PER_LONG][3] = {
> > + static char mnemonics[BITS_PER_LONG][3] = {
> > /*
> > * In case if we meet a flag we don't know about.
> > */
> > @@ -1129,6 +1129,16 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> > [ilog2(VM_SEALED)] = "sl",
> > #endif
> > };
> > +/*
> > + * We should remove the VM_SOFTDIRTY flag if the PTE soft-dirty bit is
> > + * unavailable on which the kernel is running, even if the architecture
> > + * allows providing the PTE resource and soft-dirty is compiled in.
> > + */
> > +#ifdef CONFIG_MEM_SOFT_DIRTY
> > + if (!pte_soft_dirty_available())
> > + mnemonics[ilog2(VM_SOFTDIRTY)][0] = 0;
> > +#endif
> > +
> > size_t i;
> >
> > seq_puts(m, "VmFlags: ");
> > @@ -1531,6 +1541,8 @@ static inline bool pte_is_pinned(struct vm_area_struct *vma, unsigned long addr,
> > static inline void clear_soft_dirty(struct vm_area_struct *vma,
> > unsigned long addr, pte_t *pte)
> > {
> > + if (!pte_soft_dirty_available())
> > + return;
> > /*
> > * The soft-dirty tracker uses #PF-s to catch writes
> > * to pages, so write-protect the pte as well. See the
> > @@ -1566,6 +1578,9 @@ static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma,
> > {
> > pmd_t old, pmd = *pmdp;
> >
> > + if (!pte_soft_dirty_available())
> > + return;
> > +
> > if (pmd_present(pmd)) {
> > /* See comment in change_huge_pmd() */
> > old = pmdp_invalidate(vma, addr, pmdp);
> > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> > index 4c035637eeb7..c0e2a6dc69f4 100644
> > --- a/include/linux/pgtable.h
> > +++ b/include/linux/pgtable.h
> > @@ -1538,6 +1538,15 @@ static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
> > #endif
> >
> > #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
> > +
> > +/*
> > + * Some platforms can customize the PTE soft dirty bit and make it unavailable
> > + * even if the architecture allows providing the PTE resource.
> > + */
> > +#ifndef pte_soft_dirty_available
> > +#define pte_soft_dirty_available() (true)
> > +#endif
> > +
> > #ifndef CONFIG_ARCH_ENABLE_THP_MIGRATION
> > static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
> > {
> > @@ -1555,6 +1564,7 @@ static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
> > }
> > #endif
> > #else /* !CONFIG_HAVE_ARCH_SOFT_DIRTY */
> > +#define pte_soft_dirty_available() (false)
> > static inline int pte_soft_dirty(pte_t pte)
> > {
> > return 0;
> > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> > index 830107b6dd08..98ed7e22ccec 100644
> > --- a/mm/debug_vm_pgtable.c
> > +++ b/mm/debug_vm_pgtable.c
> > @@ -690,7 +690,7 @@ static void __init pte_soft_dirty_tests(struct pgtable_debug_args *args)
> > {
> > pte_t pte = pfn_pte(args->fixed_pte_pfn, args->page_prot);
> >
> > - if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY))
> > + if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY) || !pte_soft_dirty_available())
>
> I suggest that you instead make pte_soft_dirty_available() be false without CONFIG_MEM_SOFT_DIRTY.
>
> e.g., for the default implementation
>
> define pte_soft_dirty_available() IS_ENABLED(CONFIG_MEM_SOFT_DIRTY)
>
> That way you can avoid some ifefs and cleanup these checks.
Do you mean something like this:
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1538,6 +1538,16 @@ static inline pgprot_t pgprot_modify(pgprot_t
oldprot, pgprot_t newprot)
#endif
#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
+#ifndef arch_soft_dirty_available
+#define arch_soft_dirty_available() (true)
+#endif
+#define pgtable_soft_dirty_supported()
(IS_ENABLED(CONFIG_MEM_SOFT_DIRTY) && arch_soft_dirty_available())
+
#ifndef CONFIG_ARCH_ENABLE_THP_MIGRATION
static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
{
@@ -1555,6 +1565,7 @@ static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
}
#endif
#else /* !CONFIG_HAVE_ARCH_SOFT_DIRTY */
+#define pgtable_soft_dirty_supported() (false)
>
>
> But as we do also have PMD soft-dirty support, I guess we would want to call this
> something more abstract "pgtable_soft_dirty_available" or "pgtable_soft_dirty_supported"
>
> --
> Cheers
>
> David / dhildenb
>
Powered by blists - more mailing lists