Re: [PATCH 1/3] vfs: add inode iteration method super_iter_inodes

[Huang Shijie]

在 Mon, Nov 09, 2026 at 11:43:19AM +0100，Jan Kara 写道：
> On Thu 15-10-26 17:38:52, Huang Shijie wrote:
> > +static bool super_iter_iget(struct inode *inode, int flags)
> > +{
> > +	bool ret = false;
> > +
> 
> Here I'd do a lockless icount_read_once(inode) check if INO_ITER_F_REFERENCED
> is set to avoid unnecessary i_lock round trip.
okay. I will add icount_read_once() in later version.

> 
> > +	spin_lock(&inode->i_lock);
> > +	if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE))
> > +		goto out_unlock;
> > +
> > +	if ((flags & INO_ITER_F_REFERENCED) && !icount_read(inode))
> > +		goto out_unlock;
> > +
> > +	__iget(inode);
> > +	ret = true;
> > +out_unlock:
> > +	spin_unlock(&inode->i_lock);
> > +	return ret;
> > +}
> > +
> > +static int generic_iter_inodes(struct super_block *sb, ino_iter_fn iter_fn,
> > +			       void *private_data, int flags)
> > +{
> > +	struct inode *inode, *old_inode = NULL;
> > +	int ret = INO_ITER_RET_DONE;
> > +
> > +	spin_lock(&sb->s_inode_list_lock);
> > +	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> > +		if (!super_iter_iget(inode, flags))
> > +			continue;
> > +
> > +		spin_unlock(&sb->s_inode_list_lock);
> > +		iput(old_inode);
> > +
> > +		ret = iter_fn(inode, private_data);
> > +
> > +		old_inode = inode;
> > +		if (ret == INO_ITER_RET_ABORT) {
> > +			ret = INO_ITER_RET_DONE;
> > +			goto out_loop;
> > +		}
> > +		if (ret < 0)
> > +			goto out_loop;
> > +
> > +		cond_resched();
> > +		spin_lock(&sb->s_inode_list_lock);
> > +	}
> > +	spin_unlock(&sb->s_inode_list_lock);
> > +out_loop:
> > +	iput(old_inode);
> > +	return ret;
> > +}
> > +
> > +/*
> > + * Unsafe iteration for internal VFS use only: @iter_fn runs in atomic
> > + * context with only an inode existence guarantee, so it must not block
> > + * and must perform its own inode state checks. Return INO_ITER_RET_ABORT
> > + * to stop the iteration early.
> > + */
> > +static int generic_iter_inodes_unsafe(struct super_block *sb, ino_iter_fn iter_fn,
> > +				      void *private_data)
> > +{
> > +	struct inode *inode;
> > +	int ret = INO_ITER_RET_DONE;
> > +
> > +	rcu_read_lock();
> > +	spin_lock(&sb->s_inode_list_lock);
> > +	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> > +		ret = iter_fn(inode, private_data);
> 
> After some thought, I'd probably make this function inline in some VFS
> internal header. That way the compiler can inline everything and avoid the
No problem. 
I will try to convert it to inline function later.

> extra costs of indirect function calls, etc. These iterators tend to be
> rather lightweight so it would avoid performance regressions from the
> conversion.
> 
> I was considering the same for the generic iterator as well but there I
> don't have so strong opinion as the function is already relatively big and
> the callbacks are usually more heavy. Might still be worth some more
> detailed investigation and performance measurements...
Yes.
This function is already big enough.

Let's keep it as it is.

Thanks
Huang Shijie