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

[Shakeel Butt]

On Fri, Jun 26, 2026 at 06:25:48PM +0200, David Hildenbrand (Arm) wrote:
> On 6/24/26 23:04, Barry Song wrote:
> > On Wed, Jun 24, 2026 at 11:02 PM David Hildenbrand (Arm)
> > <david@xxxxxxxxxx> wrote:
> >>
> >> On 6/24/26 01:16, 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.
> >>>
> >>> Acked-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> >>> Reviewed-by: Baoquan He <baoquan.he@xxxxxxxxx>
> >>> 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 ff338c2abe92..f6848f4234a6 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;
> >>
> >> I'm not keen on making this function even uglier, so no, not like that.
> >>
> >> We have the earlier "folio_ref_count(folio) > 3" check.
> >>
> >> In which scenarios can you trigger this such that we would care?
> >>
> >> If the answer is "I don't know" there is no reason for a change.
> > 
> > As I replied to Shakeel in the v1 discussion [1], this can avoid a
> > large number of drains, both during Ubuntu boot and under normal
> > workloads:
> > 
> > "I booted the system into Ubuntu, and after
> 
> is this with this patch only?
> 
> > 
> > boot completed I observed:
> > 
> > wp_reuse_skipped_drain: 5542
> > do_swap_skipped_drain: 0
> > 
> > Then I built the kernel in a 1GB memcg using zRAM swap, and observed:
> > 
> > wp_reuse_skipped_drain: 25017
> > do_swap_skipped_drain: 43595
> 
> This is all data that belongs into this patch description, not hidden
> somewhere on the internet :)

I asked the same i.e. to include this information in the commit message [1].

[1] https://lore.kernel.org/linux-mm/ajK4N2zK3sPZFQuf@xxxxxxxxx/

> 
> 
> And why do we care about local draining? This is not a drain-all.

Let me take a stab at it. Local draining can potentially make the lru cache's
batching ineffective. The whole reason for lru caches is to batch the lru
operations to keep the lru lock contention low but if we keep draining
prematurely we will continue to make lru lock contention worse. This is
particularly bad for workloads which chrun a lot of LRU pages through
allocation, free and/or reclaim.