[PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Anshuman Khandual]

Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
anyways defaults into READ_ONCE() in cases where platform does not override

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/mapping_dirty_helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
index c193de6cb23a..737c407f4081 100644
--- a/mm/mapping_dirty_helpers.c
+++ b/mm/mapping_dirty_helpers.c
@@ -149,7 +149,7 @@ static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
 			      struct mm_walk *walk)
 {
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
-	pud_t pudval = READ_ONCE(*pud);
+	pud_t pudval = pudp_get(pud);
 
 	/* Do not split a huge pud */
 	if (pud_trans_huge(pudval)) {
-- 
2.30.2

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Dev Jain]

On 06/10/25 11:22 am, Anshuman Khandual wrote:
> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
> anyways defaults into READ_ONCE() in cases where platform does not override
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   mm/mapping_dirty_helpers.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
> index c193de6cb23a..737c407f4081 100644
> --- a/mm/mapping_dirty_helpers.c
> +++ b/mm/mapping_dirty_helpers.c
> @@ -149,7 +149,7 @@ static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
>   			      struct mm_walk *walk)
>   {
>   #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> -	pud_t pudval = READ_ONCE(*pud);
> +	pud_t pudval = pudp_get(pud);
>   
>   	/* Do not split a huge pud */
>   	if (pud_trans_huge(pudval)) {

Talking about mm, why not also make changes for these READ_ONCE accesses
in gup, hmm, memory, mprotect, sparse-vmemmap?

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Anshuman Khandual]

On 06/10/25 11:37 AM, Dev Jain wrote:
> 
> On 06/10/25 11:22 am, Anshuman Khandual wrote:
>> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
>> anyways defaults into READ_ONCE() in cases where platform does not override
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   mm/mapping_dirty_helpers.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
>> index c193de6cb23a..737c407f4081 100644
>> --- a/mm/mapping_dirty_helpers.c
>> +++ b/mm/mapping_dirty_helpers.c
>> @@ -149,7 +149,7 @@ static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
>>                     struct mm_walk *walk)
>>   {
>>   #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>> -    pud_t pudval = READ_ONCE(*pud);
>> +    pud_t pudval = pudp_get(pud);
>>         /* Do not split a huge pud */
>>       if (pud_trans_huge(pudval)) {
> 
> Talking about mm, why not also make changes for these READ_ONCE accesses
> in gup, hmm, memory, mprotect, sparse-vmemmap?
> 

Right, could replace all mm/ READ_ONCE() for pxdp pointers with the pgtable helpers
but that will create too much code churn in a single patch. Thought of doing these
replacements per file will be much more contained which is easy both for review and
testing.

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[David Hildenbrand]

On 06.10.25 07:52, Anshuman Khandual wrote:
> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
> anyways defaults into READ_ONCE() in cases where platform does not override
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Lance Yang]

On Mon, Oct 6, 2025 at 2:28 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 06/10/25 11:37 AM, Dev Jain wrote:
> >
> > On 06/10/25 11:22 am, Anshuman Khandual wrote:
> >> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
> >> anyways defaults into READ_ONCE() in cases where platform does not override

Nice cleanup!

> >>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Cc: David Hildenbrand <david@redhat.com>
> >> Cc: linux-mm@kvack.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >>   mm/mapping_dirty_helpers.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
> >> index c193de6cb23a..737c407f4081 100644
> >> --- a/mm/mapping_dirty_helpers.c
> >> +++ b/mm/mapping_dirty_helpers.c
> >> @@ -149,7 +149,7 @@ static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
> >>                     struct mm_walk *walk)
> >>   {
> >>   #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
> >> -    pud_t pudval = READ_ONCE(*pud);
> >> +    pud_t pudval = pudp_get(pud);
> >>         /* Do not split a huge pud */
> >>       if (pud_trans_huge(pudval)) {
> >
> > Talking about mm, why not also make changes for these READ_ONCE accesses
> > in gup, hmm, memory, mprotect, sparse-vmemmap?

Yep, I agree with Dev on that one. Because the change is assumed to be a
no-op on all architectures for now, right?

> >
>
> Right, could replace all mm/ READ_ONCE() for pxdp pointers with the pgtable helpers
> but that will create too much code churn in a single patch. Thought of doing these
> replacements per file will be much more contained which is easy both for review and
> testing.

Emm... as pointed out by Dev, it would get the entire cleanup done in one
go, avoiding the churn of multiple small patches ;)

Cheers,
Lance

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[David Hildenbrand]

On 06.10.25 08:48, Lance Yang wrote:
> On Mon, Oct 6, 2025 at 2:28 PM Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>>
>>
>> On 06/10/25 11:37 AM, Dev Jain wrote:
>>>
>>> On 06/10/25 11:22 am, Anshuman Khandual wrote:
>>>> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
>>>> anyways defaults into READ_ONCE() in cases where platform does not override
> 
> Nice cleanup!
> 
>>>>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Cc: David Hildenbrand <david@redhat.com>
>>>> Cc: linux-mm@kvack.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>    mm/mapping_dirty_helpers.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
>>>> index c193de6cb23a..737c407f4081 100644
>>>> --- a/mm/mapping_dirty_helpers.c
>>>> +++ b/mm/mapping_dirty_helpers.c
>>>> @@ -149,7 +149,7 @@ static int wp_clean_pud_entry(pud_t *pud, unsigned long addr, unsigned long end,
>>>>                      struct mm_walk *walk)
>>>>    {
>>>>    #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>>> -    pud_t pudval = READ_ONCE(*pud);
>>>> +    pud_t pudval = pudp_get(pud);
>>>>          /* Do not split a huge pud */
>>>>        if (pud_trans_huge(pudval)) {
>>>
>>> Talking about mm, why not also make changes for these READ_ONCE accesses
>>> in gup, hmm, memory, mprotect, sparse-vmemmap?
> 
> Yep, I agree with Dev on that one. Because the change is assumed to be a
> no-op on all architectures for now, right?
> 
>>>
>>
>> Right, could replace all mm/ READ_ONCE() for pxdp pointers with the pgtable helpers
>> but that will create too much code churn in a single patch. Thought of doing these
>> replacements per file will be much more contained which is easy both for review and
>> testing.
> 
> Emm... as pointed out by Dev, it would get the entire cleanup done in one
> go, avoiding the churn of multiple small patches ;)

I think I'd prefer smaller patches here. For example, the discussion on 
the debug PT stuff was quite helpful and probably would just have fallen 
through the cracks when blindly replacing all READ_ONCE.

Having that said, combining some simple cases across multiple files does 
not sound too bad either.

-- 
Cheers

David / dhildenb

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Dev Jain]

On 06/10/25 11:22 am, Anshuman Khandual wrote:
> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
> anyways defaults into READ_ONCE() in cases where platform does not override
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>   

Reviewed-by: Dev Jain <dev.jain@arm.com>

Re: [PATCH] mm/dirty: Replace READ_ONCE() with pudp_get()

[Oscar Salvador]

On Mon, Oct 06, 2025 at 06:52:14AM +0100, Anshuman Khandual wrote:
> Replace READ_ONCE() with a standard page table accessor i.e pudp_get() that
> anyways defaults into READ_ONCE() in cases where platform does not override
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

 

-- 
Oscar Salvador
SUSE Labs