Re: [PATCH 2/3] vfs: convert inode iterators to the unsafe super_iter_inodes()

[Huang Shijie]

在 Mon, Nov 09, 2026 at 11:08:00AM +0100，Jan Kara 写道：
> On Thu 15-10-26 17:38:53, Huang Shijie wrote:
> > Convert VFS internal superblock inode iterators that cannot use
> > referenced inodes to super_iter_inodes() with INO_ITER_F_UNSAFE set:
> >   - Dquot removal
> >   - inode eviction
> >   - The nr_blockdev_pages()
> >   - generic_shutdown_super()
> > 
> > Signed-off-by: Huang Shijie <huangsj@xxxxxxxx>
> 
> See some comments below.
> 
> > diff --git a/fs/inode.c b/fs/inode.c
> > index a9d37be390a1..139c56ee9d11 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -853,8 +853,11 @@ 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 bool dispose_list(struct list_head *head)
> >  {
> > +	if (list_empty(head))
> > +		return false;
> > +
> >  	while (!list_empty(head)) {
> >  		struct inode *inode;
> >  
> > @@ -864,6 +867,29 @@ static void dispose_list(struct list_head *head)
> >  		evict(inode);
> >  		cond_resched();
> >  	}
> > +	return true;
> > +}
> > +
> > +static int evict_inode_fn(struct inode *inode, void *data)
> > +{
> > +	struct list_head *dispose = data;
> > +
> 
> Here we miss the unlocked check
>             if (icount_read_once(inode))
>                     continue;
Thanks for pointing this.
	
> so every inode will now go through the i_lock roundtrip.
> 
> > +	spin_lock(&inode->i_lock);
> > +	if (icount_read(inode) ||
> > +	    (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE))) {
> > +		spin_unlock(&inode->i_lock);
> > +		return INO_ITER_RET_DONE;
> > +	}
> > +
> > +	inode_state_set(inode, I_FREEING);
> > +	inode_lru_list_del(inode);
> > +	spin_unlock(&inode->i_lock);
> > +	list_add(&inode->i_lru, dispose);
> > +
> > +	/* Abort so evict_inodes() can dispose of the queued inodes first. */
> > +	if (need_resched())
> > +		return INO_ITER_RET_ABORT;
> > +	return INO_ITER_RET_DONE;
> >  }
> 
> The need_resched() handling needs more thought here. The way you've done it
> will just abort the iteration, evict gathered inodes and then restart the
> whole iteration (which will then have to skip again all the inodes we could
> not evict). Which is not what evict_inodes() does today and we really had
> to resort to those tricks to avoid softlockups under certain loads.
> Frankly, this iteration is so special that I'd be willing to leave it
> unconverted for now rather than trying to bend the general iterator for its
> needs.
okay. I will think over it again.

If I really cannot find a good way to avoid the softlockups,
I will keep the code unconverted.

Thanks
Huang Shijie