[PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Kaitao Cheng]

From: Kaitao Cheng <chengkaitao@kylinos.cn>

Successful writeback calls zswap_entry_free(), which removes the entry
from the LRU and decrements both the sublist and node item counts.
shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
to decrement the node count again.

The affected call path before this change is:

  __list_lru_walk_one()
    shrink_memcg_cb()
      zswap_writeback_entry()
        zswap_entry_free()
          zswap_lru_del()
            list_lru_del()
              __list_lru_del()
                atomic_long_dec()  /* first node count decrement */
      return LRU_REMOVED_RETRY
    atomic_long_dec()              /* second node count decrement */

Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
count, leaving node accounting to the walker. Split data cleanup into
__zswap_entry_free() so writeback can free the entry without performing
another LRU deletion. Other free paths retain their existing accounting.

Fixes: 5878303c5353 ("mm/zswap: fix race between lru writeback and swapoff")
Link: https://sashiko.dev/#/bug/linux-68d2ca49-23be-4504-9a75-b8526ec92388
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
 mm/zswap.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index ad01c691a263..d0c98daddf2e 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -719,6 +719,22 @@ static void zswap_lru_del(struct zswap_entry *entry)
 	rcu_read_unlock();
 }
 
+/* Like zswap_lru_del(), but does not decrement the node item count. */
+static void zswap_lru_del_nocnt(struct zswap_entry *entry)
+{
+	int nid = entry_to_nid(entry);
+	struct mem_cgroup *memcg;
+	struct list_lru_one *l;
+
+	rcu_read_lock();
+	memcg = obj_cgroup_memcg(entry->objcg);
+	l = list_lru_lock(&zswap_list_lru, nid, &memcg);
+	if (!list_empty(&entry->lru))
+		list_lru_isolate(l, &entry->lru);
+	list_lru_unlock(l);
+	rcu_read_unlock();
+}
+
 void zswap_lruvec_state_init(struct lruvec *lruvec)
 {
 	atomic_long_set(&lruvec->zswap_lruvec_state.nr_disk_swapins, 0);
@@ -781,11 +797,10 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
  * Carries out the common pattern of freeing an entry's zsmalloc allocation,
  * freeing the entry itself, and decrementing the number of stored pages.
  */
-static void zswap_entry_free(struct zswap_entry *entry)
+static void __zswap_entry_free(struct zswap_entry *entry)
 {
 	struct zswap_pool *pool = zswap_entry_pool(entry);
 
-	zswap_lru_del(entry);
 	if (!WARN_ON_ONCE(!pool)) {
 		zs_free(pool->zs_pool, entry->handle);
 		zswap_pool_put(pool);
@@ -800,6 +815,12 @@ static void zswap_entry_free(struct zswap_entry *entry)
 	atomic_long_dec(&zswap_stored_pages);
 }
 
+static void zswap_entry_free(struct zswap_entry *entry)
+{
+	zswap_lru_del(entry);
+	__zswap_entry_free(entry);
+}
+
 /*********************************
 * compressed storage functions
 **********************************/
@@ -1119,7 +1140,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
 	if (entry->objcg)
 		count_objcg_events(entry->objcg, ZSWPWB, 1);
 
-	zswap_entry_free(entry);
+	zswap_lru_del_nocnt(entry);
+	__zswap_entry_free(entry);
 
 	/* folio is up to date */
 	folio_mark_uptodate(folio);
-- 
2.53.0

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Nhat Pham]

On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>
> Successful writeback calls zswap_entry_free(), which removes the entry
> from the LRU and decrements both the sublist and node item counts.
> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> to decrement the node count again.
>
> The affected call path before this change is:
>
>   __list_lru_walk_one()
>     shrink_memcg_cb()
>       zswap_writeback_entry()
>         zswap_entry_free()
>           zswap_lru_del()
>             list_lru_del()
>               __list_lru_del()
>                 atomic_long_dec()  /* first node count decrement */
>       return LRU_REMOVED_RETRY
>     atomic_long_dec()              /* second node count decrement */
>
> Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
> count, leaving node accounting to the walker. Split data cleanup into
> __zswap_entry_free() so writeback can free the entry without performing
> another LRU deletion. Other free paths retain their existing accounting.

What is the visible effect? Is there any effect, be it in reclaim
dynamics or userspace counters? Can you add it to the changelog?

I'm not against fixing it, but need to gauge the effect for
prioritization, whether to cc stable etc. etc.


>
> Fixes: 5878303c5353 ("mm/zswap: fix race between lru writeback and swapoff")
> Link: https://sashiko.dev/#/bug/linux-68d2ca49-23be-4504-9a75-b8526ec92388
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Kaitao Cheng]

在 2026/10/29 02:47, Nhat Pham 写道:
> On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>>
>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>
>> Successful writeback calls zswap_entry_free(), which removes the entry
>> from the LRU and decrements both the sublist and node item counts.
>> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
>> to decrement the node count again.
>>
>> The affected call path before this change is:
>>
>>   __list_lru_walk_one()
>>     shrink_memcg_cb()
>>       zswap_writeback_entry()
>>         zswap_entry_free()
>>           zswap_lru_del()
>>             list_lru_del()
>>               __list_lru_del()
>>                 atomic_long_dec()  /* first node count decrement */
>>       return LRU_REMOVED_RETRY
>>     atomic_long_dec()              /* second node count decrement */
>>
>> Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
>> count, leaving node accounting to the walker. Split data cleanup into
>> __zswap_entry_free() so writeback can free the entry without performing
>> another LRU deletion. Other free paths retain their existing accounting.
> 
> What is the visible effect? Is there any effect, be it in reclaim
> dynamics or userspace counters? Can you add it to the changelog?
> 
> I'm not against fixing it, but need to gauge the effect for
> prioritization, whether to cc stable etc. etc.
> 

This is only an accounting issue; I could not find any code in zswap that
reads lru->node[nid].nr_items.

>>
>> Fixes: 5878303c5353 ("mm/zswap: fix race between lru writeback and swapoff")
>> Link: https://sashiko.dev/#/bug/linux-68d2ca49-23be-4504-9a75-b8526ec92388
>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>

-- 
Thanks
Kaitao Cheng

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Nhat Pham]

On Wed, Oct 28, 2026 at 9:15 PM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>
>
>
> 在 2026/10/29 02:47, Nhat Pham 写道:
> > On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> >>
> >> From: Kaitao Cheng <chengkaitao@kylinos.cn>
> >>
> >> Successful writeback calls zswap_entry_free(), which removes the entry
> >> from the LRU and decrements both the sublist and node item counts.
> >> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> >> to decrement the node count again.
> >>
> >> The affected call path before this change is:
> >>
> >>   __list_lru_walk_one()
> >>     shrink_memcg_cb()
> >>       zswap_writeback_entry()
> >>         zswap_entry_free()
> >>           zswap_lru_del()
> >>             list_lru_del()
> >>               __list_lru_del()
> >>                 atomic_long_dec()  /* first node count decrement */
> >>       return LRU_REMOVED_RETRY
> >>     atomic_long_dec()              /* second node count decrement */
> >>
> >> Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
> >> count, leaving node accounting to the walker. Split data cleanup into
> >> __zswap_entry_free() so writeback can free the entry without performing
> >> another LRU deletion. Other free paths retain their existing accounting.
> >
> > What is the visible effect? Is there any effect, be it in reclaim
> > dynamics or userspace counters? Can you add it to the changelog?
> >
> > I'm not against fixing it, but need to gauge the effect for
> > prioritization, whether to cc stable etc. etc.
> >
>
> This is only an accounting issue; I could not find any code in zswap that
> reads lru->node[nid].nr_items.

Yeah I double checked - this is the node counter
(list_lru_node->nr_items). zswap uses the per-node-per-memcg counter
(list_lru_one->nr_items).

I think Johannes' approach SGTM.

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Yosry Ahmed]

On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>
> Successful writeback calls zswap_entry_free(), which removes the entry
> from the LRU and decrements both the sublist and node item counts.
> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> to decrement the node count again.

I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
LRU_DELETED instead to simply avoid the decrement.

Adding shrinker maintainers.

>
> The affected call path before this change is:
>
>   __list_lru_walk_one()
>     shrink_memcg_cb()
>       zswap_writeback_entry()
>         zswap_entry_free()
>           zswap_lru_del()
>             list_lru_del()
>               __list_lru_del()
>                 atomic_long_dec()  /* first node count decrement */
>       return LRU_REMOVED_RETRY
>     atomic_long_dec()              /* second node count decrement */
>
> Add zswap_lru_del_nocnt() to remove the entry and update only the sublist
> count, leaving node accounting to the walker. Split data cleanup into
> __zswap_entry_free() so writeback can free the entry without performing
> another LRU deletion. Other free paths retain their existing accounting.
>
> Fixes: 5878303c5353 ("mm/zswap: fix race between lru writeback and swapoff")
> Link: https://sashiko.dev/#/bug/linux-68d2ca49-23be-4504-9a75-b8526ec92388
> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
> ---
>  mm/zswap.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ad01c691a263..d0c98daddf2e 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -719,6 +719,22 @@ static void zswap_lru_del(struct zswap_entry *entry)
>         rcu_read_unlock();
>  }
>
> +/* Like zswap_lru_del(), but does not decrement the node item count. */
> +static void zswap_lru_del_nocnt(struct zswap_entry *entry)
> +{
> +       int nid = entry_to_nid(entry);
> +       struct mem_cgroup *memcg;
> +       struct list_lru_one *l;
> +
> +       rcu_read_lock();
> +       memcg = obj_cgroup_memcg(entry->objcg);
> +       l = list_lru_lock(&zswap_list_lru, nid, &memcg);
> +       if (!list_empty(&entry->lru))
> +               list_lru_isolate(l, &entry->lru);
> +       list_lru_unlock(l);
> +       rcu_read_unlock();
> +}
> +
>  void zswap_lruvec_state_init(struct lruvec *lruvec)
>  {
>         atomic_long_set(&lruvec->zswap_lruvec_state.nr_disk_swapins, 0);
> @@ -781,11 +797,10 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
>   * Carries out the common pattern of freeing an entry's zsmalloc allocation,
>   * freeing the entry itself, and decrementing the number of stored pages.
>   */
> -static void zswap_entry_free(struct zswap_entry *entry)
> +static void __zswap_entry_free(struct zswap_entry *entry)
>  {
>         struct zswap_pool *pool = zswap_entry_pool(entry);
>
> -       zswap_lru_del(entry);
>         if (!WARN_ON_ONCE(!pool)) {
>                 zs_free(pool->zs_pool, entry->handle);
>                 zswap_pool_put(pool);
> @@ -800,6 +815,12 @@ static void zswap_entry_free(struct zswap_entry *entry)
>         atomic_long_dec(&zswap_stored_pages);
>  }
>
> +static void zswap_entry_free(struct zswap_entry *entry)
> +{
> +       zswap_lru_del(entry);
> +       __zswap_entry_free(entry);
> +}
> +
>  /*********************************
>  * compressed storage functions
>  **********************************/
> @@ -1119,7 +1140,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>         if (entry->objcg)
>                 count_objcg_events(entry->objcg, ZSWPWB, 1);
>
> -       zswap_entry_free(entry);
> +       zswap_lru_del_nocnt(entry);
> +       __zswap_entry_free(entry);
>
>         /* folio is up to date */
>         folio_mark_uptodate(folio);
> --
> 2.53.0
>

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Johannes Weiner]

On Wed, Oct 28, 2026 at 12:22:43PM -0700, Yosry Ahmed wrote:
> On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> >
> > From: Kaitao Cheng <chengkaitao@kylinos.cn>
> >
> > Successful writeback calls zswap_entry_free(), which removes the entry
> > from the LRU and decrements both the sublist and node item counts.
> > shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> > to decrement the node count again.
> 
> I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
> LRU_DELETED instead to simply avoid the decrement.

It bugs me that it's the walker, not the callback, that adjusts the
count. That composes poorly with the normal list manipulation API.

How about pushing the accounting into the isolation callback, which
can then simply use __list_lru_del()?

The tradeoff is that more things need to be passed into the isolation
callback, but that seems reasonable. It does make the change noisy,
however, and we might want something like Kaitao's change as a
backportable fix first regardless.

Thoughts?

---

diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
index b0c797523bc2..157256e52dd0 100644
--- a/drivers/android/binder/page_range.rs
+++ b/drivers/android/binder/page_range.rs
@@ -24,7 +24,7 @@
 use kernel::{
     bindings,
     error::Result,
-    ffi::{c_ulong, c_void},
+    ffi::{c_int, c_ulong, c_void},
     mm::{virt, Mm, MmWithUser},
     new_mutex, new_spinlock,
     page::{Page, PAGE_SHIFT, PAGE_SIZE},
@@ -684,8 +684,10 @@ fn drop(self: Pin<&mut Self>) {
 /// Called by the shrinker.
 #[no_mangle]
 unsafe extern "C" fn rust_shrink_free_page(
+    lru: *mut bindings::list_lru,
+    l: *mut bindings::list_lru_one,
     item: *mut bindings::list_head,
-    lru: *mut bindings::list_lru_one,
+    nid: c_int,
     _cb_arg: *mut c_void,
 ) -> bindings::lru_status {
     // Fields that should survive after unlocking the lru lock.
@@ -728,7 +730,7 @@ fn drop(self: Pin<&mut Self>) {
         };
 
         // SAFETY: The item is in this lru list, so it's okay to remove it.
-        unsafe { bindings::list_lru_isolate(lru, item) };
+        unsafe { bindings::__list_lru_del(lru, l, item, nid) };
 
         // SAFETY: Both pointers are in bounds of the same allocation.
         page_index = unsafe { info.offset_from(inner.pages) } as usize;
@@ -746,7 +748,7 @@ fn drop(self: Pin<&mut Self>) {
     }
 
     // SAFETY: The lru lock is locked when this method is called.
-    unsafe { bindings::spin_unlock(&raw mut (*lru).lock) };
+    unsafe { bindings::spin_unlock(&raw mut (*l).lock) };
 
     if let Some(vma) = check_vma(&vma_read, range_ptr) {
         let user_page_addr = vma_addr + (page_index << PAGE_SHIFT);
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
index 1bf37023fed6..2e858c05e30b 100644
--- a/drivers/gpu/drm/ttm/ttm_pool.c
+++ b/drivers/gpu/drm/ttm/ttm_pool.c
@@ -345,13 +345,14 @@ static void ttm_pool_type_give(struct ttm_pool_type *pt, struct page *p)
 	mod_lruvec_page_state(p, NR_GPU_RECLAIM, num_pages);
 }
 
-static enum lru_status take_one_from_lru(struct list_head *item,
-					 struct list_lru_one *list,
+static enum lru_status take_one_from_lru(struct list_lru *lru,
+					 struct list_lru_one *l,
+					 struct list_head *item, int nid,
 					 void *cb_arg)
 {
 	struct page **out_page = cb_arg;
 	struct page *p = container_of(item, struct page, lru);
-	list_lru_isolate(list, item);
+	__list_lru_del(lru, l, item, nid);
 
 	*out_page = p;
 	return LRU_REMOVED;
@@ -387,13 +388,14 @@ static void ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
 	spin_unlock(&shrinker_lock);
 }
 
-static enum lru_status pool_move_to_dispose_list(struct list_head *item,
-						 struct list_lru_one *list,
+static enum lru_status pool_move_to_dispose_list(struct list_lru *lru,
+						 struct list_lru_one *l,
+						 struct list_head *item, int nid,
 						 void *cb_arg)
 {
 	struct list_head *dispose = cb_arg;
 
-	list_lru_isolate_move(list, item, dispose);
+	__list_lru_move(lru, l, item, nid, dispose);
 
 	return LRU_REMOVED;
 }
diff --git a/fs/dcache.c b/fs/dcache.c
index 37d021d22a7d..ce8063c2f5a6 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -556,24 +556,26 @@ static void d_shrink_add(struct dentry *dentry, struct list_head *list)
  * LRU lists entirely, while shrink_move moves it to the indicated
  * private list.
  */
-static void d_lru_isolate(struct list_lru_one *lru, struct dentry *dentry)
+static void d_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
+			  struct dentry *dentry, int nid)
 {
 	D_FLAG_VERIFY(dentry, DCACHE_LRU_LIST);
 	dentry->d_flags &= ~DCACHE_LRU_LIST;
 	this_cpu_dec(nr_dentry_unused);
 	if (d_is_negative(dentry))
 		this_cpu_dec(nr_dentry_negative);
-	list_lru_isolate(lru, &dentry->d_lru);
+	__list_lru_del(lru, l, &dentry->d_lru, nid);
 }
 
-static void d_lru_shrink_move(struct list_lru_one *lru, struct dentry *dentry,
+static void d_lru_shrink_move(struct list_lru *lru, struct list_lru_one *l,
+			      struct dentry *dentry, int nid,
 			      struct list_head *list)
 {
 	D_FLAG_VERIFY(dentry, DCACHE_LRU_LIST);
 	dentry->d_flags |= DCACHE_SHRINK_LIST;
 	if (d_is_negative(dentry))
 		this_cpu_dec(nr_dentry_negative);
-	list_lru_isolate_move(lru, &dentry->d_lru, list);
+	__list_lru_move(lru, l, &dentry->d_lru, nid, list);
 }
 
 static void ___d_drop(struct dentry *dentry)
@@ -1308,8 +1310,8 @@ void shrink_dentry_list(struct list_head *list)
 }
 EXPORT_SYMBOL(shrink_dentry_list);
 
-static enum lru_status dentry_lru_isolate(struct list_head *item,
-		struct list_lru_one *lru, void *arg)
+static enum lru_status dentry_lru_isolate(struct list_lru *lru,
+		struct list_lru_one *l, struct list_head *item, int nid, void *arg)
 {
 	struct list_head *freeable = arg;
 	struct dentry	*dentry = container_of(item, struct dentry, d_lru);
@@ -1329,7 +1331,7 @@ static enum lru_status dentry_lru_isolate(struct list_head *item,
 	 * another pass through the LRU.
 	 */
 	if (dentry->d_lockref.count) {
-		d_lru_isolate(lru, dentry);
+		d_lru_isolate(lru, l, dentry, nid);
 		spin_unlock(&dentry->d_lock);
 		return LRU_REMOVED;
 	}
@@ -1360,7 +1362,7 @@ static enum lru_status dentry_lru_isolate(struct list_head *item,
 		return LRU_ROTATE;
 	}
 
-	d_lru_shrink_move(lru, dentry, freeable);
+	d_lru_shrink_move(lru, l, dentry, nid, freeable);
 	spin_unlock(&dentry->d_lock);
 
 	return LRU_REMOVED;
@@ -1389,8 +1391,8 @@ long prune_dcache_sb(struct super_block *sb, struct shrink_control *sc)
 	return freed;
 }
 
-static enum lru_status dentry_lru_isolate_shrink(struct list_head *item,
-		struct list_lru_one *lru, void *arg)
+static enum lru_status dentry_lru_isolate_shrink(struct list_lru *lru,
+		struct list_lru_one *l, struct list_head *item, int nid, void *arg)
 {
 	struct list_head *freeable = arg;
 	struct dentry	*dentry = container_of(item, struct dentry, d_lru);
@@ -1403,7 +1405,7 @@ static enum lru_status dentry_lru_isolate_shrink(struct list_head *item,
 	if (!spin_trylock(&dentry->d_lock))
 		return LRU_SKIP;
 
-	d_lru_shrink_move(lru, dentry, freeable);
+	d_lru_shrink_move(lru, l, dentry, nid, freeable);
 	spin_unlock(&dentry->d_lock);
 
 	return LRU_REMOVED;
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index b2984fb7b35c..c8afe8236805 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -148,8 +148,8 @@ static void gfs2_qd_list_dispose(struct list_head *list)
 }
 
 
-static enum lru_status gfs2_qd_isolate(struct list_head *item,
-		struct list_lru_one *lru, void *arg)
+static enum lru_status gfs2_qd_isolate(struct list_lru *lru,
+		struct list_lru_one *l, struct list_head *item, int nid, void *arg)
 {
 	struct list_head *dispose = arg;
 	struct gfs2_quota_data *qd =
@@ -162,7 +162,7 @@ static enum lru_status gfs2_qd_isolate(struct list_head *item,
 	status = LRU_SKIP;
 	if (qd->qd_lockref.count == 0) {
 		lockref_mark_dead(&qd->qd_lockref);
-		list_lru_isolate_move(lru, &qd->qd_lru, dispose);
+		__list_lru_move(lru, l, &qd->qd_lru, nid, dispose);
 		status = LRU_REMOVED;
 	}
 
diff --git a/fs/inode.c b/fs/inode.c
index 1cb6293b237a..675f4a8b9d95 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -929,8 +929,8 @@ EXPORT_SYMBOL_GPL(evict_inodes);
  * LRU does not have strict ordering. Hence we don't want to reclaim inodes
  * with this flag set because they are the inodes that are out of order.
  */
-static enum lru_status inode_lru_isolate(struct list_head *item,
-		struct list_lru_one *lru, void *arg)
+static enum lru_status inode_lru_isolate(struct list_lru *lru,
+		struct list_lru_one *l, struct list_head *item, int nid, void *arg)
 {
 	struct list_head *freeable = arg;
 	struct inode	*inode = container_of(item, struct inode, i_lru);
@@ -951,7 +951,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 	if (icount_read(inode) ||
 	    (inode_state_read(inode) & ~I_REFERENCED) ||
 	    !mapping_shrinkable(&inode->i_data)) {
-		list_lru_isolate(lru, &inode->i_lru);
+		__list_lru_del(lru, l, &inode->i_lru, nid);
 		spin_unlock(&inode->i_lock);
 		this_cpu_dec(nr_unused);
 		return LRU_REMOVED;
@@ -974,7 +974,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 
 		inode_pin_lru_isolating(inode);
 		spin_unlock(&inode->i_lock);
-		spin_unlock(&lru->lock);
+		spin_unlock(&l->lock);
 		reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
 		if (current_is_kswapd())
 			__count_vm_events(KSWAPD_INODESTEAL, reap);
@@ -987,7 +987,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 
 	WARN_ON(inode_state_read(inode) & I_NEW);
 	inode_state_set(inode, I_FREEING);
-	list_lru_isolate_move(lru, &inode->i_lru, freeable);
+	__list_lru_move(lru, l, &inode->i_lru, nid, freeable);
 	spin_unlock(&inode->i_lock);
 
 	this_cpu_dec(nr_unused);
diff --git a/fs/nfs/nfs42xattr.c b/fs/nfs/nfs42xattr.c
index 37d79400e5f4..17eb14b40830 100644
--- a/fs/nfs/nfs42xattr.c
+++ b/fs/nfs/nfs42xattr.c
@@ -801,8 +801,8 @@ static struct shrinker *nfs4_xattr_entry_shrinker;
 static struct shrinker *nfs4_xattr_large_entry_shrinker;
 
 static enum lru_status
-cache_lru_isolate(struct list_head *item,
-	struct list_lru_one *lru, void *arg)
+cache_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
+	struct list_head *item, int nid, void *arg)
 {
 	struct list_head *dispose = arg;
 	struct inode *inode;
@@ -827,7 +827,7 @@ cache_lru_isolate(struct list_head *item,
 	cache->inode = NULL;
 	NFS_I(inode)->xattr_cache = NULL;
 	NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_XATTR;
-	list_lru_isolate(lru, &cache->lru);
+	__list_lru_del(lru, l, &cache->lru, nid);
 
 	spin_unlock(&inode->i_lock);
 
@@ -866,8 +866,8 @@ nfs4_xattr_cache_count(struct shrinker *shrink, struct shrink_control *sc)
 }
 
 static enum lru_status
-entry_lru_isolate(struct list_head *item,
-	struct list_lru_one *lru, void *arg)
+entry_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
+	struct list_head *item, int nid, void *arg)
 {
 	struct list_head *dispose = arg;
 	struct nfs4_xattr_bucket *bucket;
@@ -896,7 +896,7 @@ entry_lru_isolate(struct list_head *item,
 
 		hlist_del_init(&entry->hnode);
 		atomic_long_dec(&cache->nent);
-		list_lru_isolate(lru, &entry->lru);
+		__list_lru_del(lru, l, &entry->lru, nid);
 
 		spin_unlock(&bucket->lock);
 	} else {
@@ -907,7 +907,7 @@ entry_lru_isolate(struct list_head *item,
 		kref_get(&entry->ref);
 
 		cache->listxattr = NULL;
-		list_lru_isolate(lru, &entry->lru);
+		__list_lru_del(lru, l, &entry->lru, nid);
 
 		spin_unlock(&cache->listxattr_lock);
 	}
diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index 17a94e6fcb15..6b9165e5c3c5 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -509,8 +509,10 @@ void nfsd_file_net_dispose(struct nfsd_net *nn)
 
 /**
  * nfsd_file_lru_cb - Examine an entry on the LRU list
- * @item: LRU entry to examine
  * @lru: controlling LRU
+ * @l: locked sublist of @lru that holds @item
+ * @item: LRU entry to examine
+ * @nid: node id of @l
  * @arg: dispose list
  *
  * Return values:
@@ -519,8 +521,8 @@ void nfsd_file_net_dispose(struct nfsd_net *nn)
  *   %LRU_SKIP: @item cannot be evicted
  */
 static enum lru_status
-nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
-		 void *arg)
+nfsd_file_lru_cb(struct list_lru *lru, struct list_lru_one *l,
+		 struct list_head *item, int nid, void *arg)
 {
 	struct list_head *head = arg;
 	struct nfsd_file *nf = list_entry(item, struct nfsd_file, nf_lru);
@@ -554,7 +556,7 @@ nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
 
 	/* Refcount went to zero. Unhash it and queue it to the dispose list */
 	nfsd_file_unhash(nf);
-	list_lru_isolate(lru, &nf->nf_lru);
+	__list_lru_del(lru, l, &nf->nf_lru, nid);
 	list_add(&nf->nf_gc, head);
 	this_cpu_inc(nfsd_file_evictions);
 	trace_nfsd_file_gc_disposed(nf);
@@ -562,8 +564,8 @@ nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
 }
 
 static enum lru_status
-nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
-		 void *arg)
+nfsd_file_gc_cb(struct list_lru *lru, struct list_lru_one *l,
+		 struct list_head *item, int nid, void *arg)
 {
 	struct nfsd_file *nf = list_entry(item, struct nfsd_file, nf_lru);
 
@@ -576,7 +578,7 @@ nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
 		trace_nfsd_file_gc_aged(nf);
 		return LRU_ROTATE;
 	}
-	return nfsd_file_lru_cb(item, lru, arg);
+	return nfsd_file_lru_cb(lru, l, item, nid, arg);
 }
 
 static void
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 6c93b4f5629c..436aca8f6619 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1467,8 +1467,10 @@ __xfs_buf_mark_corrupt(
  */
 static enum lru_status
 xfs_buftarg_drain_rele(
+	struct list_lru		*lru,
+	struct list_lru_one	*l,
 	struct list_head	*item,
-	struct list_lru_one	*lru,
+	int			nid,
 	void			*arg)
 
 {
@@ -1485,7 +1487,7 @@ xfs_buftarg_drain_rele(
 	}
 
 	xfs_buf_kill(bp);
-	list_lru_isolate_move(lru, item, dispose);
+	__list_lru_move(lru, l, item, nid, dispose);
 	spin_unlock(&bp->b_lockref.lock);
 	return LRU_REMOVED;
 }
@@ -1560,8 +1562,10 @@ xfs_buftarg_drain(
 
 static enum lru_status
 xfs_buftarg_isolate(
+	struct list_lru		*lru,
+	struct list_lru_one	*l,
 	struct list_head	*item,
-	struct list_lru_one	*lru,
+	int			nid,
 	void			*arg)
 {
 	struct xfs_buf		*bp = container_of(item, struct xfs_buf, b_lru);
@@ -1581,7 +1585,7 @@ xfs_buftarg_isolate(
 	 * yet.
 	 */
 	if (bp->b_lockref.count > 0) {
-		list_lru_isolate(lru, &bp->b_lru);
+		__list_lru_del(lru, l, &bp->b_lru, nid);
 		spin_unlock(&bp->b_lockref.lock);
 		return LRU_REMOVED;
 	}
@@ -1597,7 +1601,7 @@ xfs_buftarg_isolate(
 	}
 
 	xfs_buf_kill(bp);
-	list_lru_isolate_move(lru, item, dispose);
+	__list_lru_move(lru, l, item, nid, dispose);
 	spin_unlock(&bp->b_lockref.lock);
 	return LRU_REMOVED;
 }
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index dd5011f578a6..3933dd222509 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -411,10 +411,12 @@ struct xfs_qm_isolate {
 
 static enum lru_status
 xfs_qm_dquot_isolate(
+	struct list_lru		*lru,
+	struct list_lru_one	*l,
 	struct list_head	*item,
-	struct list_lru_one	*lru,
+	int			nid,
 	void			*arg)
-		__releases(&lru->lock) __acquires(&lru->lock)
+		__releases(&l->lock) __acquires(&l->lock)
 {
 	struct xfs_dquot	*dqp = container_of(item,
 						struct xfs_dquot, q_lru);
@@ -450,7 +452,7 @@ xfs_qm_dquot_isolate(
 		XFS_STATS_INC(dqp->q_mount, xs_qm_dqwants);
 
 		trace_xfs_dqreclaim_want(dqp);
-		list_lru_isolate(lru, &dqp->q_lru);
+		__list_lru_del(lru, l, &dqp->q_lru, nid);
 		XFS_STATS_DEC(dqp->q_mount, xs_qm_dquot_unused);
 		return LRU_REMOVED;
 	}
@@ -473,7 +475,7 @@ xfs_qm_dquot_isolate(
 	lockref_mark_dead(&dqp->q_lockref);
 	spin_unlock(&dqp->q_lockref.lock);
 
-	list_lru_isolate_move(lru, &dqp->q_lru, &isol->dispose);
+	__list_lru_move(lru, l, &dqp->q_lru, nid, &isol->dispose);
 	XFS_STATS_DEC(dqp->q_mount, xs_qm_dquot_unused);
 	trace_xfs_dqreclaim_done(dqp);
 	XFS_STATS_INC(dqp->q_mount, xs_qm_dqreclaims);
diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
index a450fffe1550..f6d94474a4c7 100644
--- a/include/linux/list_lru.h
+++ b/include/linux/list_lru.h
@@ -150,6 +150,15 @@ bool __list_lru_add(struct list_lru *lru, struct list_lru_one *l,
 bool __list_lru_del(struct list_lru *lru, struct list_lru_one *l,
 		struct list_head *item, int nid);
 
+/* Take @item off the locked sublist @l and put it on a private list @head */
+static inline void __list_lru_move(struct list_lru *lru, struct list_lru_one *l,
+				   struct list_head *item, int nid,
+				   struct list_head *head)
+{
+	__list_lru_del(lru, l, item, nid);
+	list_add(item, head);
+}
+
 /**
  * list_lru_add: add an element to the lru list's tail
  * @lru: the lru pointer
@@ -271,12 +280,9 @@ static inline unsigned long list_lru_count(struct list_lru *lru)
 	return count;
 }
 
-void list_lru_isolate(struct list_lru_one *list, struct list_head *item);
-void list_lru_isolate_move(struct list_lru_one *list, struct list_head *item,
-			   struct list_head *head);
-
-typedef enum lru_status (*list_lru_walk_cb)(struct list_head *item,
-		struct list_lru_one *list, void *cb_arg);
+typedef enum lru_status (*list_lru_walk_cb)(struct list_lru *lru,
+		struct list_lru_one *l, struct list_head *item, int nid,
+		void *cb_arg);
 
 /**
  * list_lru_walk_one: walk a @lru, isolating and disposing freeable items.
@@ -289,11 +295,14 @@ typedef enum lru_status (*list_lru_walk_cb)(struct list_head *item,
  * @nr_to_walk: how many items to scan.
  *
  * This function will scan all elements in a particular @lru, calling the
- * @isolate callback for each of those items, along with the current list
- * spinlock and a caller-provided opaque. The @isolate callback can choose to
+ * @isolate callback for each of those items, along with the locked sublist
+ * and a caller-provided opaque. The @isolate callback can choose to
  * drop the lock internally, but *must* return with the lock held. The callback
  * will return an enum lru_status telling the @lru infrastructure what to
- * do with the object being scanned.
+ * do with the object being scanned. A callback that takes the item off
+ * the lru does so with __list_lru_del() or __list_lru_move(), or with
+ * list_lru_del() once it has dropped the lock, and returns LRU_REMOVED
+ * or LRU_REMOVED_RETRY.
  *
  * Please note that @nr_to_walk does not mean how many objects will be freed,
  * just how many objects will be scanned.
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index bd44d58e8684..87722e1a7eee 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -4638,8 +4638,9 @@ static bool thp_underused(struct folio *folio)
 	return false;
 }
 
-static enum lru_status deferred_split_isolate(struct list_head *item,
-					      struct list_lru_one *lru,
+static enum lru_status deferred_split_isolate(struct list_lru *lru,
+					      struct list_lru_one *l,
+					      struct list_head *item, int nid,
 					      void *cb_arg)
 {
 	struct folio *folio = container_of(item, struct folio, _deferred_list);
@@ -4649,7 +4650,7 @@ static enum lru_status deferred_split_isolate(struct list_head *item,
 	if (!folio_try_get(folio))
 		return LRU_SKIP;
 
-	list_lru_isolate_move(lru, item, freeable);
+	__list_lru_move(lru, l, item, nid, freeable);
 	return LRU_REMOVED;
 }
 
diff --git a/mm/list_lru.c b/mm/list_lru.c
index d1d256822ccc..9469117aa424 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -218,6 +218,7 @@ bool __list_lru_del(struct list_lru *lru, struct list_lru_one *l,
 	}
 	return false;
 }
+EXPORT_SYMBOL_GPL(__list_lru_del);
 
 /* The caller must ensure the memcg lifetime. */
 bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
@@ -291,21 +292,6 @@ bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
 }
 EXPORT_SYMBOL_GPL(list_lru_del_obj);
 
-void list_lru_isolate(struct list_lru_one *list, struct list_head *item)
-{
-	list_del_init(item);
-	list->nr_items--;
-}
-EXPORT_SYMBOL_GPL(list_lru_isolate);
-
-void list_lru_isolate_move(struct list_lru_one *list, struct list_head *item,
-			   struct list_head *head)
-{
-	list_move(item, head);
-	list->nr_items--;
-}
-EXPORT_SYMBOL_GPL(list_lru_isolate_move);
-
 unsigned long list_lru_count_one(struct list_lru *lru,
 				 int nid, struct mem_cgroup *memcg)
 {
@@ -338,7 +324,6 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
 		    list_lru_walk_cb isolate, void *cb_arg,
 		    unsigned long *nr_to_walk, bool irq_off)
 {
-	struct list_lru_node *nlru = &lru->node[nid];
 	struct list_lru_one *l = NULL;
 	struct list_head *item, *n;
 	unsigned long isolated = 0;
@@ -359,7 +344,7 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
 			break;
 		--*nr_to_walk;
 
-		ret = isolate(item, l, cb_arg);
+		ret = isolate(lru, l, item, nid, cb_arg);
 		switch (ret) {
 		/*
 		 * LRU_RETRY, LRU_REMOVED_RETRY and LRU_STOP will drop the lru
@@ -371,7 +356,6 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
 			fallthrough;
 		case LRU_REMOVED:
 			isolated++;
-			atomic_long_dec(&nlru->nr_items);
 			if (ret == LRU_REMOVED_RETRY)
 				goto restart;
 			break;
diff --git a/mm/shmem.c b/mm/shmem.c
index 3c0c7edd2020..e4c1d9dfcfa2 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -852,8 +852,9 @@ struct shmem_unused_huge_scan {
 	struct shrink_control *sc;
 };
 
-static enum lru_status shmem_unused_huge_isolate(struct list_head *item,
-						 struct list_lru_one *lru,
+static enum lru_status shmem_unused_huge_isolate(struct list_lru *lru,
+						 struct list_lru_one *l,
+						 struct list_head *item, int nid,
 						 void *arg)
 {
 	struct shmem_unused_huge_scan *scan = arg;
@@ -874,14 +875,14 @@ static enum lru_status shmem_unused_huge_isolate(struct list_head *item,
 	inode = igrab(&info->vfs_inode);
 	/* inode is about to be evicted */
 	if (!inode) {
-		list_lru_isolate(lru, item);
+		__list_lru_del(lru, l, item, nid);
 		objcg = shmem_get_and_clear_objcg(info);
 		spin_unlock(&info->lock);
 		obj_cgroup_put(objcg);
 		return LRU_REMOVED;
 	}
 
-	list_lru_isolate(lru, item);
+	__list_lru_del(lru, l, item, nid);
 	objcg = shmem_get_and_clear_objcg(info);
 	set_shmem_unused_huge_isolated(info);
 	list_add_tail(&info->shrinklist, &scan->list);
diff --git a/mm/workingset.c b/mm/workingset.c
index 1504f91cdca5..c5d5690bda9f 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -706,9 +706,10 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker,
 	return nodes - max_nodes;
 }
 
-static enum lru_status shadow_lru_isolate(struct list_head *item,
-					  struct list_lru_one *lru,
-					  void *arg) __must_hold(lru->lock)
+static enum lru_status shadow_lru_isolate(struct list_lru *lru,
+					  struct list_lru_one *l,
+					  struct list_head *item, int nid,
+					  void *arg) __must_hold(l->lock)
 {
 	struct xa_node *node = container_of(item, struct xa_node, private_list);
 	struct address_space *mapping;
@@ -717,20 +718,20 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
 	/*
 	 * Page cache insertions and deletions synchronously maintain
 	 * the shadow node LRU under the i_pages lock and the
-	 * &lru->lock. Because the page cache tree is emptied before
-	 * the inode can be destroyed, holding the &lru->lock pins any
+	 * &l->lock. Because the page cache tree is emptied before
+	 * the inode can be destroyed, holding the &l->lock pins any
 	 * address_space that has nodes on the LRU.
 	 *
 	 * We can then safely transition to the i_pages lock to
 	 * pin only the address_space of the particular node we want
-	 * to reclaim, take the node off-LRU, and drop the &lru->lock.
+	 * to reclaim, take the node off-LRU, and drop the &l->lock.
 	 */
 
 	mapping = container_of(node->array, struct address_space, i_pages);
 
 	/* Coming from the list, invert the lock order */
 	if (!xa_trylock(&mapping->i_pages)) {
-		spin_unlock_irq(&lru->lock);
+		spin_unlock_irq(&l->lock);
 		ret = LRU_RETRY;
 		goto out;
 	}
@@ -739,16 +740,16 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
 	if (mapping->host != NULL) {
 		if (!spin_trylock(&mapping->host->i_lock)) {
 			xa_unlock(&mapping->i_pages);
-			spin_unlock_irq(&lru->lock);
+			spin_unlock_irq(&l->lock);
 			ret = LRU_RETRY;
 			goto out;
 		}
 	}
 
-	list_lru_isolate(lru, item);
+	__list_lru_del(lru, l, item, nid);
 	__dec_node_page_state(virt_to_page(node), WORKINGSET_NODES);
 
-	spin_unlock(&lru->lock);
+	spin_unlock(&l->lock);
 
 	/*
 	 * The nodes should only contain one or more shadow entries,
diff --git a/mm/zswap.c b/mm/zswap.c
index ad01c691a263..72eae0745b62 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1177,8 +1177,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
  *    can expect from writeback. We scale down the number of objects available
  *    for reclaim by this ratio.
  */
-static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_one *l,
-				       void *arg)
+static enum lru_status shrink_memcg_cb(struct list_lru *lru, struct list_lru_one *l,
+				       struct list_head *item, int nid, void *arg)
 {
 	struct zswap_entry *entry = container_of(item, struct zswap_entry, lru);
 	bool *encountered_page_in_swapcache = (bool *)arg;
-- 
2.56.0

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Yosry Ahmed]

On Wed, Oct 28, 2026 at 2:10 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Wed, Oct 28, 2026 at 12:22:43PM -0700, Yosry Ahmed wrote:
> > On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> > >
> > > From: Kaitao Cheng <chengkaitao@kylinos.cn>
> > >
> > > Successful writeback calls zswap_entry_free(), which removes the entry
> > > from the LRU and decrements both the sublist and node item counts.
> > > shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> > > to decrement the node count again.
> >
> > I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
> > LRU_DELETED instead to simply avoid the decrement.
>
> It bugs me that it's the walker, not the callback, that adjusts the
> count. That composes poorly with the normal list manipulation API.
>
> How about pushing the accounting into the isolation callback, which
> can then simply use __list_lru_del()?
>
> The tradeoff is that more things need to be passed into the isolation
> callback, but that seems reasonable. It does make the change noisy,
> however, and we might want something like Kaitao's change as a
> backportable fix first regardless.

I actually had the same thought initially, why is the walker updating
the counts? Then I reached the same conclusion that it would be too
noisy :)

I didn't look closely at the code but I agree in principle that it's
better if the callback is responsible for updating the count.

>
> Thoughts?
>
> ---
>
> diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
> index b0c797523bc2..157256e52dd0 100644
> --- a/drivers/android/binder/page_range.rs
> +++ b/drivers/android/binder/page_range.rs
> @@ -24,7 +24,7 @@
>  use kernel::{
>      bindings,
>      error::Result,
> -    ffi::{c_ulong, c_void},
> +    ffi::{c_int, c_ulong, c_void},
>      mm::{virt, Mm, MmWithUser},
>      new_mutex, new_spinlock,
>      page::{Page, PAGE_SHIFT, PAGE_SIZE},
> @@ -684,8 +684,10 @@ fn drop(self: Pin<&mut Self>) {
>  /// Called by the shrinker.
>  #[no_mangle]
>  unsafe extern "C" fn rust_shrink_free_page(
> +    lru: *mut bindings::list_lru,
> +    l: *mut bindings::list_lru_one,
>      item: *mut bindings::list_head,
> -    lru: *mut bindings::list_lru_one,
> +    nid: c_int,
>      _cb_arg: *mut c_void,
>  ) -> bindings::lru_status {
>      // Fields that should survive after unlocking the lru lock.
> @@ -728,7 +730,7 @@ fn drop(self: Pin<&mut Self>) {
>          };
>
>          // SAFETY: The item is in this lru list, so it's okay to remove it.
> -        unsafe { bindings::list_lru_isolate(lru, item) };
> +        unsafe { bindings::__list_lru_del(lru, l, item, nid) };
>
>          // SAFETY: Both pointers are in bounds of the same allocation.
>          page_index = unsafe { info.offset_from(inner.pages) } as usize;
> @@ -746,7 +748,7 @@ fn drop(self: Pin<&mut Self>) {
>      }
>
>      // SAFETY: The lru lock is locked when this method is called.
> -    unsafe { bindings::spin_unlock(&raw mut (*lru).lock) };
> +    unsafe { bindings::spin_unlock(&raw mut (*l).lock) };
>
>      if let Some(vma) = check_vma(&vma_read, range_ptr) {
>          let user_page_addr = vma_addr + (page_index << PAGE_SHIFT);
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 1bf37023fed6..2e858c05e30b 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -345,13 +345,14 @@ static void ttm_pool_type_give(struct ttm_pool_type *pt, struct page *p)
>         mod_lruvec_page_state(p, NR_GPU_RECLAIM, num_pages);
>  }
>
> -static enum lru_status take_one_from_lru(struct list_head *item,
> -                                        struct list_lru_one *list,
> +static enum lru_status take_one_from_lru(struct list_lru *lru,
> +                                        struct list_lru_one *l,
> +                                        struct list_head *item, int nid,
>                                          void *cb_arg)
>  {
>         struct page **out_page = cb_arg;
>         struct page *p = container_of(item, struct page, lru);
> -       list_lru_isolate(list, item);
> +       __list_lru_del(lru, l, item, nid);
>
>         *out_page = p;
>         return LRU_REMOVED;
> @@ -387,13 +388,14 @@ static void ttm_pool_type_init(struct ttm_pool_type *pt, struct ttm_pool *pool,
>         spin_unlock(&shrinker_lock);
>  }
>
> -static enum lru_status pool_move_to_dispose_list(struct list_head *item,
> -                                                struct list_lru_one *list,
> +static enum lru_status pool_move_to_dispose_list(struct list_lru *lru,
> +                                                struct list_lru_one *l,
> +                                                struct list_head *item, int nid,
>                                                  void *cb_arg)
>  {
>         struct list_head *dispose = cb_arg;
>
> -       list_lru_isolate_move(list, item, dispose);
> +       __list_lru_move(lru, l, item, nid, dispose);
>
>         return LRU_REMOVED;
>  }
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 37d021d22a7d..ce8063c2f5a6 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -556,24 +556,26 @@ static void d_shrink_add(struct dentry *dentry, struct list_head *list)
>   * LRU lists entirely, while shrink_move moves it to the indicated
>   * private list.
>   */
> -static void d_lru_isolate(struct list_lru_one *lru, struct dentry *dentry)
> +static void d_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
> +                         struct dentry *dentry, int nid)
>  {
>         D_FLAG_VERIFY(dentry, DCACHE_LRU_LIST);
>         dentry->d_flags &= ~DCACHE_LRU_LIST;
>         this_cpu_dec(nr_dentry_unused);
>         if (d_is_negative(dentry))
>                 this_cpu_dec(nr_dentry_negative);
> -       list_lru_isolate(lru, &dentry->d_lru);
> +       __list_lru_del(lru, l, &dentry->d_lru, nid);
>  }
>
> -static void d_lru_shrink_move(struct list_lru_one *lru, struct dentry *dentry,
> +static void d_lru_shrink_move(struct list_lru *lru, struct list_lru_one *l,
> +                             struct dentry *dentry, int nid,
>                               struct list_head *list)
>  {
>         D_FLAG_VERIFY(dentry, DCACHE_LRU_LIST);
>         dentry->d_flags |= DCACHE_SHRINK_LIST;
>         if (d_is_negative(dentry))
>                 this_cpu_dec(nr_dentry_negative);
> -       list_lru_isolate_move(lru, &dentry->d_lru, list);
> +       __list_lru_move(lru, l, &dentry->d_lru, nid, list);
>  }
>
>  static void ___d_drop(struct dentry *dentry)
> @@ -1308,8 +1310,8 @@ void shrink_dentry_list(struct list_head *list)
>  }
>  EXPORT_SYMBOL(shrink_dentry_list);
>
> -static enum lru_status dentry_lru_isolate(struct list_head *item,
> -               struct list_lru_one *lru, void *arg)
> +static enum lru_status dentry_lru_isolate(struct list_lru *lru,
> +               struct list_lru_one *l, struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *freeable = arg;
>         struct dentry   *dentry = container_of(item, struct dentry, d_lru);
> @@ -1329,7 +1331,7 @@ static enum lru_status dentry_lru_isolate(struct list_head *item,
>          * another pass through the LRU.
>          */
>         if (dentry->d_lockref.count) {
> -               d_lru_isolate(lru, dentry);
> +               d_lru_isolate(lru, l, dentry, nid);
>                 spin_unlock(&dentry->d_lock);
>                 return LRU_REMOVED;
>         }
> @@ -1360,7 +1362,7 @@ static enum lru_status dentry_lru_isolate(struct list_head *item,
>                 return LRU_ROTATE;
>         }
>
> -       d_lru_shrink_move(lru, dentry, freeable);
> +       d_lru_shrink_move(lru, l, dentry, nid, freeable);
>         spin_unlock(&dentry->d_lock);
>
>         return LRU_REMOVED;
> @@ -1389,8 +1391,8 @@ long prune_dcache_sb(struct super_block *sb, struct shrink_control *sc)
>         return freed;
>  }
>
> -static enum lru_status dentry_lru_isolate_shrink(struct list_head *item,
> -               struct list_lru_one *lru, void *arg)
> +static enum lru_status dentry_lru_isolate_shrink(struct list_lru *lru,
> +               struct list_lru_one *l, struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *freeable = arg;
>         struct dentry   *dentry = container_of(item, struct dentry, d_lru);
> @@ -1403,7 +1405,7 @@ static enum lru_status dentry_lru_isolate_shrink(struct list_head *item,
>         if (!spin_trylock(&dentry->d_lock))
>                 return LRU_SKIP;
>
> -       d_lru_shrink_move(lru, dentry, freeable);
> +       d_lru_shrink_move(lru, l, dentry, nid, freeable);
>         spin_unlock(&dentry->d_lock);
>
>         return LRU_REMOVED;
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index b2984fb7b35c..c8afe8236805 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -148,8 +148,8 @@ static void gfs2_qd_list_dispose(struct list_head *list)
>  }
>
>
> -static enum lru_status gfs2_qd_isolate(struct list_head *item,
> -               struct list_lru_one *lru, void *arg)
> +static enum lru_status gfs2_qd_isolate(struct list_lru *lru,
> +               struct list_lru_one *l, struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *dispose = arg;
>         struct gfs2_quota_data *qd =
> @@ -162,7 +162,7 @@ static enum lru_status gfs2_qd_isolate(struct list_head *item,
>         status = LRU_SKIP;
>         if (qd->qd_lockref.count == 0) {
>                 lockref_mark_dead(&qd->qd_lockref);
> -               list_lru_isolate_move(lru, &qd->qd_lru, dispose);
> +               __list_lru_move(lru, l, &qd->qd_lru, nid, dispose);
>                 status = LRU_REMOVED;
>         }
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 1cb6293b237a..675f4a8b9d95 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -929,8 +929,8 @@ EXPORT_SYMBOL_GPL(evict_inodes);
>   * LRU does not have strict ordering. Hence we don't want to reclaim inodes
>   * with this flag set because they are the inodes that are out of order.
>   */
> -static enum lru_status inode_lru_isolate(struct list_head *item,
> -               struct list_lru_one *lru, void *arg)
> +static enum lru_status inode_lru_isolate(struct list_lru *lru,
> +               struct list_lru_one *l, struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *freeable = arg;
>         struct inode    *inode = container_of(item, struct inode, i_lru);
> @@ -951,7 +951,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>         if (icount_read(inode) ||
>             (inode_state_read(inode) & ~I_REFERENCED) ||
>             !mapping_shrinkable(&inode->i_data)) {
> -               list_lru_isolate(lru, &inode->i_lru);
> +               __list_lru_del(lru, l, &inode->i_lru, nid);
>                 spin_unlock(&inode->i_lock);
>                 this_cpu_dec(nr_unused);
>                 return LRU_REMOVED;
> @@ -974,7 +974,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>
>                 inode_pin_lru_isolating(inode);
>                 spin_unlock(&inode->i_lock);
> -               spin_unlock(&lru->lock);
> +               spin_unlock(&l->lock);
>                 reap = invalidate_mapping_pages(&inode->i_data, 0, -1);
>                 if (current_is_kswapd())
>                         __count_vm_events(KSWAPD_INODESTEAL, reap);
> @@ -987,7 +987,7 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>
>         WARN_ON(inode_state_read(inode) & I_NEW);
>         inode_state_set(inode, I_FREEING);
> -       list_lru_isolate_move(lru, &inode->i_lru, freeable);
> +       __list_lru_move(lru, l, &inode->i_lru, nid, freeable);
>         spin_unlock(&inode->i_lock);
>
>         this_cpu_dec(nr_unused);
> diff --git a/fs/nfs/nfs42xattr.c b/fs/nfs/nfs42xattr.c
> index 37d79400e5f4..17eb14b40830 100644
> --- a/fs/nfs/nfs42xattr.c
> +++ b/fs/nfs/nfs42xattr.c
> @@ -801,8 +801,8 @@ static struct shrinker *nfs4_xattr_entry_shrinker;
>  static struct shrinker *nfs4_xattr_large_entry_shrinker;
>
>  static enum lru_status
> -cache_lru_isolate(struct list_head *item,
> -       struct list_lru_one *lru, void *arg)
> +cache_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
> +       struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *dispose = arg;
>         struct inode *inode;
> @@ -827,7 +827,7 @@ cache_lru_isolate(struct list_head *item,
>         cache->inode = NULL;
>         NFS_I(inode)->xattr_cache = NULL;
>         NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_XATTR;
> -       list_lru_isolate(lru, &cache->lru);
> +       __list_lru_del(lru, l, &cache->lru, nid);
>
>         spin_unlock(&inode->i_lock);
>
> @@ -866,8 +866,8 @@ nfs4_xattr_cache_count(struct shrinker *shrink, struct shrink_control *sc)
>  }
>
>  static enum lru_status
> -entry_lru_isolate(struct list_head *item,
> -       struct list_lru_one *lru, void *arg)
> +entry_lru_isolate(struct list_lru *lru, struct list_lru_one *l,
> +       struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *dispose = arg;
>         struct nfs4_xattr_bucket *bucket;
> @@ -896,7 +896,7 @@ entry_lru_isolate(struct list_head *item,
>
>                 hlist_del_init(&entry->hnode);
>                 atomic_long_dec(&cache->nent);
> -               list_lru_isolate(lru, &entry->lru);
> +               __list_lru_del(lru, l, &entry->lru, nid);
>
>                 spin_unlock(&bucket->lock);
>         } else {
> @@ -907,7 +907,7 @@ entry_lru_isolate(struct list_head *item,
>                 kref_get(&entry->ref);
>
>                 cache->listxattr = NULL;
> -               list_lru_isolate(lru, &entry->lru);
> +               __list_lru_del(lru, l, &entry->lru, nid);
>
>                 spin_unlock(&cache->listxattr_lock);
>         }
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 17a94e6fcb15..6b9165e5c3c5 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -509,8 +509,10 @@ void nfsd_file_net_dispose(struct nfsd_net *nn)
>
>  /**
>   * nfsd_file_lru_cb - Examine an entry on the LRU list
> - * @item: LRU entry to examine
>   * @lru: controlling LRU
> + * @l: locked sublist of @lru that holds @item
> + * @item: LRU entry to examine
> + * @nid: node id of @l
>   * @arg: dispose list
>   *
>   * Return values:
> @@ -519,8 +521,8 @@ void nfsd_file_net_dispose(struct nfsd_net *nn)
>   *   %LRU_SKIP: @item cannot be evicted
>   */
>  static enum lru_status
> -nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
> -                void *arg)
> +nfsd_file_lru_cb(struct list_lru *lru, struct list_lru_one *l,
> +                struct list_head *item, int nid, void *arg)
>  {
>         struct list_head *head = arg;
>         struct nfsd_file *nf = list_entry(item, struct nfsd_file, nf_lru);
> @@ -554,7 +556,7 @@ nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
>
>         /* Refcount went to zero. Unhash it and queue it to the dispose list */
>         nfsd_file_unhash(nf);
> -       list_lru_isolate(lru, &nf->nf_lru);
> +       __list_lru_del(lru, l, &nf->nf_lru, nid);
>         list_add(&nf->nf_gc, head);
>         this_cpu_inc(nfsd_file_evictions);
>         trace_nfsd_file_gc_disposed(nf);
> @@ -562,8 +564,8 @@ nfsd_file_lru_cb(struct list_head *item, struct list_lru_one *lru,
>  }
>
>  static enum lru_status
> -nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
> -                void *arg)
> +nfsd_file_gc_cb(struct list_lru *lru, struct list_lru_one *l,
> +                struct list_head *item, int nid, void *arg)
>  {
>         struct nfsd_file *nf = list_entry(item, struct nfsd_file, nf_lru);
>
> @@ -576,7 +578,7 @@ nfsd_file_gc_cb(struct list_head *item, struct list_lru_one *lru,
>                 trace_nfsd_file_gc_aged(nf);
>                 return LRU_ROTATE;
>         }
> -       return nfsd_file_lru_cb(item, lru, arg);
> +       return nfsd_file_lru_cb(lru, l, item, nid, arg);
>  }
>
>  static void
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 6c93b4f5629c..436aca8f6619 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1467,8 +1467,10 @@ __xfs_buf_mark_corrupt(
>   */
>  static enum lru_status
>  xfs_buftarg_drain_rele(
> +       struct list_lru         *lru,
> +       struct list_lru_one     *l,
>         struct list_head        *item,
> -       struct list_lru_one     *lru,
> +       int                     nid,
>         void                    *arg)
>
>  {
> @@ -1485,7 +1487,7 @@ xfs_buftarg_drain_rele(
>         }
>
>         xfs_buf_kill(bp);
> -       list_lru_isolate_move(lru, item, dispose);
> +       __list_lru_move(lru, l, item, nid, dispose);
>         spin_unlock(&bp->b_lockref.lock);
>         return LRU_REMOVED;
>  }
> @@ -1560,8 +1562,10 @@ xfs_buftarg_drain(
>
>  static enum lru_status
>  xfs_buftarg_isolate(
> +       struct list_lru         *lru,
> +       struct list_lru_one     *l,
>         struct list_head        *item,
> -       struct list_lru_one     *lru,
> +       int                     nid,
>         void                    *arg)
>  {
>         struct xfs_buf          *bp = container_of(item, struct xfs_buf, b_lru);
> @@ -1581,7 +1585,7 @@ xfs_buftarg_isolate(
>          * yet.
>          */
>         if (bp->b_lockref.count > 0) {
> -               list_lru_isolate(lru, &bp->b_lru);
> +               __list_lru_del(lru, l, &bp->b_lru, nid);
>                 spin_unlock(&bp->b_lockref.lock);
>                 return LRU_REMOVED;
>         }
> @@ -1597,7 +1601,7 @@ xfs_buftarg_isolate(
>         }
>
>         xfs_buf_kill(bp);
> -       list_lru_isolate_move(lru, item, dispose);
> +       __list_lru_move(lru, l, item, nid, dispose);
>         spin_unlock(&bp->b_lockref.lock);
>         return LRU_REMOVED;
>  }
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index dd5011f578a6..3933dd222509 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -411,10 +411,12 @@ struct xfs_qm_isolate {
>
>  static enum lru_status
>  xfs_qm_dquot_isolate(
> +       struct list_lru         *lru,
> +       struct list_lru_one     *l,
>         struct list_head        *item,
> -       struct list_lru_one     *lru,
> +       int                     nid,
>         void                    *arg)
> -               __releases(&lru->lock) __acquires(&lru->lock)
> +               __releases(&l->lock) __acquires(&l->lock)
>  {
>         struct xfs_dquot        *dqp = container_of(item,
>                                                 struct xfs_dquot, q_lru);
> @@ -450,7 +452,7 @@ xfs_qm_dquot_isolate(
>                 XFS_STATS_INC(dqp->q_mount, xs_qm_dqwants);
>
>                 trace_xfs_dqreclaim_want(dqp);
> -               list_lru_isolate(lru, &dqp->q_lru);
> +               __list_lru_del(lru, l, &dqp->q_lru, nid);
>                 XFS_STATS_DEC(dqp->q_mount, xs_qm_dquot_unused);
>                 return LRU_REMOVED;
>         }
> @@ -473,7 +475,7 @@ xfs_qm_dquot_isolate(
>         lockref_mark_dead(&dqp->q_lockref);
>         spin_unlock(&dqp->q_lockref.lock);
>
> -       list_lru_isolate_move(lru, &dqp->q_lru, &isol->dispose);
> +       __list_lru_move(lru, l, &dqp->q_lru, nid, &isol->dispose);
>         XFS_STATS_DEC(dqp->q_mount, xs_qm_dquot_unused);
>         trace_xfs_dqreclaim_done(dqp);
>         XFS_STATS_INC(dqp->q_mount, xs_qm_dqreclaims);
> diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
> index a450fffe1550..f6d94474a4c7 100644
> --- a/include/linux/list_lru.h
> +++ b/include/linux/list_lru.h
> @@ -150,6 +150,15 @@ bool __list_lru_add(struct list_lru *lru, struct list_lru_one *l,
>  bool __list_lru_del(struct list_lru *lru, struct list_lru_one *l,
>                 struct list_head *item, int nid);
>
> +/* Take @item off the locked sublist @l and put it on a private list @head */
> +static inline void __list_lru_move(struct list_lru *lru, struct list_lru_one *l,
> +                                  struct list_head *item, int nid,
> +                                  struct list_head *head)
> +{
> +       __list_lru_del(lru, l, item, nid);
> +       list_add(item, head);
> +}
> +
>  /**
>   * list_lru_add: add an element to the lru list's tail
>   * @lru: the lru pointer
> @@ -271,12 +280,9 @@ static inline unsigned long list_lru_count(struct list_lru *lru)
>         return count;
>  }
>
> -void list_lru_isolate(struct list_lru_one *list, struct list_head *item);
> -void list_lru_isolate_move(struct list_lru_one *list, struct list_head *item,
> -                          struct list_head *head);
> -
> -typedef enum lru_status (*list_lru_walk_cb)(struct list_head *item,
> -               struct list_lru_one *list, void *cb_arg);
> +typedef enum lru_status (*list_lru_walk_cb)(struct list_lru *lru,
> +               struct list_lru_one *l, struct list_head *item, int nid,
> +               void *cb_arg);
>
>  /**
>   * list_lru_walk_one: walk a @lru, isolating and disposing freeable items.
> @@ -289,11 +295,14 @@ typedef enum lru_status (*list_lru_walk_cb)(struct list_head *item,
>   * @nr_to_walk: how many items to scan.
>   *
>   * This function will scan all elements in a particular @lru, calling the
> - * @isolate callback for each of those items, along with the current list
> - * spinlock and a caller-provided opaque. The @isolate callback can choose to
> + * @isolate callback for each of those items, along with the locked sublist
> + * and a caller-provided opaque. The @isolate callback can choose to
>   * drop the lock internally, but *must* return with the lock held. The callback
>   * will return an enum lru_status telling the @lru infrastructure what to
> - * do with the object being scanned.
> + * do with the object being scanned. A callback that takes the item off
> + * the lru does so with __list_lru_del() or __list_lru_move(), or with
> + * list_lru_del() once it has dropped the lock, and returns LRU_REMOVED
> + * or LRU_REMOVED_RETRY.
>   *
>   * Please note that @nr_to_walk does not mean how many objects will be freed,
>   * just how many objects will be scanned.
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index bd44d58e8684..87722e1a7eee 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4638,8 +4638,9 @@ static bool thp_underused(struct folio *folio)
>         return false;
>  }
>
> -static enum lru_status deferred_split_isolate(struct list_head *item,
> -                                             struct list_lru_one *lru,
> +static enum lru_status deferred_split_isolate(struct list_lru *lru,
> +                                             struct list_lru_one *l,
> +                                             struct list_head *item, int nid,
>                                               void *cb_arg)
>  {
>         struct folio *folio = container_of(item, struct folio, _deferred_list);
> @@ -4649,7 +4650,7 @@ static enum lru_status deferred_split_isolate(struct list_head *item,
>         if (!folio_try_get(folio))
>                 return LRU_SKIP;
>
> -       list_lru_isolate_move(lru, item, freeable);
> +       __list_lru_move(lru, l, item, nid, freeable);
>         return LRU_REMOVED;
>  }
>
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index d1d256822ccc..9469117aa424 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -218,6 +218,7 @@ bool __list_lru_del(struct list_lru *lru, struct list_lru_one *l,
>         }
>         return false;
>  }
> +EXPORT_SYMBOL_GPL(__list_lru_del);
>
>  /* The caller must ensure the memcg lifetime. */
>  bool list_lru_add(struct list_lru *lru, struct list_head *item, int nid,
> @@ -291,21 +292,6 @@ bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
>  }
>  EXPORT_SYMBOL_GPL(list_lru_del_obj);
>
> -void list_lru_isolate(struct list_lru_one *list, struct list_head *item)
> -{
> -       list_del_init(item);
> -       list->nr_items--;
> -}
> -EXPORT_SYMBOL_GPL(list_lru_isolate);
> -
> -void list_lru_isolate_move(struct list_lru_one *list, struct list_head *item,
> -                          struct list_head *head)
> -{
> -       list_move(item, head);
> -       list->nr_items--;
> -}
> -EXPORT_SYMBOL_GPL(list_lru_isolate_move);
> -
>  unsigned long list_lru_count_one(struct list_lru *lru,
>                                  int nid, struct mem_cgroup *memcg)
>  {
> @@ -338,7 +324,6 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
>                     list_lru_walk_cb isolate, void *cb_arg,
>                     unsigned long *nr_to_walk, bool irq_off)
>  {
> -       struct list_lru_node *nlru = &lru->node[nid];
>         struct list_lru_one *l = NULL;
>         struct list_head *item, *n;
>         unsigned long isolated = 0;
> @@ -359,7 +344,7 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
>                         break;
>                 --*nr_to_walk;
>
> -               ret = isolate(item, l, cb_arg);
> +               ret = isolate(lru, l, item, nid, cb_arg);
>                 switch (ret) {
>                 /*
>                  * LRU_RETRY, LRU_REMOVED_RETRY and LRU_STOP will drop the lru
> @@ -371,7 +356,6 @@ __list_lru_walk_one(struct list_lru *lru, int nid, struct mem_cgroup *memcg,
>                         fallthrough;
>                 case LRU_REMOVED:
>                         isolated++;
> -                       atomic_long_dec(&nlru->nr_items);
>                         if (ret == LRU_REMOVED_RETRY)
>                                 goto restart;
>                         break;
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 3c0c7edd2020..e4c1d9dfcfa2 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -852,8 +852,9 @@ struct shmem_unused_huge_scan {
>         struct shrink_control *sc;
>  };
>
> -static enum lru_status shmem_unused_huge_isolate(struct list_head *item,
> -                                                struct list_lru_one *lru,
> +static enum lru_status shmem_unused_huge_isolate(struct list_lru *lru,
> +                                                struct list_lru_one *l,
> +                                                struct list_head *item, int nid,
>                                                  void *arg)
>  {
>         struct shmem_unused_huge_scan *scan = arg;
> @@ -874,14 +875,14 @@ static enum lru_status shmem_unused_huge_isolate(struct list_head *item,
>         inode = igrab(&info->vfs_inode);
>         /* inode is about to be evicted */
>         if (!inode) {
> -               list_lru_isolate(lru, item);
> +               __list_lru_del(lru, l, item, nid);
>                 objcg = shmem_get_and_clear_objcg(info);
>                 spin_unlock(&info->lock);
>                 obj_cgroup_put(objcg);
>                 return LRU_REMOVED;
>         }
>
> -       list_lru_isolate(lru, item);
> +       __list_lru_del(lru, l, item, nid);
>         objcg = shmem_get_and_clear_objcg(info);
>         set_shmem_unused_huge_isolated(info);
>         list_add_tail(&info->shrinklist, &scan->list);
> diff --git a/mm/workingset.c b/mm/workingset.c
> index 1504f91cdca5..c5d5690bda9f 100644
> --- a/mm/workingset.c
> +++ b/mm/workingset.c
> @@ -706,9 +706,10 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker,
>         return nodes - max_nodes;
>  }
>
> -static enum lru_status shadow_lru_isolate(struct list_head *item,
> -                                         struct list_lru_one *lru,
> -                                         void *arg) __must_hold(lru->lock)
> +static enum lru_status shadow_lru_isolate(struct list_lru *lru,
> +                                         struct list_lru_one *l,
> +                                         struct list_head *item, int nid,
> +                                         void *arg) __must_hold(l->lock)
>  {
>         struct xa_node *node = container_of(item, struct xa_node, private_list);
>         struct address_space *mapping;
> @@ -717,20 +718,20 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
>         /*
>          * Page cache insertions and deletions synchronously maintain
>          * the shadow node LRU under the i_pages lock and the
> -        * &lru->lock. Because the page cache tree is emptied before
> -        * the inode can be destroyed, holding the &lru->lock pins any
> +        * &l->lock. Because the page cache tree is emptied before
> +        * the inode can be destroyed, holding the &l->lock pins any
>          * address_space that has nodes on the LRU.
>          *
>          * We can then safely transition to the i_pages lock to
>          * pin only the address_space of the particular node we want
> -        * to reclaim, take the node off-LRU, and drop the &lru->lock.
> +        * to reclaim, take the node off-LRU, and drop the &l->lock.
>          */
>
>         mapping = container_of(node->array, struct address_space, i_pages);
>
>         /* Coming from the list, invert the lock order */
>         if (!xa_trylock(&mapping->i_pages)) {
> -               spin_unlock_irq(&lru->lock);
> +               spin_unlock_irq(&l->lock);
>                 ret = LRU_RETRY;
>                 goto out;
>         }
> @@ -739,16 +740,16 @@ static enum lru_status shadow_lru_isolate(struct list_head *item,
>         if (mapping->host != NULL) {
>                 if (!spin_trylock(&mapping->host->i_lock)) {
>                         xa_unlock(&mapping->i_pages);
> -                       spin_unlock_irq(&lru->lock);
> +                       spin_unlock_irq(&l->lock);
>                         ret = LRU_RETRY;
>                         goto out;
>                 }
>         }
>
> -       list_lru_isolate(lru, item);
> +       __list_lru_del(lru, l, item, nid);
>         __dec_node_page_state(virt_to_page(node), WORKINGSET_NODES);
>
> -       spin_unlock(&lru->lock);
> +       spin_unlock(&l->lock);
>
>         /*
>          * The nodes should only contain one or more shadow entries,
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ad01c691a263..72eae0745b62 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1177,8 +1177,8 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
>   *    can expect from writeback. We scale down the number of objects available
>   *    for reclaim by this ratio.
>   */
> -static enum lru_status shrink_memcg_cb(struct list_head *item, struct list_lru_one *l,
> -                                      void *arg)
> +static enum lru_status shrink_memcg_cb(struct list_lru *lru, struct list_lru_one *l,
> +                                      struct list_head *item, int nid, void *arg)
>  {
>         struct zswap_entry *entry = container_of(item, struct zswap_entry, lru);
>         bool *encountered_page_in_swapcache = (bool *)arg;
> --
> 2.56.0

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Kaitao Cheng]

在 2026/10/29 05:10, Johannes Weiner 写道:
> On Wed, Oct 28, 2026 at 12:22:43PM -0700, Yosry Ahmed wrote:
>> On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>>>
>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>
>>> Successful writeback calls zswap_entry_free(), which removes the entry
>>> from the LRU and decrements both the sublist and node item counts.
>>> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
>>> to decrement the node count again.
>>
>> I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
>> LRU_DELETED instead to simply avoid the decrement.
> 
> It bugs me that it's the walker, not the callback, that adjusts the
> count. That composes poorly with the normal list manipulation API.
> 
> How about pushing the accounting into the isolation callback, which
> can then simply use __list_lru_del()?
> 
> The tradeoff is that more things need to be passed into the isolation
> callback, but that seems reasonable. It does make the change noisy,
> however, and we might want something like Kaitao's change as a
> backportable fix first regardless.

I tried three approaches.

The first was "pushing the accounting into the isolation callback", as
Johannes Weiner suggested. However, as Johannes's example below also shows,
this would require extensive changes across multiple subsystems. I doubted
that such a broad change would be accepted, so I dropped this approach.

The second was to add a new flag to enum lru_status, as Yosry Ahmed suggested,
to handle cases like zswap where the callback has already decremented the
count. However, the incorrect lru->node[nid].nr_items value in zswap is only
an accounting issue: I could not find any code that reads this counter for
zswap. Changing a generic interface to fix an accounting error with no
apparent functional impact did not seem worthwhile.

The third approach is the one implemented in this patch. It keeps the changes
within zswap and fixes the accounting issue, so I think it is appropriate.

> Thoughts?
> 
> ---
> 
> diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs
> index b0c797523bc2..157256e52dd0 100644
> --- a/drivers/android/binder/page_range.rs
> +++ b/drivers/android/binder/page_range.rs
> @@ -24,7 +24,7 @@
>  use kernel::{
>      bindings,
>      error::Result,
> -    ffi::{c_ulong, c_void},
> +    ffi::{c_int, c_ulong, c_void},
>      mm::{virt, Mm, MmWithUser},
>      new_mutex, new_spinlock,
>      page::{Page, PAGE_SHIFT, PAGE_SIZE},
> @@ -684,8 +684,10 @@ fn drop(self: Pin<&mut Self>) {
>  /// Called by the shrinker.
>  #[no_mangle]
>  unsafe extern "C" fn rust_shrink_free_page(
> +    lru: *mut bindings::list_lru,
> +    l: *mut bindings::list_lru_one,
>      item: *mut bindings::list_head,
> -    lru: *mut bindings::list_lru_one,
> +    nid: c_int,
>      _cb_arg: *mut c_void,
>  ) -> bindings::lru_status {
>      // Fields that should survive after unlocking the lru lock.
> @@ -728,7 +730,7 @@ fn drop(self: Pin<&mut Self>) {
>          };
>  
>          // SAFETY: The item is in this lru list, so it's okay to remove it.
> -        unsafe { bindings::list_lru_isolate(lru, item) };
> +        unsafe { bindings::__list_lru_del(lru, l, item, nid) };
>  
>          // SAFETY: Both pointers are in bounds of the same allocation.
>          page_index = unsafe { info.offset_from(inner.pages) } as usize;
> @@ -746,7 +748,7 @@ fn drop(self: Pin<&mut Self>) {
>      }
>  
>      // SAFETY: The lru lock is locked when this method is called.
>  >  ........
>  ........

-- 
Thanks
Kaitao Cheng

Re: [PATCH] mm/zswap: fix double decrement of the LRU node count on writeback

[Yosry Ahmed]

On Wed, Oct 28, 2026 at 9:23 PM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>
>
>
> 在 2026/10/29 05:10, Johannes Weiner 写道:
> > On Wed, Oct 28, 2026 at 12:22:43PM -0700, Yosry Ahmed wrote:
> >> On Wed, Oct 28, 2026 at 4:36 AM Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
> >>>
> >>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
> >>>
> >>> Successful writeback calls zswap_entry_free(), which removes the entry
> >>> from the LRU and decrements both the sublist and node item counts.
> >>> shrink_memcg_cb() then returns LRU_REMOVED_RETRY, causing the LRU walker
> >>> to decrement the node count again.
> >>
> >> I wonder if we have a valid use case for adding LRU_REMOVED_COUNTED or
> >> LRU_DELETED instead to simply avoid the decrement.
> >
> > It bugs me that it's the walker, not the callback, that adjusts the
> > count. That composes poorly with the normal list manipulation API.
> >
> > How about pushing the accounting into the isolation callback, which
> > can then simply use __list_lru_del()?
> >
> > The tradeoff is that more things need to be passed into the isolation
> > callback, but that seems reasonable. It does make the change noisy,
> > however, and we might want something like Kaitao's change as a
> > backportable fix first regardless.
>
> I tried three approaches.
>
> The first was "pushing the accounting into the isolation callback", as
> Johannes Weiner suggested. However, as Johannes's example below also shows,
> this would require extensive changes across multiple subsystems. I doubted
> that such a broad change would be accepted, so I dropped this approach.
>
> The second was to add a new flag to enum lru_status, as Yosry Ahmed suggested,
> to handle cases like zswap where the callback has already decremented the
> count. However, the incorrect lru->node[nid].nr_items value in zswap is only
> an accounting issue: I could not find any code that reads this counter for
> zswap. Changing a generic interface to fix an accounting error with no
> apparent functional impact did not seem worthwhile.
>
> The third approach is the one implemented in this patch. It keeps the changes
> within zswap and fixes the accounting issue, so I think it is appropriate.

I honestly think it's a problem with the list_lru API, as Johannes
said, walkers compose poorly with the API. So I prefer we either take
Johannes's suggestion or add LRU_DELETED, unless there's strong
objection to both ideas.

FWIW, we don't need a minimal fix for backports, I don't think this
needs to be backported at all. As Kaitao said, I don't think the wrong
list_lru count is being used anywhere for zswap.