Re: [PATCH v2 2/3] mm: rmap: support batched checks of the references for large folios

[Baolin Wang]

On 2025/12/18 20:08, Ryan Roberts wrote:
> > > >    +#define clear_flush_young_ptes clear_flush_young_ptes
> > > > +static inline int clear_flush_young_ptes(struct vm_area_struct *vma,
> > > > +                    unsigned long addr, pte_t *ptep,
> > > > +                    unsigned int nr)
> > > > +{
> > > > +    if (likely(nr == 1))
> > > > +        return __ptep_clear_flush_young(vma, addr, ptep);
> > >
> > > Bug: This is broken if core-mm tries to call this for nr=1 on a pte that is part
> > > of a contpte mapping.
> > >
> > > The similar fastpaths are here to prevent regressing the common small folio case.
> >
> > Thanks for catching this. I had considered this before, but I still missed it.
> >
> > > I guess here the best approach is (note no leading underscores):
> > >
> > >      if (likely(nr == 1))
> > >          return ptep_clear_flush_young(vma, addr, ptep);
> >
> > However, I prefer to use pte_cont() to check it. Later, I plan to clean up the
> > ptep_clear_flush_young().
> >
> >      if (nr == 1 && !pte_cont(__ptep_get(ptep))
> >          return __ptep_clear_flush_young(vma, addr, ptep);
>
> Sure. That would follow the pattern in clear_young_dirty_ptes(). Please use the
> likely() hint as is done everywhere else:
>
> 	if (likely(nr == 1 && !pte_cont(__ptep_get(ptep))))

Sure.

> I notice that ptep_test_and_clear_young() and ptep_clear_flush_young() are both
> testing aginst pte_valid_cont(). These could probably be relaxed to pte_cont()
> since it is implicit the the pte must be valid?

Yes, I think so. I can do a cleanup later in a separate patch.