On Tue, Aug 26, 2025 at 11:39:18AM -0400, Josef Bacik wrote:
> At evict_inodes() time, we no longer have SB_ACTIVE set, so we can
> easily go through the normal iput path to clear any inodes. Update
> dispose_list() to check how we need to free the inode, and then grab a
> full reference to the inode while we're looping through the remaining
> inodes, and simply iput them at the end.
>
> Since we're just calling iput we don't really care about the i_count on
> the inode at the current time. Remove the i_count checks and just call
> iput on every inode we find.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/inode.c | 26 +++++++++++---------------
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 399598e90693..ede9118bb649 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -933,7 +933,7 @@ static void evict(struct inode *inode)
> * Dispose-list gets a local list with local inodes in it, so it doesn't
> * need to worry about list corruption and SMP locks.
> */
> -static void dispose_list(struct list_head *head)
> +static void dispose_list(struct list_head *head, bool for_lru)
> {
> while (!list_empty(head)) {
> struct inode *inode;
> @@ -941,8 +941,12 @@ static void dispose_list(struct list_head *head)
> inode = list_first_entry(head, struct inode, i_lru);
> list_del_init(&inode->i_lru);
>
> - evict(inode);
> - iobj_put(inode);
> + if (for_lru) {
> + evict(inode);
> + iobj_put(inode);
> + } else {
> + iput(inode);
> + }
I would really like to see a transitionary comment here or at least some
more details in the commit. Something like:
Once the inode has gone through iput_final() and has ended up on
the LRU it's inode->i_count will have gone to zero but the LRU
still holds an inode->i_obj_count reference. So we need to evict
and put that i_obj_count reference when disposing the collected
inodes.
When the inodes have been collected via evict_inodes() e.g, on
umount() they will now be made to hold a full reference that we
can simply iput().
Calling iput() in this case is safe as we no longer have
SB_ACTIVE set and thus cannot be readded to the LRU.
I think that's easier to follow and better describes the change.
But, there's a wrinkle. It is not guaranteed that at evict_inodes() time
SB_ACTIVE isn't raised anymore. Afaict, both bch2_fs_bdev_mark_dead()
and fs_bdev_mark_dead() will be called with SB_ACTIVE set from the block
layer on device removal. So in that case iput() would add them back to
the LRU.
> cond_resched();
> }
> }
> @@ -964,21 +968,13 @@ void evict_inodes(struct super_block *sb)
> again:
> spin_lock(&sb->s_inode_list_lock);
> list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> - if (icount_read(inode))
> - continue;
> -
> spin_lock(&inode->i_lock);
> - if (icount_read(inode)) {
> - spin_unlock(&inode->i_lock);
> - continue;
> - }
> if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
> spin_unlock(&inode->i_lock);
> continue;
> }
>
> - inode->i_state |= I_FREEING;
> - iobj_get(inode);
> + __iget(inode);
> inode_lru_list_del(inode);
> spin_unlock(&inode->i_lock);
> list_add(&inode->i_lru, &dispose);
> @@ -991,13 +987,13 @@ void evict_inodes(struct super_block *sb)
> if (need_resched()) {
> spin_unlock(&sb->s_inode_list_lock);
> cond_resched();
> - dispose_list(&dispose);
> + dispose_list(&dispose, false);
> goto again;
> }
> }
> spin_unlock(&sb->s_inode_list_lock);
>
> - dispose_list(&dispose);
> + dispose_list(&dispose, false);
> }
> EXPORT_SYMBOL_GPL(evict_inodes);
>
> @@ -1108,7 +1104,7 @@ long prune_icache_sb(struct super_block *sb, struct shrink_control *sc)
>
> freed = list_lru_shrink_walk(&sb->s_inode_lru, sc,
> inode_lru_isolate, &freeable);
> - dispose_list(&freeable);
> + dispose_list(&freeable, true);
> return freed;
> }
>
> --
> 2.49.0
>
