Re: [PATCH v4 4/4] mm: huge_memory: use folio_needs_prot_numa() for pmd folio

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Tue, Oct 21, 2025 at 09:37:02PM +0800, Kefeng Wang wrote:
>
>
> On 2025/10/20 23:18, Kefeng Wang wrote:
> >
> >
> > On 2025/10/20 21:23, David Hildenbrand wrote:
> > >
> > > > >           /*
> > > > >            * Avoid trapping faults against the zero page. The read-only
> > > > >            * data is likely to be read-cached on the local CPU and
> > > > > @@ -2490,19 +2490,13 @@ int change_huge_pmd(struct
> > > > > mmu_gather *tlb, struct vm_area_struct *vma,
> > > > >           if (pmd_protnone(*pmd))
> > > > >               goto unlock;
> > > > >
> > > > > -        folio = pmd_folio(*pmd);
> > > > > -        toptier = node_is_toptier(folio_nid(folio));
> > > > > -        /*
> > > > > -         * Skip scanning top tier node if normal numa
> > > > > -         * balancing is disabled
> > > > > -         */
> > > > > -        if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL) &&
> > > > > -            toptier)
> > > > > -            goto unlock;
> > > > > +        /* Get target node for single threaded private VMAs */
> > > > > +        if (!(vma->vm_flags & VM_SHARED) &&
> > > > > +            atomic_read(&vma->vm_mm->mm_users) == 1)
> > > > > +            target_node = numa_node_id();
> > > >
> > > > This is duplicated in both callers, and only used by
> > > > folio_needs_prot_numa(),
> > > > why not abstract this to the function also?
> > >
> > > There was a discussion on that in v3 I think where I asked the same
> > > question.
> > >
> >
> > Yes, it is in v1, for pte, we could avoid 512 times check and the
> > numa_node_id(), so we leave it as is.
> >
>
> If no objection, I won't change this and will send a new version with

See feedback to your response.

In general - please do leave time for reviewers to get back to you, the
review load in THP is very high and we can't always respond right away.

>
> 1) kerneldoc refresh for folio_need_prot_numa, per Lorenzo

OK you said in the other thread you weren't going to do anything, I guess a
miscommunication as you are saying here that you _are_ going to address at
least this part of my feedback.

We're going to need a long and clear kerneldoc there to clarify things.

> 2) update the changelog of this one, per David

See also my feedback here.

>
> Thanks
>