mm: drop redundant lru_add_drain in anon folio reuse paths

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +7 -2

index ff338c2abe92..f6848f4234a6 100644
@@ -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;
                /*
                 * 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))
                return false;
        if (!folio_trylock(folio))
--

2.39.3 (Apple Git-146)

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index ff338c2abe92..f6848f4234a6 100644

Quoted 17 lines · Barry Song (Xiaomi)

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index ff338c2abe92..f6848f4234a6 100644

mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201

Quoted 10 lines

mm/memory.c +6 -1

@@ -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.

Quoted 36 lines · David Hildenbrand (Arm)

Quoted 17 lines · Barry Song (Xiaomi)

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index ff338c2abe92..f6848f4234a6 100644

mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201

Quoted 10 lines

mm/memory.c +6 -1

@@ -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?

Quoted 1 line

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

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

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.

Quoted 38 lines · Barry Song

Quoted 36 lines · David Hildenbrand (Arm)

Quoted 17 lines · Barry Song (Xiaomi)

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index ff338c2abe92..f6848f4234a6 100644

mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201

Quoted 10 lines

mm/memory.c +6 -1

@@ -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?

Quoted 7 lines

Quoted 1 line

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?

Quoted 6 lines

boot completed I observed:

wp_reuse_skipped_drain: 5542
do_swap_skipped_drain: 0

Quoted 4 lines

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.

Quoted 41 lines · David Hildenbrand (Arm)

Quoted 40 lines · Barry Song

Quoted 37 lines · David Hildenbrand (Arm)

Quoted 17 lines · Barry Song (Xiaomi)

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 <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index ff338c2abe92..f6848f4234a6 100644

mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201

Quoted 10 lines

mm/memory.c +6 -1

@@ -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.

Quoted 7 lines

Quoted 5 lines

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?

Quoted 7 lines

Quoted 7 lines

boot completed I observed:

wp_reuse_skipped_drain: 5542
do_swap_skipped_drain: 0

Quoted 7 lines

Quoted 4 lines

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/[email protected]/

Quoted 4 lines

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.

Quoted 4 lines · Shakeel Butt

Quoted 2 lines · David Hildenbrand (Arm)

is this with this patch only?

Right. wp_reuse_skipped_drain accounts for the behavior introduced by this patch, while do_swap_skipped_drain comes from patch 3/3.

Quoted 4 lines

Quoted 4 lines

This is all data that belongs into this patch description, not hidden somewhere on the internet :)

Agreed. Sorry, I was being a bit lazy.

Quoted 4 lines

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

[1] https://lore.kernel.org/linux-mm/[email protected]/

Sorry, I missed this one. I'll add it in v3.

Quoted 11 lines

Quoted 4 lines

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.

I actually asked the same question myself in the RFC here[1]:

"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. 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."

After thinking about it for a couple of days, my gut feeling is that keeping it might increase the chances of reuse in at least two cases:
1. It could be a newly allocated folio in do_swap_page(), and do_swap_page() may subsequently call do_wp_page(). In that case, task migration is less likely to have occurred.
2. On systems with fewer CPUs (and typically less memory), the chance of having the folio in the same CPU's lru_cache, which is roughly 1 / nr_cpus, may still be reasonably high.

I'm not entirely sure about this, so I chose a relatively conservative approach: making an obviously correct improvement without introducing dramatic changes. Another reason is that it's also hard to evaluate the impact of removing the local drain across all systems and workloads.

Shakeel, David, what are your thoughts on this? Should we go with a code refinement in v3 as David suggested, or completely remove this drain?

So yeah, if Nadav's reproducer works even without the drain (which I assume), we could indeed think about just removing this optimization here.

On Mon, Jun 29, 2026 at 3:52 PM David Hildenbrand (Arm) <[email protected]> wrote:
[...]

Quoted 3 lines

So yeah, if Nadav's reproducer works even without the drain (which I assume), we could indeed think about just removing this optimization here.

Yet I think the problem is resolved with the exclusive flag. Right now, we can still have cases where "exclusive" cannot address it, such as:

#include <sys/mman.h>

int main(int argc, char *argv[])

{
    int i;
    #define SIZE 30*1024*1024
    while(1) {
        volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
        volatile int q;

        for (int i = 0; i < SIZE/sizeof(int); i++)
              p[i] =  i;
        madvise((void *)p, SIZE, MADV_PAGEOUT);
        if (!fork())
             _exit(0);
        for (int i = 0; i < SIZE/sizeof(int); i++)
              q = p[i];
        for (int i = 0; i < SIZE/sizeof(int); i++)
              p[i] =  i;
        munmap(p, SIZE);
    }
    return 0;
}

I assume this is less frequent and less of a concern?

The "we just allocated them without exposing them to the swapcache" case no longer exists, as Kairui has routed synchronous I/O through the swapcache as well in his series "unify swapin use swap cache and cleanup flags"[1]. As a result, folio_ref_count() should never be 1 in this path, since at least two references are held (base ref plus swapcache). Remove the folio_ref_count()==1 check and update the comment accordingly.

[1] https://lore.kernel.org/all/[email protected]/

Acked-by Usama Arif <[email protected]>

Reviewed-by Kairui Song <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

mm/memory.c +0 -0

index f6848f4234a6..abd0adcf65f0 100644

Quoted 25 lines · Barry Song (Xiaomi)

The "we just allocated them without exposing them to the swapcache" case no longer exists, as Kairui has routed synchronous I/O through the swapcache as well in his series "unify swapin use swap cache and cleanup flags"[1]. As a result, folio_ref_count() should never be 1 in this path, since at least two references are held (base ref plus swapcache). Remove the folio_ref_count()==1 check and update the comment accordingly.

[1] https://lore.kernel.org/all/[email protected]/

Acked-by Usama Arif <[email protected]>

Reviewed-by Kairui Song <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Acked-by Shakeel Butt <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

mm/memory.c +0 -0

index f6848f4234a6..abd0adcf65f0 100644
@@ -5049,12 +5049,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 

mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5051

Quoted 2 lines

mm/memory.c +0 -0

      /*
       * Same logic as in do_wp_page(); however, optimize for pages that are

s/Same/Similar/ ?

mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5054

Quoted 9 lines

mm/memory.c +2 -5

-      * certainly not shared either because we just allocated them without
-      * exposing them to the swapcache or because the swap entry indicates
-      * exclusivity.
+      * certainly not because the swap entry indicates exclusivity.
       */
-     if (!folio_test_ksm(folio) &&
-         (exclusive || folio_ref_count(folio) == 1)) {
+     if (!folio_test_ksm(folio) && exclusive) {

Hmm, but KSM folios should never have "exclusive" set. So I think you can drop that as well (was only relevant with folio_ref_count==1 check IIRC).

We are doing a lot of redundant lru_add_drain() calls in do_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draining lru_cache 100% of the time:

int main(int argc, char *argv[])

{
        int i;
 #define SIZE 100*1024*1024
        while(1) {
                volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
                        MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

                for (int i = 0; i < SIZE/sizeof(int); i++)
                        p[i] =  i%64;
                madvise((void *)p, SIZE, MADV_PAGEOUT);
                for (int i = 0; i < SIZE/sizeof(int); i++)
                        p[i] =  i%64;
                munmap(p, SIZE);
        }
        return 0;
}

Folio reuse now relies primarily on the exclusive hint, making lru_cache draining to drop the refcount in lru_cache largely irrelevant.

Acked-by Shakeel Butt <[email protected]>

Reviewed-by Baoquan He <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 10 ----------
 1 file changed, 10 deletions(-)

mm/memory.c +0 -11

index abd0adcf65f0..2983a6baf474 100644
@@ -4903,16 +4903,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
        } else if (folio != swapcache)
                page = folio_page(folio, 0);
 
-       /*
-        * If we want to map a page that's in the swapcache writable, we
-        * have to detect via the refcount if we're really the exclusive
-        * owner. Try removing the extra reference from the local LRU
-        * caches if required.
-        */
-       if ((vmf->flags & FAULT_FLAG_WRITE) &&
-           !folio_test_ksm(folio) && !folio_test_lru(folio))
-               lru_add_drain();
-
        folio_throttle_swaprate(folio, GFP_KERNEL);
 
        /*
--

2.39.3 (Apple Git-146)

We are doing a lot of redundant lru_add_drain() calls in do_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draining lru_cache 100% of the time:

int main(int argc, char *argv[])

{
        int i;
 #define SIZE 100*1024*1024
	while(1) {
		volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
                        MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

		for (int i = 0; i < SIZE/sizeof(int); i++)
			p[i] =  i%64;
		madvise((void *)p, SIZE, MADV_PAGEOUT);
		for (int i = 0; i < SIZE/sizeof(int); i++)
			p[i] =  i%64;
		munmap(p, SIZE);
	}
	return 0;
}

Folio reuse now relies primarily on the exclusive hint, making lru_cache draining to drop the refcount in lru_cache largely irrelevant.

Originally, we unconditionally called lru_add_drain() for write swap-in page faults. This might drop the reference held by the per-CPU LRU cache if the folio happened to reside there. However, there was no guarantee that the folio was actually cached on the current CPU.

Now that lru_add_drain() has been removed, we have lost one opportunity to drop a reference held by the LRU cache. We could instead incorporate that possibility into the condition evaluated by should_try_to_free_swap().

Suggested-by Kairui Song <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index 2983a6baf474..14577c67c61a 100644
@@ -5087,8 +5087,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	 * Remove the swap entry and conditionally try to free up the swapcache.
 	 * Do it after mapping, so raced page faults will likely see the folio

Quoted 23 lines · Barry Song (Xiaomi)

Originally, we unconditionally called lru_add_drain() for write swap-in page faults. This might drop the reference held by the per-CPU LRU cache if the folio happened to reside there. However, there was no guarantee that the folio was actually cached on the current CPU.

Now that lru_add_drain() has been removed, we have lost one opportunity to drop a reference held by the LRU cache. We could instead incorporate that possibility into the condition evaluated by should_try_to_free_swap().

Suggested-by Kairui Song <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index 2983a6baf474..14577c67c61a 100644
@@ -5087,8 +5087,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
       * Remove the swap entry and conditionally try to free up the swapcache.
       * Do it after mapping, so raced page faults will likely see the folio

mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5097

Quoted 10 lines

mm/memory.c +4 -1

       * in swap cache and wait on the folio lock.
+      * Assume non-LRU folios may be queued in the LRU cache, which contributes
+      * an additional reference to the folio.
       */
-     if (should_try_to_free_swap(si, folio, vma, nr_pages, vmf->flags))
+     if (should_try_to_free_swap(si, folio, vma, nr_pages +
+                     !folio_test_lru(folio), vmf->flags))
              folio_free_swap(folio);
 
      folio_unlock(folio);

Hm, in wp_can_reuse_anon_folio() we'll try dropping the swapcache ourselves.

So I wonder if we still need that handling ("If we want to map a page that's in the swapcache writable, we ...") at all?

Ahh, I see the problem now:

commit 4b34f1d82c6549837b2061096dea249e881a4495

Author: Kairui Song <[email protected]>
Date:   Sat Dec 20 03:43:35 2025 +0800

mm, swap: free the swap cache after folio is mapped

Currently, we remove the folio from the swap cache and free the swap cache before mapping the PTE. To reduce repeated faults due to parallel swapins of the same PTE, change it to remove the folio from the swap cache after it is mapped. So new faults from the swap PTE will be much more likely to see the folio in the swap cache and wait on it.

This does not eliminate all swapin races: an ongoing swapin fault may still see an empty swap cache. That's harmless, as the PTE is changed before the swap cache is cleared, so it will just return and not trigger any repeated faults. This does help to reduce the chance.

That changed that behavior such that we *must* now always fallback to do_wp_page().

What a mess (I didn't ack)

Quoted 62 lines · David Hildenbrand (Arm)

Quoted 23 lines · Barry Song (Xiaomi)

Originally, we unconditionally called lru_add_drain() for write swap-in page faults. This might drop the reference held by the per-CPU LRU cache if the folio happened to reside there. However, there was no guarantee that the folio was actually cached on the current CPU.

Now that lru_add_drain() has been removed, we have lost one opportunity to drop a reference held by the LRU cache. We could instead incorporate that possibility into the condition evaluated by should_try_to_free_swap().

Suggested-by Kairui Song <[email protected]>

Signed-off-by Barry Song (Xiaomi) <[email protected]>

---
 mm/memory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

mm/memory.c +0 -0

index 2983a6baf474..14577c67c61a 100644
@@ -5087,8 +5087,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
       * Remove the swap entry and conditionally try to free up the swapcache.
       * Do it after mapping, so raced page faults will likely see the folio

mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5097

Quoted 10 lines

mm/memory.c +4 -1

       * in swap cache and wait on the folio lock.
+      * Assume non-LRU folios may be queued in the LRU cache, which contributes
+      * an additional reference to the folio.
       */
-     if (should_try_to_free_swap(si, folio, vma, nr_pages, vmf->flags))
+     if (should_try_to_free_swap(si, folio, vma, nr_pages +
+                     !folio_test_lru(folio), vmf->flags))
              folio_free_swap(folio);
 
      folio_unlock(folio);

Hm, in wp_can_reuse_anon_folio() we'll try dropping the swapcache ourselves.

So I wonder if we still need that handling ("If we want to map a page that's in the swapcache writable, we ...") at all?

Ahh, I see the problem now:

commit 4b34f1d82c6549837b2061096dea249e881a4495

Author: Kairui Song <[email protected]>
Date:   Sat Dec 20 03:43:35 2025 +0800

mm, swap: free the swap cache after folio is mapped

Currently, we remove the folio from the swap cache and free the swap cache before mapping the PTE. To reduce repeated faults due to parallel swapins of the same PTE, change it to remove the folio from the swap cache after it is mapped. So new faults from the swap PTE will be much more likely to see the folio in the swap cache and wait on it.

This does not eliminate all swapin races: an ongoing swapin fault may still see an empty swap cache. That's harmless, as the PTE is changed before the swap cache is cleared, so it will just return and not trigger any repeated faults. This does help to reduce the chance.

Quoted 3 lines

That changed that behavior such that we *must* now always fallback to do_wp_page().

What a mess (I didn't ack)

So it seems we need a patch before patch 4 to fix the missed swapcache freeing on write faults, because the write bit has already been cleared by the time the folio is mapped and reused?

mm/memory.c +1 -0

index 14577c67c61a..e8fc5a86f6ea 100644
@@ -4752,6 +4752,7 @@ static void check_swap_exclusive(struct folio
 *folio, swp_entry_t entry,
  */
 vm_fault_t do_swap_page(struct vm_fault *vmf)
 {
+       enum fault_flag fault_flags = vmf->flags;
        struct vm_area_struct *vma = vmf->vma;
        struct folio *swapcache = NULL, *folio;

        struct page *page;

mm/memory.c +1 -1

@@ -5091,7 +5092,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
         * an additional reference to the folio.
         */
        if (should_try_to_free_swap(si, folio, vma, nr_pages +
-                       !folio_test_lru(folio), vmf->flags))
+                       !folio_test_lru(folio), fault_flags))
                folio_free_swap(folio);
 
        folio_unlock(folio);

Best Regards
Barry

Quoted 3 lines

Quoted 3 lines

That changed that behavior such that we *must* now always fallback to do_wp_page().

What a mess (I didn't ack)

That's awkward, my bad, terribly sorry about this :(

I sincerely apologize for the oversight and the trouble this has caused.

I haven't seen any performance regression in any workload recently though, or any correctness issue, perhaps the round trip of do_wp_page wasn't that bad. It should still catch the reuse folios, just more costly than doing things in-place.

I think we should restore the original check first. We might also want to avoid dropping the swap cache if the folio will not be reused, which was discussed here:

https://lore.kernel.org/linux-mm/CAMgjq7BDfvNXdWH0cqarsujjUn3i3tDDhDkmSg01TR4h-tDorQ@mail.gmail.com/

Maybe extracting some common part into a helper can help make this cleaner.

Quoted 29 lines

So it seems we need a patch before patch 4 to fix the missed swapcache freeing on write faults, because the write bit has already been cleared by the time the folio is mapped and reused?

mm/memory.c +1 -0

index 14577c67c61a..e8fc5a86f6ea 100644
@@ -4752,6 +4752,7 @@ static void check_swap_exclusive(struct folio
 *folio, swp_entry_t entry,
  */
 vm_fault_t do_swap_page(struct vm_fault *vmf)
 {
+       enum fault_flag fault_flags = vmf->flags;
        struct vm_area_struct *vma = vmf->vma;
        struct folio *swapcache = NULL, *folio;

        struct page *page;

mm/memory.c +1 -1

@@ -5091,7 +5092,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
         * an additional reference to the folio.
         */
        if (should_try_to_free_swap(si, folio, vma, nr_pages +
-                       !folio_test_lru(folio), vmf->flags))
+                       !folio_test_lru(folio), fault_flags))
                folio_free_swap(folio);
 
        folio_unlock(folio);

Quoted 2 lines

Best Regards
Barry

Hi Barry,

The problem is more than that, the exclusive || folio_ref_count(folio) == 1 in do_swap_page is also ineffective now.

Quoted 9 lines

Quoted 4 lines

Quoted 4 lines

That changed that behavior such that we *must* now always fallback to do_wp_page().

What a mess (I didn't ack)

That's awkward, my bad, terribly sorry about this :(

I sincerely apologize for the oversight and the trouble this has caused.

All good, I was just surprised to see a previous optimization partially reverted without a clear reasoning :)

Because it should have removed the handling in should_try_to_free_swap() as well.

It's good that we are discussing it now!

Quoted 4 lines

I haven't seen any performance regression in any workload recently though, or any correctness issue, perhaps the round trip of do_wp_page wasn't that bad. It should still catch the reuse folios, just more costly than doing things in-place.

Right, do_wp_page() handles it, after the page was mapped. It adds some overhead, but fortunately no TLB flush if we're just upgrading write permissions.

The optimization dates back to pre PageAnonExclusive handling.

Quoted 39 lines

I think we should restore the original check first. We might also want to avoid dropping the swap cache if the folio will not be reused, which was discussed here:

https://lore.kernel.org/linux-mm/CAMgjq7BDfvNXdWH0cqarsujjUn3i3tDDhDkmSg01TR4h-tDorQ@mail.gmail.com/

Maybe extracting some common part into a helper can help make this cleaner.

Quoted 29 lines

So it seems we need a patch before patch 4 to fix the missed swapcache freeing on write faults, because the write bit has already been cleared by the time the folio is mapped and reused?

mm/memory.c +1 -0

index 14577c67c61a..e8fc5a86f6ea 100644
@@ -4752,6 +4752,7 @@ static void check_swap_exclusive(struct folio
 *folio, swp_entry_t entry,
  */
 vm_fault_t do_swap_page(struct vm_fault *vmf)
 {
+       enum fault_flag fault_flags = vmf->flags;
        struct vm_area_struct *vma = vmf->vma;
        struct folio *swapcache = NULL, *folio;

        struct page *page;

mm/memory.c +1 -1

@@ -5091,7 +5092,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
         * an additional reference to the folio.
         */
        if (should_try_to_free_swap(si, folio, vma, nr_pages +
-                       !folio_test_lru(folio), vmf->flags))
+                       !folio_test_lru(folio), fault_flags))
                folio_free_swap(folio);
 
        folio_unlock(folio);

Quoted 7 lines

Quoted 2 lines

Best Regards
Barry

Hi Barry,

The problem is more than that, the exclusive || folio_ref_count(folio) == 1 in do_swap_page is also ineffective now.

Exactly.

If the roundtrip through do_wp_page() is good enough today, we can just do

mm/memory.c +2 -10

@@ -4512,7 +4516,6 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf)
 static inline bool should_try_to_free_swap(struct swap_info_struct *si,
                                           struct folio *folio,
                                           struct vm_area_struct *vma,
-                                          unsigned int extra_refs,
                                           unsigned int fault_flags)
 {
        if (!folio_test_swapcache(folio))

@@ -4528,14 +4531,7 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
        if (mem_cgroup_swap_full(folio) || (vma->vm_flags & VM_LOCKED) ||
            folio_test_mlocked(folio))
                return true;
-       /*
-        * If we want to map a page that's in the swapcache writable, we
-        * have to detect via the refcount if we're really the exclusive
-        * user. Try freeing the swapcache to get rid of the swapcache
-        * reference only in case it's likely that we'll be the exclusive user.
-        */
-       return (fault_flags & FAULT_FLAG_WRITE) && !folio_test_ksm(folio) &&
-               folio_ref_count(folio) == (extra_refs + folio_nr_pages(folio));
+       return false;
 }
 
 static vm_fault_t pte_marker_clear(struct vm_fault *vmf)

@@ -5095,7 +5091,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
         * Do it after mapping, so raced page faults will likely see the folio
         * in swap cache and wait on the folio lock.
         */
-       if (should_try_to_free_swap(si, folio, vma, nr_pages, vmf->flags))
+       if (should_try_to_free_swap(si, folio, vma, vmf->flags))
                folio_free_swap(folio);
 
        folio_unlock(folio);

But then, one question is whether we'd actually want to try removing the swapcache when we mapped the page writable (iow: exclusive)?

mm/memory.c +1 -2

@@ -4512,7 +4516,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf)
 static inline bool should_try_to_free_swap(struct swap_info_struct *si,
                                           struct folio *folio,
                                           struct vm_area_struct *vma,
-                                          unsigned int extra_refs,
+                                          bool exclusive,
                                           unsigned int fault_flags)
 {
        if (!folio_test_swapcache(folio))

@@ -4529,13 +4533,11 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
            folio_test_mlocked(folio))
                return true;
        /*
-        * If we want to map a page that's in the swapcache writable, we

Quoted 78 lines · David Hildenbrand (Arm)

Quoted 6 lines

That's awkward, my bad, terribly sorry about this :(

I sincerely apologize for the oversight and the trouble this has caused.

All good, I was just surprised to see a previous optimization partially reverted without a clear reasoning :)

Because it should have removed the handling in should_try_to_free_swap() as well.

It's good that we are discussing it now!

Quoted 4 lines

I haven't seen any performance regression in any workload recently though, or any correctness issue, perhaps the round trip of do_wp_page wasn't that bad. It should still catch the reuse folios, just more costly than doing things in-place.

Right, do_wp_page() handles it, after the page was mapped. It adds some overhead, but fortunately no TLB flush if we're just upgrading write permissions.

The optimization dates back to pre PageAnonExclusive handling.

Quoted 7 lines

I think we should restore the original check first. We might also want to avoid dropping the swap cache if the folio will not be reused, which was discussed here:

https://lore.kernel.org/linux-mm/CAMgjq7BDfvNXdWH0cqarsujjUn3i3tDDhDkmSg01TR4h-tDorQ@mail.gmail.com/

Quoted 8 lines

Maybe extracting some common part into a helper can help make this cleaner.

Hi Barry,

The problem is more than that, the exclusive || folio_ref_count(folio) == 1 in do_swap_page is also ineffective now.

Exactly.

If the roundtrip through do_wp_page() is good enough today, we can just do

(quoted diff) +2 -10

@@ -4512,7 +4516,6 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf)
 static inline bool should_try_to_free_swap(struct swap_info_struct *si,
                                           struct folio *folio,
                                           struct vm_area_struct *vma,
-                                          unsigned int extra_refs,
                                           unsigned int fault_flags)
 {
        if (!folio_test_swapcache(folio))

@@ -4528,14 +4531,7 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
        if (mem_cgroup_swap_full(folio) || (vma->vm_flags & VM_LOCKED) ||
            folio_test_mlocked(folio))
                return true;
-       /*
-        * If we want to map a page that's in the swapcache writable, we
-        * have to detect via the refcount if we're really the exclusive
-        * user. Try freeing the swapcache to get rid of the swapcache
-        * reference only in case it's likely that we'll be the exclusive user.
-        */
-       return (fault_flags & FAULT_FLAG_WRITE) && !folio_test_ksm(folio) &&
-               folio_ref_count(folio) == (extra_refs + folio_nr_pages(folio));
+       return false;
 }
 
 static vm_fault_t pte_marker_clear(struct vm_fault *vmf)

@@ -5095,7 +5091,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
         * Do it after mapping, so raced page faults will likely see the folio
         * in swap cache and wait on the folio lock.
         */
-       if (should_try_to_free_swap(si, folio, vma, nr_pages, vmf->flags))
+       if (should_try_to_free_swap(si, folio, vma, vmf->flags))
                folio_free_swap(folio);
 
        folio_unlock(folio);

Quoted 2 lines

But then, one question is whether we'd actually want to try removing the swapcache when we mapped the page writable (iow: exclusive)?

I guess this question comes from my earlier commit c18160dba5ff ("mm: swap: reuse exclusive folio directly instead of wp page faults"), where the folio is reused even for READ faults. In that case, we would miss do_wp_page(), which could later drop the swapcache? Also, again, nobody has reported any regression for this.

Holding swapcache for a clean folio for non-sync swap I/O has the benefit of avoiding a potential pageout(). Now, reuse even for read faults and always dropping swapcache seems to somewhat defeat that benefit. On the other hand, we always drop swapcache for sync I/O to avoid the copy in zRAM or elsewhere consuming memory, so it seems safe enough to always enable reuse in do_swap_page() for sync I/O.

Quoted 21 lines

(quoted diff) +2 -5

@@ -4512,7 +4516,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf)
 static inline bool should_try_to_free_swap(struct swap_info_struct *si,
                                           struct folio *folio,
                                           struct vm_area_struct *vma,
-                                          unsigned int extra_refs,
+                                          bool exclusive,
                                           unsigned int fault_flags)
 {
        if (!folio_test_swapcache(folio))

@@ -4529,13 +4533,11 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
            folio_test_mlocked(folio))
                return true;
        /*
-        * If we want to map a page that's in the swapcache writable, we
-        * have to detect via the refcount if we're really the exclusive
-        * user. Try freeing the swapcache to get rid of the swapcache
-        * reference only in case it's likely that we'll be the exclusive user.
+        * We have an exclusive page that was mapped writable or will soon

Review static inline bool should_try_to_free_swap(struct swap_info_struct *si, · line 4540

Quoted 6 lines

(quoted diff) +3 -2

+        * be mapped writable (as we are in a write fault). Let's just try
+        * to reclaim swap immediately.
         */
-       return (fault_flags & FAULT_FLAG_WRITE) && !folio_test_ksm(folio) &&
-               folio_ref_count(folio) == (extra_refs + folio_nr_pages(folio));
+       return (fault_flags & FAULT_FLAG_WRITE) && exclusive;

I assume you mean (fault_flags & FAULT_FLAG_WRITE) || exclusive, or

we can just remove (fault_flags & FAULT_FLAG_WRITE) and use "exclusive" instead since we are always using do_wp_page() now,

and FAULT_FLAG_WRITE in fault_flags could have been cleared by the reuse of do_swap_page().

On Tue, Jun 30, 2026 at 1:48 PM David Hildenbrand (Arm) <[email protected]> wrote:
[...]

mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5116

Quoted 7 lines

mm/memory.c +1 -1

-       if (vmf->flags & FAULT_FLAG_WRITE) {
+       if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
                ret |= do_wp_page(vmf);
                if (ret & VM_FAULT_ERROR)
                        ret &= VM_FAULT_ERROR;

Thanks! I'm working on a v3, but one thing still seems a bit odd to me.

In patch 1, we're removing the local LRU drain from do_wp_page(). But in patch 4, do_swap_page() calls do_wp_page() for folios without the "exclusive" hint, since folios with the hint have already been reused in do_swap_page().

This can happen when we allocate a new folio whose swap entry lacks the exclusive flag, even though the folio is still reusable. For example, in the scenario I mentioned in patch 1, the swap count is effectively 1, but there is no exclusive flag:

#include <sys/mman.h>

int main(int argc, char *argv[])

{
    int i;
    #define SIZE 30*1024*1024
    while(1) {
        volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
        volatile int q;

        for (int i = 0; i < SIZE/sizeof(int); i++)
              p[i] =  i;
        madvise((void *)p, SIZE, MADV_PAGEOUT);
        if (!fork())
             _exit(0);
        for (int i = 0; i < SIZE/sizeof(int); i++)
              q = p[i];
        for (int i = 0; i < SIZE/sizeof(int); i++)
              p[i] =  i;
        munmap(p, SIZE);
    }
    return 0;
}

The newly allocated folio will likely still be sitting in this CPU's local LRU cache. If we drop the drain in patch 1, it seems there is little point in calling do_wp_page() from do_swap_page(), since we won't be able to reuse the folio while it remains in the local LRU cache.

So I'm wondering whether we should keep the drain in patch 1 by checking for a refcount of 3, or instead do something like the following in patch 4:

        if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
                 lru_add_drain();
                 ret |= do_wp_page(vmf);
                 if (ret & VM_FAULT_ERROR)
                         ret &= VM_FAULT_ERROR;
       }

Otherwise, the combination of patches 1 and 4 seems somewhat inconsistent. It feels like a strange mix of sweet, spicy, hot, and cold—kind of contradictory :-)

Quoted 6 lines

The newly allocated folio will likely still be sitting in this CPU's local LRU cache. If we drop the drain in patch 1, it seems there is little point in calling do_wp_page() from do_swap_page(), since we won't be able to reuse the folio while it remains in the local LRU cache.

Well, we call do_wp_page() here to avoid another page fault where we would end up in do_wp_page() and actually break COW.

Quoted 14 lines

So I'm wondering whether we should keep the drain in patch 1 by checking for a refcount of 3, or instead do something like the following in patch 4:

        if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
                 lru_add_drain();
                 ret |= do_wp_page(vmf);
                 if (ret & VM_FAULT_ERROR)
                         ret &= VM_FAULT_ERROR;
       }

Quoted 3 lines

Otherwise, the combination of patches 1 and 4 seems somewhat inconsistent. It feels like a strange mix of sweet, spicy, hot, and cold—kind of contradictory :-)

Yes, this needs a proper thought.

What you describe is relevant for

e.g.,

swapout
fork -> exclusive bit cleared
child exit -> parent is single user
swapin -> added to swapcache and LRU cache

This is not very odd, so we should keep that working (I think I have micro-benchmarks for that).

On Wed, Jul 1, 2026 at 4:18 PM David Hildenbrand (Arm) <[email protected]> wrote:
[...]

Quoted 9 lines

Quoted 6 lines

The newly allocated folio will likely still be sitting in this CPU's local LRU cache. If we drop the drain in patch 1, it seems there is little point in calling do_wp_page() from do_swap_page(), since we won't be able to reuse the folio while it remains in the local LRU cache.

Well, we call do_wp_page() here to avoid another page fault where we would end up in do_wp_page() and actually break COW.

Yep. We still need do_wp_page() to handle CoW, but we could actually reuse the folio if we don't drop the LRU drain.

Quoted 30 lines

Quoted 14 lines

So I'm wondering whether we should keep the drain in patch 1 by checking for a refcount of 3, or instead do something like the following in patch 4:

        if ((vmf->flags & FAULT_FLAG_WRITE) && !pte_write(pte)) {
                 lru_add_drain();
                 ret |= do_wp_page(vmf);
                 if (ret & VM_FAULT_ERROR)
                         ret &= VM_FAULT_ERROR;
       }

Quoted 3 lines

Otherwise, the combination of patches 1 and 4 seems somewhat inconsistent. It feels like a strange mix of sweet, spicy, hot, and cold—kind of contradictory :-)

Yes, this needs a proper thought.

What you describe is relevant for

e.g.,

swapout
fork -> exclusive bit cleared
child exit -> parent is single user
swapin -> added to swapcache and LRU cache

Quoted 2 lines

This is not very odd, so we should keep that working (I think I have micro-benchmarks for that).

Maybe the simplest approach is to keep the drain in patch 1 and just make the code improvements you suggested in [1]?