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@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> > > > Cc: David Hildenbrand <david@xxxxxxxxxx>
> > > > Cc: linux-mm@xxxxxxxxx
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> > > > ---
> > > >    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