Re: [PATCH 2/7] vfs: add inode iteration superblock method

[Christoph Hellwig <hch@infradead.org>]

On Wed, Oct 02, 2024 at 11:33:19AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Add a new superblock method for iterating all cached inodes in the
> inode cache.

The method is added later, this just adds an abstraction.

> +/**
> + * super_iter_inodes - iterate all the cached inodes on a superblock
> + * @sb: superblock to iterate
> + * @iter_fn: callback to run on every inode found.
> + *
> + * This function iterates all cached inodes on a superblock that are not in
> + * the process of being initialised or torn down. It will run @iter_fn() with
> + * a valid, referenced inode, so it is safe for the caller to do anything
> + * it wants with the inode except drop the reference the iterator holds.
> + *
> + */

Spurious empty comment line above.

> +void super_iter_inodes_unsafe(struct super_block *sb, ino_iter_fn iter_fn,
> +		void *private_data)
> +{
> +	struct inode *inode;
> +	int ret;
> +
> +	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);
> +		if (ret == INO_ITER_ABORT)
> +			break;
> +	}

Looking at the entire series, splitting the helpers for the unsafe
vs safe iteration feels a bit of an odd API design given that the
INO_ITER_REFERENCED can be passed to super_iter_inodes, but is an
internal flag pass here to the file system method.  Not sure what
the best way to do it, but maybe just make super_iter_inodes
a wrapper that calls into the method if available, or
a generic_iter_inodes_unsafe if the unsafe flag is set, else
a plain generic_iter_inodes?

> +/* Inode iteration callback return values */
> +#define INO_ITER_DONE		0
> +#define INO_ITER_ABORT		1
> +
> +/* Inode iteration control flags */
> +#define INO_ITER_REFERENCED	(1U << 0)
> +#define INO_ITER_UNSAFE		(1U << 1)

Please adjust the naming a bit to make clear these are different
namespaces, e.g. INO_ITER_RET_ and INO_ITER_F_.