On Wed, Oct 02, 2024 at 11:33:19AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> 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_.
