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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/29/26 02:35, Ackerley Tng via B4 Relay wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> 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 <ackerleytng@google.com>
> ---
>  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/c5bac539-fd8a-4db7-c21c-cd3e457eee91@google.com/

... 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)" <david@kernel.org>
Date: Thu, 30 Jul 2026 12:20:14 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 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