All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths
@ 2026-07-01 23:59 Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio() Barry Song (Xiaomi)
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Barry Song (Xiaomi) @ 2026-07-01 23:59 UTC (permalink / raw)
  To: akpm, linux-mm
  Cc: baoquan.he, chrisl, david, jp.kobryn, kasong, liam, linux-kernel,
	ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park, Barry Song (Xiaomi)

We are doing a large number of redundant lru_add_drain() calls in
both wp_can_reuse_anon_folio() and do_swap_page(), leading to LRU
lock contention and unnecessary overhead.

In wp_can_reuse_anon_folio(), we can check the refcount against the
lru_cache before deciding to drain. In do_swap_page(), the drain is
now entirely redundant after Kairui's work to route SYNC I/O through
the swapcache in the same way as ASYNC I/O.

Build the kernel within a 1 GB memcg using 20 threads with zRAM swap.
The number of lru_add_drain() calls is reduced from 276,278 to
226,318, a reduction of about 18%.

Build the kernel within an 800 MB memcg using 20 threads with zRAM
swap. The number of lru_add_drain() calls is reduced from 778,950
to 541,149, a reduction of 30.5%.

-v3:
 * code improvements for patch1/4 and patch2/4 as suggested by
   David;
 * improve changelog as suggested by Shakeel and David;
 * collect the reviewed-by and acked-by tags;
 * for patch4, remove all dependency for refcount regarding
   swapcache free in do_swap_page, as suggested by David

-v2:
 https://lore.kernel.org/linux-mm/20260623231635.43086-1-baohua@kernel.org/
 * collect the reviewed-by and acked-by tags from Usama, Baoquan,
   Shakeel, Kairui, thanks!
 * add patch4 to free swapcache for non-LRU folios, as suggested
   by Kairui, thanks!

-RFC:
 https://lore.kernel.org/linux-mm/20260611105124.98668-1-baohua@kernel.org/

Barry Song (Xiaomi) (4):
  mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  mm: entirely remove lru_add_drain in do_swap_page
  mm: clarify the folio_free_swap() for do_swap_page()

 mm/memory.c | 49 ++++++++++++++++++-------------------------------
 1 file changed, 18 insertions(+), 31 deletions(-)

-- 
2.39.3 (Apple Git-146)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  2026-07-01 23:59 [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths Barry Song (Xiaomi)
@ 2026-07-01 23:59 ` Barry Song (Xiaomi)
  2026-07-02  8:05   ` David Hildenbrand (Arm)
  2026-07-01 23:59 ` [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic Barry Song (Xiaomi)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Barry Song (Xiaomi) @ 2026-07-01 23:59 UTC (permalink / raw)
  To: akpm, linux-mm
  Cc: baoquan.he, chrisl, david, jp.kobryn, kasong, liam, linux-kernel,
	ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park, Barry Song (Xiaomi)

There is a case where `folio_ref_count(folio) == 3` and
`!folio_test_swapcache(folio)`. In that case, both
`folio_ref_count(folio) > 3` and
`folio_ref_count(folio) > 1 + folio_test_swapcache(folio)` evaluate
false, causing an unnecessary local LRU drain.

During an Ubuntu boot, I observed over 5,000 redundant local LRU
drains. For a kernel build with a minimal configuration, I observed
more than 20,000 redundant drains.

Fix this by checking against: `1 + in_swapcache + in_lrucache`
instead of hardcoding `folio_ref_count(folio) > 3`.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Kairui Song <kasong@tencent.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Reviewed-by: Baoquan He <baoquan.he@linux.dev>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/memory.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..87da78eb1abd 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;
-- 
2.39.3 (Apple Git-146)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  2026-07-01 23:59 [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio() Barry Song (Xiaomi)
@ 2026-07-01 23:59 ` Barry Song (Xiaomi)
  2026-07-02  8:07   ` David Hildenbrand (Arm)
  2026-07-02  8:14   ` David Hildenbrand (Arm)
  2026-07-01 23:59 ` [PATCH v3 3/4] mm: entirely remove lru_add_drain in do_swap_page Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page() Barry Song (Xiaomi)
  3 siblings, 2 replies; 14+ messages in thread
From: Barry Song (Xiaomi) @ 2026-07-01 23:59 UTC (permalink / raw)
  To: akpm, linux-mm
  Cc: baoquan.he, chrisl, david, jp.kobryn, kasong, liam, linux-kernel,
	ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park, 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/20251220-swap-table-p2-v5-0-8862a265a033@tencent.com/

Acked-by: Usama Arif <usama.arif@linux.dev>
Reviewed-by: Kairui Song <kasong@tencent.com>
Reviewed-by: Baoquan He <baoquan.he@linux.dev>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/memory.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 87da78eb1abd..71e9d394816b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5046,13 +5046,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		pte = pte_mkuffd_wp(pte);
 
 	/*
-	 * Same logic as in do_wp_page(); however, optimize for pages that are
-	 * certainly not shared either because we just allocated them without
-	 * exposing them to the swapcache or because the swap entry indicates
-	 * exclusivity.
+	 * Similar logic as in do_wp_page(); however, optimize for pages that are
+	 * certainly not because the swap entry indicates exclusivity.
 	 */
-	if (!folio_test_ksm(folio) &&
-	    (exclusive || folio_ref_count(folio) == 1)) {
+	if (exclusive) {
 		if ((vma->vm_flags & VM_WRITE) && !userfaultfd_pte_wp(vma, pte) &&
 		    !pte_needs_soft_dirty_wp(vma, pte)) {
 			pte = pte_mkwrite(pte, vma);
-- 
2.39.3 (Apple Git-146)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/4] mm: entirely remove lru_add_drain in do_swap_page
  2026-07-01 23:59 [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio() Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic Barry Song (Xiaomi)
@ 2026-07-01 23:59 ` Barry Song (Xiaomi)
  2026-07-01 23:59 ` [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page() Barry Song (Xiaomi)
  3 siblings, 0 replies; 14+ messages in thread
From: Barry Song (Xiaomi) @ 2026-07-01 23:59 UTC (permalink / raw)
  To: akpm, linux-mm
  Cc: baoquan.he, chrisl, david, jp.kobryn, kasong, liam, linux-kernel,
	ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park, Barry Song (Xiaomi)

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.
For a kernel build with a minimal configuration running in a 1 GB
memcg, this patch skips more than 43,000 redundant local LRU drains.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Reviewed-by: Baoquan He <baoquan.he@linux.dev>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/memory.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 71e9d394816b..4665405ace5a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4901,16 +4901,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)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page()
  2026-07-01 23:59 [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths Barry Song (Xiaomi)
                   ` (2 preceding siblings ...)
  2026-07-01 23:59 ` [PATCH v3 3/4] mm: entirely remove lru_add_drain in do_swap_page Barry Song (Xiaomi)
@ 2026-07-01 23:59 ` Barry Song (Xiaomi)
  2026-07-02  8:10   ` David Hildenbrand (Arm)
  3 siblings, 1 reply; 14+ messages in thread
From: Barry Song (Xiaomi) @ 2026-07-01 23:59 UTC (permalink / raw)
  To: akpm, linux-mm
  Cc: baoquan.he, chrisl, david, jp.kobryn, kasong, liam, linux-kernel,
	ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park, Barry Song (Xiaomi)

Since commit 4b34f1d82c654 ("mm, swap: free the swap cache after
folio is mapped"), we have relied on do_wp_page() to handle the
non-exclusive case, where the folio may either be reused or require
CoW.

As a result, using the refcount in do_swap_page() to decide when to
free the swap cache is no longer necessary, since do_wp_page() can
handle this more cleanly and consistently.

We can now simply use FAULT_FLAG_WRITE together with exclusivity to
decide when to free the swap cache in do_swap_page().

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
---
 mm/memory.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 4665405ace5a..a29cd38ad547 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4516,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))
@@ -4532,14 +4532,12 @@ 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.
+	 * Free the swapcache only if we are the exclusive user and
+	 * this is a write fault.
 	 */
-	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;
 }
 
 static vm_fault_t pte_marker_clear(struct vm_fault *vmf)
@@ -5043,10 +5041,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		if ((vma->vm_flags & VM_WRITE) && !userfaultfd_pte_wp(vma, pte) &&
 		    !pte_needs_soft_dirty_wp(vma, pte)) {
 			pte = pte_mkwrite(pte, vma);
-			if (vmf->flags & FAULT_FLAG_WRITE) {
+			if (vmf->flags & FAULT_FLAG_WRITE)
 				pte = pte_mkdirty(pte);
-				vmf->flags &= ~FAULT_FLAG_WRITE;
-			}
 		}
 		rmap_flags |= RMAP_EXCLUSIVE;
 	}
@@ -5086,7 +5082,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, exclusive, vmf->flags))
 		folio_free_swap(folio);
 
 	folio_unlock(folio);
@@ -5103,7 +5099,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		folio_put(swapcache);
 	}
 
-	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;
-- 
2.39.3 (Apple Git-146)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  2026-07-01 23:59 ` [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio() Barry Song (Xiaomi)
@ 2026-07-02  8:05   ` David Hildenbrand (Arm)
  2026-07-06  8:18     ` Barry Song
  0 siblings, 1 reply; 14+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-02  8:05 UTC (permalink / raw)
  To: Barry Song (Xiaomi), akpm, linux-mm
  Cc: baoquan.he, chrisl, jp.kobryn, kasong, liam, linux-kernel, ljs,
	mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park

On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> There is a case where `folio_ref_count(folio) == 3` and
> `!folio_test_swapcache(folio)`. In that case, both
> `folio_ref_count(folio) > 3` and
> `folio_ref_count(folio) > 1 + folio_test_swapcache(folio)` evaluate
> false, causing an unnecessary local LRU drain.
> 
> During an Ubuntu boot, I observed over 5,000 redundant local LRU
> drains. For a kernel build with a minimal configuration, I observed
> more than 20,000 redundant drains.
> 
> Fix this by checking against: `1 + in_swapcache + in_lrucache`
> instead of hardcoding `folio_ref_count(folio) > 3`.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Reviewed-by: Kairui Song <kasong@tencent.com>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
The other folks should probably re-review this patch that changed quite a bit :)


Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  2026-07-01 23:59 ` [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic Barry Song (Xiaomi)
@ 2026-07-02  8:07   ` David Hildenbrand (Arm)
  2026-07-02  8:14   ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 14+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-02  8:07 UTC (permalink / raw)
  To: Barry Song (Xiaomi), akpm, linux-mm
  Cc: baoquan.he, chrisl, jp.kobryn, kasong, liam, linux-kernel, ljs,
	mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park

On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> 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/20251220-swap-table-p2-v5-0-8862a265a033@tencent.com/
> 
> Acked-by: Usama Arif <usama.arif@linux.dev>
> Reviewed-by: Kairui Song <kasong@tencent.com>
> Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page()
  2026-07-01 23:59 ` [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page() Barry Song (Xiaomi)
@ 2026-07-02  8:10   ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 14+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-02  8:10 UTC (permalink / raw)
  To: Barry Song (Xiaomi), akpm, linux-mm
  Cc: baoquan.he, chrisl, jp.kobryn, kasong, liam, linux-kernel, ljs,
	mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park

On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> Since commit 4b34f1d82c654 ("mm, swap: free the swap cache after
> folio is mapped"), we have relied on do_wp_page() to handle the
> non-exclusive case, where the folio may either be reused or require
> CoW.
> 
> As a result, using the refcount in do_swap_page() to decide when to
> free the swap cache is no longer necessary, since do_wp_page() can
> handle this more cleanly and consistently.
> 
> We can now simply use FAULT_FLAG_WRITE together with exclusivity to
> decide when to free the swap cache in do_swap_page().
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  2026-07-01 23:59 ` [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic Barry Song (Xiaomi)
  2026-07-02  8:07   ` David Hildenbrand (Arm)
@ 2026-07-02  8:14   ` David Hildenbrand (Arm)
  2026-07-06  8:14     ` Barry Song
  2026-07-07 11:08     ` Barry Song
  1 sibling, 2 replies; 14+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-02  8:14 UTC (permalink / raw)
  To: Barry Song (Xiaomi), akpm, linux-mm
  Cc: baoquan.he, chrisl, jp.kobryn, kasong, liam, linux-kernel, ljs,
	mhocko, nphamcs, rppt, shakeel.butt, shikemeng, surenb,
	usama.arif, vbabka, youngjun.park

On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> 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.

Sashiko points out two minor things (one flagged as medium, lol, sure sure).

Here, you can clarify that folio_ref_count()==1 is true for freshly allocated
pages (due to the KSM check) in which case exclusive=true already.

> 
> [1] https://lore.kernel.org/all/20251220-swap-table-p2-v5-0-8862a265a033@tencent.com/
> 
> Acked-by: Usama Arif <usama.arif@linux.dev>
> Reviewed-by: Kairui Song <kasong@tencent.com>
> Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> ---
>  mm/memory.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 87da78eb1abd..71e9d394816b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5046,13 +5046,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  		pte = pte_mkuffd_wp(pte);
>  
>  	/*
> -	 * Same logic as in do_wp_page(); however, optimize for pages that are
> -	 * certainly not shared either because we just allocated them without
> -	 * exposing them to the swapcache or because the swap entry indicates
> -	 * exclusivity.
> +	 * Similar logic as in do_wp_page(); however, optimize for pages that are
> +	 * certainly not because the swap entry indicates exclusivity.

s/not/not shared/

but likely you should just simplify to "... pages that are certainly exclusive."

>  	 */
> -	if (!folio_test_ksm(folio) &&
> -	    (exclusive || folio_ref_count(folio) == 1)) {
> +	if (exclusive) {
>  		if ((vma->vm_flags & VM_WRITE) && !userfaultfd_pte_wp(vma, pte) &&
>  		    !pte_needs_soft_dirty_wp(vma, pte)) {
>  			pte = pte_mkwrite(pte, vma);


-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  2026-07-02  8:14   ` David Hildenbrand (Arm)
@ 2026-07-06  8:14     ` Barry Song
  2026-07-07 11:08     ` Barry Song
  1 sibling, 0 replies; 14+ messages in thread
From: Barry Song @ 2026-07-06  8:14 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: akpm, linux-mm, baoquan.he, chrisl, jp.kobryn, kasong, liam,
	linux-kernel, ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng,
	surenb, usama.arif, vbabka, youngjun.park

On Thu, Jul 2, 2026 at 4:14 PM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> > 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.
>
> Sashiko points out two minor things (one flagged as medium, lol, sure sure).
>
> Here, you can clarify that folio_ref_count()==1 is true for freshly allocated
> pages (due to the KSM check) in which case exclusive=true already.

Thanks. Sashiko's comment below seems valid. While there's no
need to change the code, the changelog should be updated to
reflect it. I'll do that in v4.

"Is the claim that folio_ref_count() should never be 1 in this path accurate?
When ksm_might_need_to_copy() allocates a fresh folio to break KSM sharing,
the new folio bypasses the swap cache:
do_swap_page() {
 ...
 if (!folio_test_ksm(folio)) {
   ...
 if (folio != swapcache) {
    /*
     * We have a fresh page that is not exposed to the
     * swapcache -> certainly exclusive.
     */
  exclusive = true;
  }
  ...
}
In this specific case, the folio refcount would still be exactly 1 when it
reaches this path.
While the code change itself is safe because the KSM copy path explicitly
sets exclusive = true earlier (satisfying the new if (exclusive) check
without needing the refcount check), the rationale in the commit message
seems to overlook this scenario."

>
> >
> > [1] https://lore.kernel.org/all/20251220-swap-table-p2-v5-0-8862a265a033@tencent.com/
> >
> > Acked-by: Usama Arif <usama.arif@linux.dev>
> > Reviewed-by: Kairui Song <kasong@tencent.com>
> > Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > ---
> >  mm/memory.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 87da78eb1abd..71e9d394816b 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -5046,13 +5046,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >               pte = pte_mkuffd_wp(pte);
> >
> >       /*
> > -      * Same logic as in do_wp_page(); however, optimize for pages that are
> > -      * certainly not shared either because we just allocated them without
> > -      * exposing them to the swapcache or because the swap entry indicates
> > -      * exclusivity.
> > +      * Similar logic as in do_wp_page(); however, optimize for pages that are
> > +      * certainly not because the swap entry indicates exclusivity.
>
> s/not/not shared/
>
> but likely you should just simplify to "... pages that are certainly exclusive."

Thanks. I will fix the doc in v4.

Best Regards
Barry


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  2026-07-02  8:05   ` David Hildenbrand (Arm)
@ 2026-07-06  8:18     ` Barry Song
  2026-07-06  9:12       ` Baoquan He
  0 siblings, 1 reply; 14+ messages in thread
From: Barry Song @ 2026-07-06  8:18 UTC (permalink / raw)
  To: David Hildenbrand (Arm), baoquan.he, kasong, shakeel.butt
  Cc: akpm, linux-mm, chrisl, jp.kobryn, liam, linux-kernel, ljs,
	mhocko, nphamcs, rppt, shikemeng, surenb, usama.arif, vbabka,
	youngjun.park

On Thu, Jul 2, 2026 at 4:05 PM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> > There is a case where `folio_ref_count(folio) == 3` and
> > `!folio_test_swapcache(folio)`. In that case, both
> > `folio_ref_count(folio) > 3` and
> > `folio_ref_count(folio) > 1 + folio_test_swapcache(folio)` evaluate
> > false, causing an unnecessary local LRU drain.
> >
> > During an Ubuntu boot, I observed over 5,000 redundant local LRU
> > drains. For a kernel build with a minimal configuration, I observed
> > more than 20,000 redundant drains.
> >
> > Fix this by checking against: `1 + in_swapcache + in_lrucache`
> > instead of hardcoding `folio_ref_count(folio) > 3`.
> >
> > Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> > Reviewed-by: Kairui Song <kasong@tencent.com>
> > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> > Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > ---
> The other folks should probably re-review this patch that changed quite a bit :)
>
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

Thanks!

Hi Baoquan, Shakeel, and Kairui,

Would you like to re-review the patches so I can re-collect your
tags? If not, are you okay with me removing your tags when I
send v4?

The concept hasn't changed since v2, but the code readability has
improved quite a bit based on David's suggestions.

Best Regards
Barry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  2026-07-06  8:18     ` Barry Song
@ 2026-07-06  9:12       ` Baoquan He
  2026-07-06  9:17         ` Barry Song
  0 siblings, 1 reply; 14+ messages in thread
From: Baoquan He @ 2026-07-06  9:12 UTC (permalink / raw)
  To: Barry Song
  Cc: David Hildenbrand (Arm), kasong, shakeel.butt, akpm, linux-mm,
	chrisl, jp.kobryn, liam, linux-kernel, ljs, mhocko, nphamcs, rppt,
	shikemeng, surenb, usama.arif, vbabka, youngjun.park

Hi Barry,

On 07/06/26 at 04:18pm, Barry Song wrote:
> On Thu, Jul 2, 2026 at 4:05 PM David Hildenbrand (Arm) <david@kernel.org> wrote:
> >
> > On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> > > There is a case where `folio_ref_count(folio) == 3` and
> > > `!folio_test_swapcache(folio)`. In that case, both
> > > `folio_ref_count(folio) > 3` and
> > > `folio_ref_count(folio) > 1 + folio_test_swapcache(folio)` evaluate
> > > false, causing an unnecessary local LRU drain.
> > >
> > > During an Ubuntu boot, I observed over 5,000 redundant local LRU
> > > drains. For a kernel build with a minimal configuration, I observed
> > > more than 20,000 redundant drains.
> > >
> > > Fix this by checking against: `1 + in_swapcache + in_lrucache`
> > > instead of hardcoding `folio_ref_count(folio) > 3`.
> > >
> > > Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> > > Reviewed-by: Kairui Song <kasong@tencent.com>
> > > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> > > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > > ---
> > The other folks should probably re-review this patch that changed quite a bit :)
> >
> >
> > Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> 
> Thanks!
> 
> Hi Baoquan, Shakeel, and Kairui,
> 
> Would you like to re-review the patches so I can re-collect your
> tags? If not, are you okay with me removing your tags when I
> send v4?

Is there change you need to do in a new version? If yes, please remove
my tags, I will review v4. Oterwise, I will check this v3. Thanks.

AFAIK, usually, if patch is updated in a new version, any reviewing tags
from the old version should be dropped unless the change is minor or
trivial.

Thanks
Baoquan

> 
> The concept hasn't changed since v2, but the code readability has
> improved quite a bit based on David's suggestions.
> 
> Best Regards
> Barry


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
  2026-07-06  9:12       ` Baoquan He
@ 2026-07-06  9:17         ` Barry Song
  0 siblings, 0 replies; 14+ messages in thread
From: Barry Song @ 2026-07-06  9:17 UTC (permalink / raw)
  To: Baoquan He
  Cc: David Hildenbrand (Arm), kasong, shakeel.butt, akpm, linux-mm,
	chrisl, jp.kobryn, liam, linux-kernel, ljs, mhocko, nphamcs, rppt,
	shikemeng, surenb, usama.arif, vbabka, youngjun.park

On Mon, Jul 6, 2026 at 5:13 PM Baoquan He <baoquan.he@linux.dev> wrote:
>
> Hi Barry,
>
> On 07/06/26 at 04:18pm, Barry Song wrote:
> > On Thu, Jul 2, 2026 at 4:05 PM David Hildenbrand (Arm) <david@kernel.org> wrote:
> > >
> > > On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> > > > There is a case where `folio_ref_count(folio) == 3` and
> > > > `!folio_test_swapcache(folio)`. In that case, both
> > > > `folio_ref_count(folio) > 3` and
> > > > `folio_ref_count(folio) > 1 + folio_test_swapcache(folio)` evaluate
> > > > false, causing an unnecessary local LRU drain.
> > > >
> > > > During an Ubuntu boot, I observed over 5,000 redundant local LRU
> > > > drains. For a kernel build with a minimal configuration, I observed
> > > > more than 20,000 redundant drains.
> > > >
> > > > Fix this by checking against: `1 + in_swapcache + in_lrucache`
> > > > instead of hardcoding `folio_ref_count(folio) > 3`.
> > > >
> > > > Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> > > > Reviewed-by: Kairui Song <kasong@tencent.com>
> > > > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > > Reviewed-by: Baoquan He <baoquan.he@linux.dev>
> > > > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > > > ---
> > > The other folks should probably re-review this patch that changed quite a bit :)
> > >
> > >
> > > Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> >
> > Thanks!
> >
> > Hi Baoquan, Shakeel, and Kairui,
> >
> > Would you like to re-review the patches so I can re-collect your
> > tags? If not, are you okay with me removing your tags when I
> > send v4?
>
> Is there change you need to do in a new version? If yes, please remove
> my tags, I will review v4. Oterwise, I will check this v3. Thanks.

no change for v4. Please feel free to re-review v3.

>
> AFAIK, usually, if patch is updated in a new version, any reviewing tags
> from the old version should be dropped unless the change is minor or
> trivial.

Thanks for the clarification. make sense to me.

Best Regards
Barry


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
  2026-07-02  8:14   ` David Hildenbrand (Arm)
  2026-07-06  8:14     ` Barry Song
@ 2026-07-07 11:08     ` Barry Song
  1 sibling, 0 replies; 14+ messages in thread
From: Barry Song @ 2026-07-07 11:08 UTC (permalink / raw)
  To: David Hildenbrand (Arm), akpm
  Cc: linux-mm, baoquan.he, chrisl, jp.kobryn, kasong, liam,
	linux-kernel, ljs, mhocko, nphamcs, rppt, shakeel.butt, shikemeng,
	surenb, usama.arif, vbabka, youngjun.park

On Thu, Jul 2, 2026 at 4:14 PM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 7/2/26 01:59, Barry Song (Xiaomi) wrote:
> > 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.
>
> Sashiko points out two minor things (one flagged as medium, lol, sure sure).
>
> Here, you can clarify that folio_ref_count()==1 is true for freshly allocated
> pages (due to the KSM check) in which case exclusive=true already.

Hi Andrew,

I saw that you've queued this in mm-new. Thanks very much for that!

Would you mind adding the following sentences to the end of the changelog?

The ksm_might_need_to_copy() check may allocate a fresh folio with
folio_ref_count() == 1. Along that path, exclusive has already been
set to true, so the folio can still be reused correctly.

Best Regards
Barry


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-07-07 11:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 23:59 [PATCH v3 0/4] mm: drop redundant lru_add_drain in anon folio reuse paths Barry Song (Xiaomi)
2026-07-01 23:59 ` [PATCH v3 1/4] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio() Barry Song (Xiaomi)
2026-07-02  8:05   ` David Hildenbrand (Arm)
2026-07-06  8:18     ` Barry Song
2026-07-06  9:12       ` Baoquan He
2026-07-06  9:17         ` Barry Song
2026-07-01 23:59 ` [PATCH v3 2/4] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic Barry Song (Xiaomi)
2026-07-02  8:07   ` David Hildenbrand (Arm)
2026-07-02  8:14   ` David Hildenbrand (Arm)
2026-07-06  8:14     ` Barry Song
2026-07-07 11:08     ` Barry Song
2026-07-01 23:59 ` [PATCH v3 3/4] mm: entirely remove lru_add_drain in do_swap_page Barry Song (Xiaomi)
2026-07-01 23:59 ` [PATCH v3 4/4] mm: clarify the folio_free_swap() for do_swap_page() Barry Song (Xiaomi)
2026-07-02  8:10   ` David Hildenbrand (Arm)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.