[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8f9a4a13-2881-4baf-ab62-3d0d79e0cd3c@redhat.com>
Date: Wed, 10 Sep 2025 10:51:10 +0200
From: David Hildenbrand <david@...hat.com>
To: Chunyan Zhang <zhang.lyra@...il.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 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).
--
Cheers
David / dhildenb
Powered by blists - more mailing lists