Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()

[Baoquan He]

On 06/12/26 at 09:57am, Barry Song wrote:
> On Fri, Jun 12, 2026 at 9:08 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> >
> > On 06/11/26 at 11:17am, Shakeel Butt wrote:
> > > On Thu, Jun 11, 2026 at 11:09:43AM -0700, Shakeel Butt wrote:
> > > > On Thu, Jun 11, 2026 at 06:51:22PM +0800, Barry Song (Xiaomi) wrote:
> > > > > We always unconditionally drain the LRU before retrying anon folio
> > > > > reuse in wp_can_reuse_anon_folio(). Instead, assume !LRU anon folios
> > > > > are in lru_cache, and use the refcount to avoid many unnecessary LRU
> > > > > drains.
> > > > >
> > > > > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > > > > ---
> > > > >  mm/memory.c | 8 +++++++-
> > > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > > index 56be920c56d7..487a34377a7b 100644
> > > > > --- a/mm/memory.c
> > > > > +++ b/mm/memory.c
> > > > > @@ -4193,12 +4193,18 @@ static bool wp_can_reuse_anon_folio(struct folio *folio,
> > > > >    */
> > > > >   if (folio_test_ksm(folio) || folio_ref_count(folio) > 3)
> > > > >           return false;
> > > > > - if (!folio_test_lru(folio))
> > > > > + if (!folio_test_lru(folio)) {
> > > > > +         /*
> > > > > +          * Assume folio is on lru_cache and holds a cache reference.
> > > > > +          */
> > > > > +         if (folio_ref_count(folio) > 2 + folio_test_swapcache(folio))
> > > > > +                 return false;
> > > >
> > > > In your experiments, how much amount of drains were reduced due to this specific
> > > > check?
> > > >
> > > > I wonder if that data can motivate to introduce lru_add_drain_folio(folio) which
> > > > only drains if the given folio is in the local lru_add cache.
> > >
> > > Actually if we can peek into lru_add cache and folio_ref_count(folio) is exactly
> > > equal to (2 + folio_test_swapcache(folio)) and folio is not on LRU then we can
> > > just reuse folio if it is in lru_add cache without draining, right?
> >
> > Sounds more reasonable if we can only touch the wanted folio if it
> > exists in pvec. Believe it can improve efficiency more than the
> > current patch.
> >
> 
> Technically yes, but in practice it seems quite hard. As I explained
> to Shakeel, we don't have an easy way to determine whether a folio is
> sitting in the lru_cache. !folio_test_lru(folio) only tells us that
> the folio may either have been removed from the LRU or still be
> sitting in the lru_cache; it does not distinguish between the two.
> Unless we want to scan the lru_cache to look up the folio, we may need
> a dedicated folio flag, similar to PG_lru, to indicate lru_cache
> membership -
> I guess people would be quite unhappy about adding a new flag?
> 
> On the other hand, the folio may be sitting in the lru_cache of
> another CPU, which the current drain cannot flush. As things stand
> today, the drain only succeeds if the folio happens to be queued on
> the current CPU's lru_cache, giving it roughly a 1/nr_cpus chance of
> working. drain_all would clearly be too expensive to avoid.

BAM, sitting in other CPU's pvec is a real barrier. If it's in local
cpu, it definitely deserves a try as Shakeel said, when refcount = (2 +
folio_test_swapcache(folio)). 

Then the current solution is optimal at present.

> 
> So another possibility is to drop this drain as well. The folio can
> be released later, at the cost of missing some opportunities for
> reuse.
> 
> Best Regards
> Barry