[PATCH] fs: rotate inodes whose page cache could not be dropped

[PATCH] fs: rotate inodes whose page cache could not be dropped

[Nikolaos Karaolidis]

When inode_lru_isolate() drops the page cache of an unused inode, it
has to unlock the list, so it returns LRU_RETRY and the walk restarts
at the head of the list, where the inode still is. If
invalidate_mapping_pages() could not empty the mapping, the restarted
walk strips the same inode again, and so on until nr_to_walk runs out.
Every walk of that list then ends on this inode, and nothing behind it
is reclaimed for as long as its cache stays.

Before commit 2a0629834cd8 ("vfs: Don't evict inode under the inode lru
traversing context") the walker pinned the inode with a reference, and
dropping it in iput_final() set I_REFERENCED, so the restarted walk
rotated the inode to the tail. Set I_REFERENCED again when the mapping
is still populated after the invalidation, in the same i_lock section
that clears I_LRU_ISOLATING. inode_unpin_lru_isolating() now expects
i_lock to be held, like inode_pin_lru_isolating().

On CONFIG_HIGHMEM kernels this can happen to any unused inode: a folio
is left behind if it is locked, under writeback, spliced into a pipe,
or still in another CPU's LRU-add batch after being read. Without
HIGHMEM it happens to DAX inodes whose only entry is at index 0, which
mapping_shrinkable() lets onto the LRU and invalidate_mapping_pages()
never removes.

Fixes: 2a0629834cd8 ("vfs: Don't evict inode under the inode lru traversing context")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Nikolaos Karaolidis <nick@karaolidis.com>
---

Found while working on reclaim of inodes pinned by page cache in
ZONE_MOVABLE.

Tested in QEMU/KVM on master af32da41b032 (v7.3-rc6-232), with lockdep
and KASAN enabled. A small init program makes one unused inode whose
page can't be invalidated, lets it reach the head of the inode LRU,
opens 2000 other files, does one "echo 2 > /proc/sys/vm/drop_caches"
and counts the inodes left via umount; kprobes count the
invalidate_mapping_pages() calls:

                              without patch           with patch
  i386 HIGHMEM, file read     2000 of 2000 left,      none left,
  on another CPU              10300 strips, 1 inode   4 strips
  i386 HIGHMEM, page          same                    same
  spliced into a pipe
  x86_64, ext4 -o dax,        same                    same
  file mmap()ed
  controls (no leftover,      none left               none left
  shadow entry at index 0)

2a0629834cd8^ behaves like the patched kernel, 2a0629834cd8 like
master. On 5.15.y x86_64, where every unused inode with page cache is
on the LRU, the read-on-another-CPU case blocks the walk as well.

With "echo 64M > /sys/fs/cgroup/memory.reclaim" instead of
drop_caches, the i386 pipe case does 262450 strips and frees nothing
without the patch, 40 strips and frees everything with it. In the DAX
case that write takes ~400ms with the patch instead of ~4ms:
invalidate_mapping_pages() counts the DAX entry it leaves behind as
reclaimed, so unpatched, the 64M are "reclaimed" by re-stripping one
inode in a tight loop while nothing is freed.

The reproducer is a single C file; I can post it.

An LLM was used to check the analysis against the code, write the
reproducer and draft the changelog; I have reviewed all of it.

 fs/inode.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index a9d37be390a1..1614a97ce953 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -593,12 +593,11 @@ static void inode_pin_lru_isolating(struct inode *inode)
 
 static void inode_unpin_lru_isolating(struct inode *inode)
 {
-	spin_lock(&inode->i_lock);
+	lockdep_assert_held(&inode->i_lock);
 	WARN_ON(!(inode_state_read(inode) & I_LRU_ISOLATING));
 	inode_state_clear(inode, I_LRU_ISOLATING);
 	/* Called with inode->i_lock which ensures memory ordering. */
 	inode_wake_up_bit(inode, __I_LRU_ISOLATING);
-	spin_unlock(&inode->i_lock);
 }
 
 static void inode_wait_for_lru_isolating(struct inode *inode)
@@ -981,7 +980,13 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 		else
 			__count_vm_events(PGINODESTEAL, reap);
 		mm_account_reclaimed_pages(reap);
+
+		spin_lock(&inode->i_lock);
+		/* The walk restarts here: don't retry leftovers right away */
+		if (!mapping_empty(&inode->i_data))
+			inode_state_set(inode, I_REFERENCED);
 		inode_unpin_lru_isolating(inode);
+		spin_unlock(&inode->i_lock);
 		return LRU_RETRY;
 	}
 

base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
-- 
2.55.0

Re: [PATCH] fs: rotate inodes whose page cache could not be dropped

[Zhihao Cheng]

在 2026/10/16 18:25, Nikolaos Karaolidis 写道:
> When inode_lru_isolate() drops the page cache of an unused inode, it
> has to unlock the list, so it returns LRU_RETRY and the walk restarts
> at the head of the list, where the inode still is. If
> invalidate_mapping_pages() could not empty the mapping, the restarted
> walk strips the same inode again, and so on until nr_to_walk runs out.
> Every walk of that list then ends on this inode, and nothing behind it
> is reclaimed for as long as its cache stays.
[...]
>   static void inode_wait_for_lru_isolating(struct inode *inode)
> @@ -981,7 +980,13 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>   		else
>   			__count_vm_events(PGINODESTEAL, reap);
>   		mm_account_reclaimed_pages(reap);
> +
> +		spin_lock(&inode->i_lock);
> +		/* The walk restarts here: don't retry leftovers right away */
> +		if (!mapping_empty(&inode->i_data))
> +			inode_state_set(inode, I_REFERENCED);
>   		inode_unpin_lru_isolating(inode);
> +		spin_unlock(&inode->i_lock);
>   		return LRU_RETRY;
>   	}

Hi Nikolaos,
I find a possible BUG_ON if this patch applied, details shown as:
    P1                          P2
prune_icache_sb
  lock(lru->lock)
  inode_lru_isolate
   if (!mapping_empty(&inode->i_data))
    unlock(lru->lock)
    spin_lock(&inode->i_lock)
    inode_state_set(inode, I_REFERENCED)
    inode_unpin_lru_isolating
    spin_unlock(&inode->i_lock)
                          unlink
                           iget_locked // find_inode_fast gets the inode
                           iput
                            iput_final
                             drop = 1
                             evict
                              BUG_ON(inode_state_read_once(inode) != 
(I_FREEING | I_CLEAR))  // inode state with I_REFERENCED

Could that be possible?

Re: [PATCH] fs: rotate inodes whose page cache could not be dropped

[Nick Karaolidis]

On Saturday, October 17th, 2026 at 8:43 AM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:

> Hi Nikolaos,
> I find a possible BUG_ON if this patch applied, details shown as:
>     P1                          P2
> prune_icache_sb
>   lock(lru->lock)
>   inode_lru_isolate
>    if (!mapping_empty(&inode->i_data))
>     unlock(lru->lock)
>     spin_lock(&inode->i_lock)
>     inode_state_set(inode, I_REFERENCED)
>     inode_unpin_lru_isolating
>     spin_unlock(&inode->i_lock)
>                           unlink
>                            iget_locked // find_inode_fast gets the inode
>                            iput
>                             iput_final
>                              drop = 1
>                              evict
>                               BUG_ON(inode_state_read_once(inode) !=
> (I_FREEING | I_CLEAR))  // inode state with I_REFERENCED
> 
> Could that be possible?

Hi Zhihao,

Thanks for looking at this. I don't think that BUG_ON can trigger.

- The patch sets I_REFERENCED in the same i_lock section that clears
  I_LRU_ISOLATING, and evict() waits in inode_wait_for_lru_isolating()
  before calling ->evict_inode(), so the walker can only set the flag
  before the filesystem's eviction runs.

- clear_inode(), which every ->evict_inode() calls, sets i_state to
  I_FREEING | I_CLEAR.

- Evicting an inode that has I_REFERENCED set already happens.
  iput_final() sets it through __inode_lru_list_add(inode, true) when
  the inode is already on the LRU.

Did I miss a path that checks i_state before clear_inode()?

Thanks,
Nikolaos

Re: [PATCH] fs: rotate inodes whose page cache could not be dropped

[Zhihao Cheng]

在 2026/10/19 16:50, Nick Karaolidis 写道:
> On Saturday, October 17th, 2026 at 8:43 AM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
>> Hi Nikolaos,
>> I find a possible BUG_ON if this patch applied, details shown as:
>>      P1                          P2
>> prune_icache_sb
>>    lock(lru->lock)
>>    inode_lru_isolate
>>     if (!mapping_empty(&inode->i_data))
>>      unlock(lru->lock)
>>      spin_lock(&inode->i_lock)
>>      inode_state_set(inode, I_REFERENCED)
>>      inode_unpin_lru_isolating
>>      spin_unlock(&inode->i_lock)
>>                            unlink
>>                             iget_locked // find_inode_fast gets the inode
>>                             iput
>>                              iput_final
>>                               drop = 1
>>                               evict
>>                                BUG_ON(inode_state_read_once(inode) !=
>> (I_FREEING | I_CLEAR))  // inode state with I_REFERENCED
>>
>> Could that be possible?
> 
> Hi Zhihao,
> 
> Thanks for looking at this. I don't think that BUG_ON can trigger.
> 
> - The patch sets I_REFERENCED in the same i_lock section that clears
>    I_LRU_ISOLATING, and evict() waits in inode_wait_for_lru_isolating()
>    before calling ->evict_inode(), so the walker can only set the flag
>    before the filesystem's eviction runs.
> 
> - clear_inode(), which every ->evict_inode() calls, sets i_state to
>    I_FREEING | I_CLEAR.
OK, I missunderstand the realization of 'inode_state_assign_raw(inode, 
I_FREEING | I_CLEAR)' in the clear_inode().
> 
> - Evicting an inode that has I_REFERENCED set already happens.
>    iput_final() sets it through __inode_lru_list_add(inode, true) when
>    the inode is already on the LRU.
> 
> Did I miss a path that checks i_state before clear_inode()?
> 
> Thanks,
> Nikolaos
> 
> .
> 

Re: [PATCH] fs: rotate inodes whose page cache could not be dropped

[Jan Kara]

On Fri 16-10-26 10:25:15, Nikolaos Karaolidis wrote:
> When inode_lru_isolate() drops the page cache of an unused inode, it
> has to unlock the list, so it returns LRU_RETRY and the walk restarts
> at the head of the list, where the inode still is. If
> invalidate_mapping_pages() could not empty the mapping, the restarted
> walk strips the same inode again, and so on until nr_to_walk runs out.
> Every walk of that list then ends on this inode, and nothing behind it
> is reclaimed for as long as its cache stays.
> 
> Before commit 2a0629834cd8 ("vfs: Don't evict inode under the inode lru
> traversing context") the walker pinned the inode with a reference, and
> dropping it in iput_final() set I_REFERENCED, so the restarted walk
> rotated the inode to the tail. Set I_REFERENCED again when the mapping
> is still populated after the invalidation, in the same i_lock section
> that clears I_LRU_ISOLATING. inode_unpin_lru_isolating() now expects
> i_lock to be held, like inode_pin_lru_isolating().
> 
> On CONFIG_HIGHMEM kernels this can happen to any unused inode: a folio
> is left behind if it is locked, under writeback, spliced into a pipe,
> or still in another CPU's LRU-add batch after being read. Without
> HIGHMEM it happens to DAX inodes whose only entry is at index 0, which
> mapping_shrinkable() lets onto the LRU and invalidate_mapping_pages()
> never removes.
> 
> Fixes: 2a0629834cd8 ("vfs: Don't evict inode under the inode lru traversing context")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Nikolaos Karaolidis <nick@karaolidis.com>

Looks sensible. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza


> diff --git a/fs/inode.c b/fs/inode.c
> index a9d37be390a1..1614a97ce953 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -593,12 +593,11 @@ static void inode_pin_lru_isolating(struct inode *inode)
>  
>  static void inode_unpin_lru_isolating(struct inode *inode)
>  {
> -	spin_lock(&inode->i_lock);
> +	lockdep_assert_held(&inode->i_lock);
>  	WARN_ON(!(inode_state_read(inode) & I_LRU_ISOLATING));
>  	inode_state_clear(inode, I_LRU_ISOLATING);
>  	/* Called with inode->i_lock which ensures memory ordering. */
>  	inode_wake_up_bit(inode, __I_LRU_ISOLATING);
> -	spin_unlock(&inode->i_lock);
>  }
>  
>  static void inode_wait_for_lru_isolating(struct inode *inode)
> @@ -981,7 +980,13 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>  		else
>  			__count_vm_events(PGINODESTEAL, reap);
>  		mm_account_reclaimed_pages(reap);
> +
> +		spin_lock(&inode->i_lock);
> +		/* The walk restarts here: don't retry leftovers right away */
> +		if (!mapping_empty(&inode->i_data))
> +			inode_state_set(inode, I_REFERENCED);
>  		inode_unpin_lru_isolating(inode);
> +		spin_unlock(&inode->i_lock);
>  		return LRU_RETRY;
>  	}
>  
> 
> base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] fs: rotate inodes whose page cache could not be dropped

[Zhihao Cheng]

在 2026/10/16 18:25, Nikolaos Karaolidis 写道:
> When inode_lru_isolate() drops the page cache of an unused inode, it
> has to unlock the list, so it returns LRU_RETRY and the walk restarts
> at the head of the list, where the inode still is. If
> invalidate_mapping_pages() could not empty the mapping, the restarted
> walk strips the same inode again, and so on until nr_to_walk runs out.
> Every walk of that list then ends on this inode, and nothing behind it
> is reclaimed for as long as its cache stays.
> 
> Before commit 2a0629834cd8 ("vfs: Don't evict inode under the inode lru
> traversing context") the walker pinned the inode with a reference, and
> dropping it in iput_final() set I_REFERENCED, so the restarted walk
> rotated the inode to the tail. Set I_REFERENCED again when the mapping
> is still populated after the invalidation, in the same i_lock section
> that clears I_LRU_ISOLATING. inode_unpin_lru_isolating() now expects
> i_lock to be held, like inode_pin_lru_isolating().
> 
> On CONFIG_HIGHMEM kernels this can happen to any unused inode: a folio
> is left behind if it is locked, under writeback, spliced into a pipe,
> or still in another CPU's LRU-add batch after being read. Without
> HIGHMEM it happens to DAX inodes whose only entry is at index 0, which
> mapping_shrinkable() lets onto the LRU and invalidate_mapping_pages()
> never removes.
> 
> Fixes: 2a0629834cd8 ("vfs: Don't evict inode under the inode lru traversing context")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Nikolaos Karaolidis <nick@karaolidis.com>
> ---
> 
> Found while working on reclaim of inodes pinned by page cache in
> ZONE_MOVABLE.
> 
> Tested in QEMU/KVM on master af32da41b032 (v7.3-rc6-232), with lockdep
> and KASAN enabled. A small init program makes one unused inode whose
> page can't be invalidated, lets it reach the head of the inode LRU,
> opens 2000 other files, does one "echo 2 > /proc/sys/vm/drop_caches"
> and counts the inodes left via umount; kprobes count the
> invalidate_mapping_pages() calls:
> 
>                                without patch           with patch
>    i386 HIGHMEM, file read     2000 of 2000 left,      none left,
>    on another CPU              10300 strips, 1 inode   4 strips
>    i386 HIGHMEM, page          same                    same
>    spliced into a pipe
>    x86_64, ext4 -o dax,        same                    same
>    file mmap()ed
>    controls (no leftover,      none left               none left
>    shadow entry at index 0)
> 
> 2a0629834cd8^ behaves like the patched kernel, 2a0629834cd8 like
> master. On 5.15.y x86_64, where every unused inode with page cache is
> on the LRU, the read-on-another-CPU case blocks the walk as well.
> 
> With "echo 64M > /sys/fs/cgroup/memory.reclaim" instead of
> drop_caches, the i386 pipe case does 262450 strips and frees nothing
> without the patch, 40 strips and frees everything with it. In the DAX
> case that write takes ~400ms with the patch instead of ~4ms:
> invalidate_mapping_pages() counts the DAX entry it leaves behind as
> reclaimed, so unpatched, the 64M are "reclaimed" by re-stripping one
> inode in a tight loop while nothing is freed.
> 
> The reproducer is a single C file; I can post it.
> 
> An LLM was used to check the analysis against the code, write the
> reproducer and draft the changelog; I have reviewed all of it.
> 
>   fs/inode.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/fs/inode.c b/fs/inode.c
> index a9d37be390a1..1614a97ce953 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -593,12 +593,11 @@ static void inode_pin_lru_isolating(struct inode *inode)
>   
>   static void inode_unpin_lru_isolating(struct inode *inode)
>   {
> -	spin_lock(&inode->i_lock);
> +	lockdep_assert_held(&inode->i_lock);
>   	WARN_ON(!(inode_state_read(inode) & I_LRU_ISOLATING));
>   	inode_state_clear(inode, I_LRU_ISOLATING);
>   	/* Called with inode->i_lock which ensures memory ordering. */
>   	inode_wake_up_bit(inode, __I_LRU_ISOLATING);
> -	spin_unlock(&inode->i_lock);
>   }
>   
>   static void inode_wait_for_lru_isolating(struct inode *inode)
> @@ -981,7 +980,13 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
>   		else
>   			__count_vm_events(PGINODESTEAL, reap);
>   		mm_account_reclaimed_pages(reap);
> +
> +		spin_lock(&inode->i_lock);
> +		/* The walk restarts here: don't retry leftovers right away */
> +		if (!mapping_empty(&inode->i_data))
> +			inode_state_set(inode, I_REFERENCED);
>   		inode_unpin_lru_isolating(inode);
> +		spin_unlock(&inode->i_lock);
>   		return LRU_RETRY;
>   	}
>   
> 
> base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
>