lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAfSe-snV-XZ3xGmK6gXNXw-D3ECWbyQgG+WG3c5gAsREz4ccQ@mail.gmail.com>
Date: Thu, 11 Sep 2025 10:51:54 +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()

On Wed, 10 Sept 2025 at 16:51, David Hildenbrand <david@...hat.com> wrote:
>
> On 10.09.25 10:25, Chunyan Zhang wrote:
> > 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)
>
> Maybe we can simplify to
>
> #ifndef pgtable_soft_dirty_supported
> #define pgtable_soft_dirty_supported()  IS_ENABLED(CONFIG_MEM_SOFT_DIRTY)
> #endif
>
> And then just let the arch that overrides this function just make it
> respect IS_ENABLED(CONFIG_MEM_SOFT_DIRTY).

Ok, got you, I will address it.

Thanks for your review,
Chunyan


>
> --
> Cheers
>
> David / dhildenb
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ