Re: [PATCH v9 14/41] mm: swap: Introduce lru_add_drain_progressive()

"David Hildenbrand (Arm)" <[email protected]>
Newsgroups org.kernel.vger.linux-kselftest,dev.linux.lists.linux-coco,org.kernel.vger.kvm,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-trace-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
On 7/29/26 02:35, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <[email protected]>
> 
> Extract the progressive LRU drain retry logic from
> collect_longterm_unpinnable_folios() into a reusable helper,
> lru_add_drain_progressive().
> 
> When attempting to isolate folios that may still reside in per-CPU folio
> batches, draining is escalated progressively:
> 
> 1. State 0: Call lru_add_drain() to flush local CPU batches.
> 2. State 1: Call lru_add_drain_all() to flush all CPU batches.
> 3. State >= 2: Return false to stop retrying.
> 
> Refactor collect_longterm_unpinnable_folios() to use this new helper.
> 
> The helper will be used by KVM's guest_memfd in a later patch.
> 
> Signed-off-by: Ackerley Tng <[email protected]>
> ---
>  include/linux/swap.h |  2 ++
>  mm/gup.c             | 19 ++++++-------------
>  mm/swap.c            | 15 +++++++++++++++
>  3 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 8f0f68e245baa..cd54f73f34f39 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -344,6 +344,8 @@ extern void lru_add_drain(void);
>  extern void lru_add_drain_cpu(int cpu);
>  extern void lru_add_drain_cpu_zone(struct zone *zone);
>  extern void lru_add_drain_all(void);
> +bool lru_add_drain_progressive(int *drain_state);
> +
>  void folio_deactivate(struct folio *folio);
>  void folio_mark_lazyfree(struct folio *folio);
>  extern void swap_setup(void);
> diff --git a/mm/gup.c b/mm/gup.c
> index 0692119b79043..5f00435e2c635 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2268,7 +2268,7 @@ static unsigned long collect_longterm_unpinnable_folios(
>  {
>  	unsigned long collected = 0;
>  	struct folio *folio;
> -	int drained = 0;
> +	int drain_state = 0;
>  	long i = 0;
>  
>  	for (folio = pofs_get_folio(pofs, i); folio;
> @@ -2287,18 +2287,11 @@ static unsigned long collect_longterm_unpinnable_folios(
>  			continue;
>  		}
>  
> -		if (drained == 0 && folio_may_be_lru_cached(folio) &&
> -				folio_ref_count(folio) !=
> -				folio_expected_ref_count(folio) + 1) {
> -			lru_add_drain();
> -			drained = 1;
> -		}
> -		if (drained == 1 && folio_may_be_lru_cached(folio) &&
> -				folio_ref_count(folio) !=
> -				folio_expected_ref_count(folio) + 1) {
> -			lru_add_drain_all();
> -			drained = 2;
> -		}
> +		while (folio_may_be_lru_cached(folio) &&
> +		       folio_ref_count(folio) !=
> +		       folio_expected_ref_count(folio) + 1 &&
> +		       lru_add_drain_progressive(&drain_state))
> +			;

That's rather nasty.

I was hoping that we could embed more logic in a helper. The history [1] of the
refcount check is rather sad:

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

... primarily because of mlock() handling.

For guest_memfd(), would mlock() ever apply on a path where you need that check?

Conceptually, I wonder whether we can do the following, and rely on the refcount
check only on the mlock path.


From ae30f7594692b0c47b44922c26b6480dd9584872 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <[email protected]>
Date: Thu, 30 Jul 2026 12:20:14 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <[email protected]>
---
 mm/gup.c | 66 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 99902c15703b0..ec43071bd035a 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2259,6 +2259,56 @@ static struct folio *pofs_next_folio(struct folio *folio,
 	return pofs_get_folio(pofs, i);
 }

+enum lru_cache_drained {
+	LRU_CACHE_NOT_DRAINED,
+	LRU_CACHE_DRAINED,
+	LRU_CACHE_DRAINED_ALL,
+};
+
+static inline void folio_likely_lru_cached(const struct folio *folio)
+{
+	if (!folio_may_be_lru_cached(folio))
+		return false;
+	/*
+	 * Having the LRU flag clear either indicates LRU cache references
+	 * or LRU isolation.
+	 */
+	if (!folio_test_lru(folio))
+		return true;
+	/*
+	 * For mlocked folios, we don't have a real indication: we can only
+	 * take a guess based on the refcount.
+	 */
+	if (!folio_test_mlock(folio))
+		return false;
+	return folio_expected_ref_count(folio) == folio_ref_count(folio) = 1;
+}
+
+/**
+ * lru_cache_drain_for_folio() - progressively try draining the lru cache
+ * @folio: The folio.
+ * @drained: Status initialized to LRU_CACHE_NOT_DRAINED by the caller.
+ *
+ * TODO
+ */
+static void lru_cache_drain_for_folio(const struct folio *folio,
+		enum lru_cache_drained *drained)
+{
+	if (!folio_likely_lru_cached(folio))
+		return false;
+
+	/* Try local draining first, if not already done previously. */
+	if (*drained == LRU_CACHE_NOT_DRAINED) {
+		lru_add_drain();
+		*drained = LRU_CACHE_DRAINED;
+	}
+	/* Try draining all CPUs next if still not an LRU folio. */
+	if (folio_likely_lru_cached(folio) && *drained == LRU_CACHE_DRAINED) {
+		lru_add_drain_all();
+		*drained = LRU_CACHE_DRAINED_ALL;
+	}
+}
+
 /*
  * Returns the number of collected folios. Return value is always >= 0.
  */
@@ -2266,9 +2316,9 @@ static unsigned long collect_longterm_unpinnable_folios(
 		struct list_head *movable_folio_list,
 		struct pages_or_folios *pofs)
 {
+	enum lru_cache_drained drained = LRU_CACHE_NOT_DRAINED;
 	unsigned long collected = 0;
 	struct folio *folio;
-	int drained = 0;
 	long i = 0;

 	for (folio = pofs_get_folio(pofs, i); folio;
@@ -2287,19 +2337,7 @@ static unsigned long collect_longterm_unpinnable_folios(
 			continue;
 		}

-		if (drained == 0 && folio_may_be_lru_cached(folio) &&
-				folio_ref_count(folio) !=
-				folio_expected_ref_count(folio) + 1) {
-			lru_add_drain();
-			drained = 1;
-		}
-		if (drained == 1 && folio_may_be_lru_cached(folio) &&
-				folio_ref_count(folio) !=
-				folio_expected_ref_count(folio) + 1) {
-			lru_add_drain_all();
-			drained = 2;
-		}
-
+		lru_cache_drain_for_folio(folio, &drained);
 		if (!folio_isolate_lru(folio))
 			continue;

-- 
2.43.0


-- 
Cheers,

David
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.