mm: drop redundant lru_add_drain in anon folio reuse paths
Subsystems (2)
cover mm: drop redundant lru_add_drain in anon folio reuse paths
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 1GB memcg using 20 threads with zRAM swap. The number of lru_add_drain() calls is reduced from 276,787 to 230,283, while sys time decreases slightly from 3m40.125s to 3m37.128s.
Build the kernel within an 800MB memcg using 20 threads with zRAM swap. The number of lru_add_drain() calls is reduced from 796,661 to 537,262, while sys time decreases slightly from 6m25.981s to 6m22.678s.
Barry Song (Xiaomi) (3):
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/memory.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-)
Signature · 1 line
2.39.3 (Apple Git-146)
RFC PATCH 1/3 mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
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.
--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
mm/memory.c +7 -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;/* * 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))
Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0
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;
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.
Quoted 12 lines
mm/memory.c +1
-0/* * 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))Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 30 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;In your experiments, how much amount of drains were reduced due to this specific check?
Quoted 2 lines
I wonder if that data can motivate to introduce
lru_add_drain_folio(folio)which only drains if the given folio is in the locallru_addcache.
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?
Quoted 13 lines
Quoted 13 lines
mm/memory.c +1
-0/* * 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))Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 31 lines · Shakeel Butt
Quoted 30 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;In your experiments, how much amount of drains were reduced due to this specific check?
Quoted 6 lines
Quoted 2 lines
I wonder if that data can motivate to introduce
lru_add_drain_folio(folio)which only drains if the given folio is in the locallru_addcache.Actually if we can peek into
lru_addcache andfolio_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 inlru_addcache 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.
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
mm/memory.c +1
-0/* * 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))Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 36 lines · Baoquan He
Quoted 34 lines · Shakeel Butt
Quoted 32 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;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 locallru_addcache.
Quoted 8 lines
Quoted 3 lines
Actually if we can peek into
lru_addcache andfolio_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 inlru_addcache 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.
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
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 56 lines · Barry Song
Quoted 36 lines · Baoquan He
Quoted 34 lines · Shakeel Butt
Quoted 32 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;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 locallru_addcache.Quoted 8 lines
Quoted 3 lines
Actually if we can peek into
lru_addcache andfolio_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 inlru_addcache 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 thelru_cache; it does not distinguish between the two. Unless we want to scan thelru_cacheto look up the folio, we may need a dedicated folio flag, similar toPG_lru, to indicatelru_cachemembership -
I guess people would be quite unhappy about adding a new flag?
Quoted 5 lines
On the other hand, the folio may be sitting in the
lru_cacheof 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'slru_cache, giving it roughly a 1/nr_cpus chance of working.drain_allwould 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.
Quoted 7 lines
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
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
In reply to Shakeel ButtQuoted 32 lines · Shakeel Butt
Quoted 30 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;In your experiments, how much amount of drains were reduced due to this specific check?
Quoted 7 lines
Quoted 2 lines
I wonder if that data can motivate to introduce
lru_add_drain_folio(folio)which only drains if the given folio is in the locallru_addcache.Actually if we can peek into
lru_addcache andfolio_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 inlru_addcache without draining, right?
The real problem is that !folio_test_lru(folio) does not tell us whether the folio is still sitting in lru_cache, and determining that is unlikely to be cheap. We could either scan the lru_cache or add a separate flag to indicate lru_cache membership, but neither option sounds particularly appealing.
Thanks
Barry
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 41 lines · Barry Song
Quoted 32 lines · Shakeel Butt
Quoted 30 lines · Shakeel Butt
Quoted 15 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +0
-0mm/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;In your experiments, how much amount of drains were reduced due to this specific check?
Quoted 7 lines
Quoted 2 lines
I wonder if that data can motivate to introduce
lru_add_drain_folio(folio)which only drains if the given folio is in the locallru_addcache.Actually if we can peek into
lru_addcache andfolio_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 inlru_addcache without draining, right?
Quoted 5 lines
The real problem is that !folio_test_lru(folio) does not tell us whether the folio is still sitting in
lru_cache, and determining that is unlikely to be cheap. We could either scan thelru_cacheor add a separate flag to indicatelru_cachemembership, but neither option sounds particularly appealing.
I was thinking of lru_cache scanning. I see this can be 4 cache misses, so might be expensive. Though if we are draining non-full cache most of the time then it might be cheaper. Anyways if you don't mind checking the cost that would be awesome.
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
In reply to Shakeel ButtQuoted 22 lines · Shakeel Butt
Quoted 20 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +1
-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)) {
Quoted 8 lines
mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201
Quoted 5 lines
mm/memory.c +5
-0+/* +* 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?
This is a very good question. I booted the system into Ubuntu, and after boot completed I observed:
wp_reuse_skipped_drain: 5542do_swap_skipped_drain: 0
Then I built the kernel in a 1GB memcg using zRAM swap, and observed:
wp_reuse_skipped_drain: 25017do_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().
Best Regards
Barry
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
Quoted 40 lines · Barry Song
Quoted 22 lines · Shakeel Butt
Quoted 20 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +1
-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)) {Quoted 8 lines
mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4201
Quoted 5 lines
mm/memory.c +5
-0+/* +* 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?
This is a very good question. I booted the system into Ubuntu, and after boot completed I observed:
wp_reuse_skipped_drain: 5542do_swap_skipped_drain: 0Then I built the kernel in a 1GB memcg using zRAM swap, and observed:
Quoted 7 lines
wp_reuse_skipped_drain: 25017do_swap_skipped_drain: 43595So 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 indo_swap_page(), while we still see a substantial number of skipped drains inwp_can_reuse_anon_folio().
Thanks and this is very good info. Add this in the commit message when you send the non-RFC version.
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
In reply to Barry Song (Xiaomi)Quoted 24 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)mm/memory.c +5
-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))
mm/memory.c static bool wp_can_reuse_anon_folio(struct folio *folio, · line 4210
Quoted 10 lines
mm/memory.c +2
-0+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))
Quoted 3 lines
Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 1/3] mm: avoid unnecessary lru drain for wp_can_reuse_anon_folio()
In reply to Barry Song (Xiaomi)Quoted 6 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 inlru_cache, and use the refcount to avoid many unnecessary LRU drains.Signed-off-by Barry Song (Xiaomi) <[email protected]>
RFC PATCH 2/3 mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
In reply to 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]/
--- mm/memory.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
mm/memory.c +2 -5
@@ -5049,12 +5049,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)/* * 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. +* 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) {if ((vma->vm_flags &VM_WRITE ) && !userfaultfd_pte_wp (vma, pte) && !pte_needs_soft_dirty_wp (vma, pte)) { pte =pte_mkwrite (pte, vma);
Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 2/3] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
Quoted 10 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]/
Signed-off-by Barry Song (Xiaomi) <[email protected]>
Re: [RFC PATCH 2/3] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
In reply to Barry Song (Xiaomi)Quoted 23 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]/
Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)mm/memory.c +0
-1@@ -5049,12 +5049,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)/* * Same logic as in do_wp_page(); however, optimize for pages that are -* certainly not shared either because we just allocated them without
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5057
Quoted 10 lines
mm/memory.c +2
-4-* 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) {if ((vma->vm_flags &VM_WRITE ) && !userfaultfd_pte_wp (vma, pte) && !pte_needs_soft_dirty_wp (vma, pte)) { pte =pte_mkwrite (pte, vma);
Quoted 4 lines
Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 2/3] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
In reply to Barry Song (Xiaomi)Quoted 24 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]/
Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)mm/memory.c +0
-1@@ -5049,12 +5049,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)/* * Same logic as in do_wp_page(); however, optimize for pages that are -* certainly not shared either because we just allocated them without
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5057
Quoted 10 lines
mm/memory.c +2
-4-* 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) {if ((vma->vm_flags &VM_WRITE ) && !userfaultfd_pte_wp (vma, pte) && !pte_needs_soft_dirty_wp (vma, pte)) { pte =pte_mkwrite (pte, vma);
Re: [RFC PATCH 2/3] mm: drop stale folio_ref_count()==1 check in do_swap_page reuse logic
In reply to Barry Song (Xiaomi)Quoted 23 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]/
Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)mm/memory.c +0
-1@@ -5049,12 +5049,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)/* * Same logic as in do_wp_page(); however, optimize for pages that are -* certainly not shared either because we just allocated them without
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5057
Quoted 10 lines
mm/memory.c +2
-4-* 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) {if ((vma->vm_flags &VM_WRITE ) && !userfaultfd_pte_wp (vma, pte) && !pte_needs_soft_dirty_wp (vma, pte)) { pte =pte_mkwrite (pte, vma);
RFC PATCH 3/3 mm: entirely remove lru_add_drain in do_swap_page
In reply to 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.
--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)
mm/memory.c +0 -10
@@ -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 );/*
Signature · 1 line
2.39.3 (Apple Git-146)
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
Quoted 23 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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; }
Quoted 5 lines
Folio reuse now relies primarily on the exclusive hint, making
lru_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
In reply to Barry Song (Xiaomi)Quoted 41 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)mm/memory.c +0
-1@@ -4903,16 +4903,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) }else if (folio != swapcache) page =folio_page (folio,0 ); -/*
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4906
Quoted 10 lines
mm/memory.c +0
-9-* 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 );
Quoted 5 lines
mm/memory.c +0
-1/* -- 2.39.3 (Apple Git-146)
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
In reply to Barry Song (Xiaomi)Quoted 55 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)mm/memory.c +0
-11@@ -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 );/* --
Quoted 3 lines
2.39.3 (Apple Git-146)
There is a ref check in should_try_to_free_swap though, perhaps we should also improve that part too or we may have more more folios stuck in swap cache after this change?
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
Quoted 55 lines · Kairui Song
Quoted 53 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)mm/memory.c +0
-10@@ -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 );
Quoted 9 lines
Quoted 5 lines
mm/memory.c +0
-1/* -- 2.39.3 (Apple Git-146)
There is a ref check in
should_try_to_free_swapthough, perhaps we should also improve that part too or we may have more more folios stuck in swap cache after this change?
Good catch. We could assume that a !LRU folio has an extra reference held by lru_cache. Does the below make sense to you?
mm/memory.c +4 -1
@@ -5094,8 +5094,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 * in swap cache and wait on the folio lock. +* Assume that lru_cache contributes an extra reference for non-LRU +* folios. */ -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);
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
Quoted 77 lines · Barry Song
Quoted 55 lines · Kairui Song
Quoted 53 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)mm/memory.c +0
-10@@ -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 );Quoted 9 lines
Quoted 5 lines
mm/memory.c +0
-1/* -- 2.39.3 (Apple Git-146)
There is a ref check in
should_try_to_free_swapthough, perhaps we should also improve that part too or we may have more more folios stuck in swap cache after this change?Good catch. We could assume that a !LRU folio has an extra reference held by
lru_cache. Does the below make sense to you?mm/memory.c +0
-0@@ -5094,8 +5094,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 5104
Quoted 10 lines
mm/memory.c +4
-1* in swap cache and wait on the folio lock. +* Assume that lru_cache contributes an extra reference for non-LRU +* folios. */ -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);
Right, this seems better. I've been thinking if we can just remove that ref check in should_try_to_free_swap and always return true to reclaim the cache for write faults. However, that seems like a bad idea after all since shared clean swapin folios on non-sync devices might benefit significantly from being cached, and such cases are unlikely to be very rare.
Just one thing: I think maybe we can have a more common helper to be shared by wp_can_reuse_anon_folio and should_try_to_free_swap. Since the cache reclaim in should_try_to_free_swap is meant to help wp_can_reuse_anon_folio (IIUC), keeping them more consistent seems easier to maintain and understand. Trivial though.
It's fine this way too, it's technically correct and this is only a heuristic check which doesn't effect correctness.
Re: [RFC PATCH 3/3] mm: entirely remove lru_add_drain in do_swap_page
Quoted 88 lines · Kairui Song
Quoted 86 lines · Barry Song
Quoted 55 lines · Kairui Song
Quoted 53 lines · Barry Song (Xiaomi)
We are doing a lot of redundant
lru_add_drain()calls indo_swap_page(), especially for synchronous I/O devices. For example, the test program below currently ends up draininglru_cache100% 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_cachedraining to drop the refcount inlru_cachelargely irrelevant.Signed-off-by Barry Song (Xiaomi) <[email protected]>--- mm/memory.c | 10 ---------- 1 file changed, 10 deletions(-)mm/memory.c +0
-10@@ -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 );Quoted 9 lines
Quoted 5 lines
mm/memory.c +0
-1/* -- 2.39.3 (Apple Git-146)
There is a ref check in
should_try_to_free_swapthough, perhaps we should also improve that part too or we may have more more folios stuck in swap cache after this change?Good catch. We could assume that a !LRU folio has an extra reference held by
lru_cache. Does the below make sense to you?mm/memory.c +4
-1@@ -5094,8 +5094,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 * in swap cache and wait on the folio lock. +* Assume that lru_cache contributes an extra reference for non-LRU +* folios. */ -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);
Quoted 8 lines
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 5104
Quoted 1 line
mm/memory.c +0
-0folio_unlock (folio);Right, this seems better. I've been thinking if we can just remove that ref check in
should_try_to_free_swapand always return true to reclaim the cache for write faults. However, that seems like a bad idea after all since shared clean swapin folios on non-sync devices might benefit significantly from being cached, and such cases are unlikely to be very rare.
I don’t quite understand what you mean. Even if we remove the refcheck folio_ref_count(folio) == (extra_refs + folio_nr_pages(folio)) from should_try_to_free_swap(), folio_maybe_swapped() would still prevent swapcache from being deleted for shared folios. The only effect is that we end up doing more work:
rcu_read_lock();
do {
if (__swp_tb_get_count(__swap_table_get(ci, ci_off))) {
ret = true;
break;
}
} while (++ci_off < ci_end);
rcu_read_unlock();and should_try_to_free_swap() currently checks fault_flags & FAULT_FLAG_WRITE, so clean folios are not touched for their swapcache.
The only thing I feel might not be perfect is sync I/O, which I think should be:
mm/memory.c +0 -0
@@ -4517,14 +4517,6 @@ static inline bool
should_try_to_free_swap(struct swap_info_struct *si,
{
if (!folio_test_swapcache(folio))
return false;
- /*
- * Always try to free swap cache for SWP_SYNCHRONOUS_IO devices. Swap
- * cache can help save some IO or memory overhead, but these devices
- * are fast, and meanwhile, swap cache pinning the slot deferring the
- * release of metadata or fragmentation is a more critical issue.
- */
- if (data_race(si->flags & SWP_SYNCHRONOUS_IO))
- return true;
if (mem_cgroup_swap_full(folio) || (vma->vm_flags & VM_LOCKED) ||
folio_test_mlocked(folio))
return true;@@ -4533,8 +4525,12 @@ static inline bool should_try_to_free_swap(struct swap_info_struct *si,
* 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.
+ * Always try to free swap cache for SWP_SYNCHRONOUS_IO devices, even
+ * when swapin is for read, to release memory used for compressed data,
+ * etc.
*/
- return (fault_flags & FAULT_FLAG_WRITE) && !folio_test_ksm(folio) &&
+ return ((fault_flags & FAULT_FLAG_WRITE) || data_race(si->flags &
+ SWP_SYNCHRONOUS_IO)) && !folio_test_ksm(folio) &&
folio_ref_count(folio) == (extra_refs + folio_nr_pages(folio));}
Not sure if it is worth it. I feel it would only save some overhead in folio_maybe_swapped() when we have many shared folios.
Quoted 9 lines
Just one thing: I think maybe we can have a more common helper to be shared by
wp_can_reuse_anon_folioandshould_try_to_free_swap. Since the cache reclaim inshould_try_to_free_swapis meant to helpwp_can_reuse_anon_folio(IIUC), keeping them more consistent seems easier to maintain and understand. Trivial though.It's fine this way too, it's technically correct and this is only a heuristic check which doesn't effect correctness.
thanks! I would stick with this approach for now, as wp_can_reuse_anon_folio() and do_swap_page() seem to differ significantly in how they handle SYNC I/O, folio locking, etc.
Best Regards
Barry