Re: [PATCH v3 10/13] mm/huge_memory: separate out the folio part of zap_huge_pmd()

[Baolin Wang]

On 3/23/26 6:42 PM, Lorenzo Stoakes (Oracle) wrote:
> On Sat, Mar 21, 2026 at 01:59:34PM +0800, Baolin Wang wrote:
> > On 3/21/26 2:07 AM, Lorenzo Stoakes (Oracle) wrote:
> > > Place the part of the logic that manipulates counters and possibly updates
> > > the accessed bit of the folio into its own function to make zap_huge_pmd()
> > > more readable.
> > >
> > > Also rename flush_needed to is_present as we only require a flush for
> > > present entries.
> > >
> > > Additionally add comments as to why we're doing what we're doing with
> > > respect to softleaf entries.
> > >
> > > This also lays the ground for further refactoring.
> > >
> > > Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx>
> > > ---
> > >    mm/huge_memory.c | 61 +++++++++++++++++++++++++++---------------------
> > >    1 file changed, 35 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > > index 673d0c4734ad..9ddf38d68406 100644
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -2325,6 +2325,37 @@ static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)
> > >    	mm_dec_nr_ptes(mm);
> > >    }
> > > +static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma,
> > > +		pmd_t pmdval, struct folio *folio, bool is_present,
> > > +		bool *has_deposit)
> > > +{
> > > +	const bool is_device_private = folio_is_device_private(folio);
> > > +
> > > +	/* Present and device private folios are rmappable. */
> > > +	if (is_present || is_device_private)
> > > +		folio_remove_rmap_pmd(folio, &folio->page, vma);
> > > +
> > > +	if (folio_test_anon(folio)) {
> > > +		*has_deposit = true;
> > > +		add_mm_counter(mm, MM_ANONPAGES, -HPAGE_PMD_NR);
> > > +	} else {
> > > +		add_mm_counter(mm, mm_counter_file(folio),
> > > +			       -HPAGE_PMD_NR);
> > > +
> > > +		/*
> > > +		 * Use flush_needed to indicate whether the PMD entry
> > > +		 * is present, instead of checking pmd_present() again.
> > > +		 */
> > > +		if (is_present && pmd_young(pmdval) &&
> > > +		    likely(vma_has_recency(vma)))
> > > +			folio_mark_accessed(folio);
> >
> > Nit: these comments were added by me to explain why 'flush_needed' was
> > used:). Since it has been renamed to the more readable 'is_present', these
> > comments are now redundant and can be removed.
>
> Ack, I think it's _probably_ ok to leave that as a later commit removes it
> anyway, if that works for you?

Ah, I saw you removed them in the following patch. Looks fine to me. Thanks.