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

[David Hildenbrand (Arm)]

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 :)


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

Really, this patch needs more motivation clearly spelled out.

> 
> So in summary, even without swap-in we can save a significant number of
> drains in wp_can_reuse_anon_folio(). With heavy swap-in workloads, most
> of the savings come from avoiding drains in do_swap_page(), while we
> still see a substantial number of skipped drains in wp_can_reuse_anon_folio()."
> 
> This is exactly the case where folio_ref_count(folio) == 3 and
> !folio_test_swapcache(folio). That is why checking
> folio_ref_count(folio) > 3 does not help.

diff --git a/mm/memory.c b/mm/memory.c
index fec2e9f2858a6..153a6166beeb4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4181,6 +4181,9 @@ static bool __wp_can_reuse_large_anon_folio(struct folio *folio,
 static bool wp_can_reuse_anon_folio(struct folio *folio,
                                    struct vm_area_struct *vma)
 {
+       const bool in_lru_cache = !folio_test_lru(folio);
+       const bool in_swapcache = folio_test_swapcache(folio);
+
        if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && folio_test_large(folio))
                return __wp_can_reuse_large_anon_folio(folio, vma);
 
@@ -4191,15 +4194,16 @@ static bool wp_can_reuse_anon_folio(struct folio *folio,
         *
         * KSM doesn't necessarily raise the folio refcount.
         */
-       if (folio_test_ksm(folio) || folio_ref_count(folio) > 3)
+       if (folio_test_ksm(folio) ||
+           folio_ref_count(folio) > 1 + in_lru_cache + in_swapcache)
                return false;
-       if (!folio_test_lru(folio))
+       if (in_lru_cache)
                /*
                 * We cannot easily detect+handle references from
                 * remote LRU caches or references to LRU folios.
                 */
                lru_add_drain();
-       if (folio_ref_count(folio) > 1 + folio_test_swapcache(folio))
+       if (folio_ref_count(folio) > 1 + in_swapcache)
                return false;
        if (!folio_trylock(folio))
                return false;

?

But I also want to hear why we care about the local draining.

-- 
Cheers,

David