[PATCH 2/7] fs: introduce sb_for_each_inodes().
Jan Kara
jack at suse.cz
Thu Sep 10 17:47:22 UTC 2026
On Wed 09-09-26 17:01:07, Julian Sun wrote:
> Add sb_for_each_inodes() to share s_inodes traversal and preserve its
> position while s_inode_list_lock is dropped.
>
> Track active iterators on sb->s_inodes_iters and advance their saved
> positions before unlinking an inode. Callbacks manage inode references
> and per-inode work, allowing both normal walks and eviction to use the
> same interface.
>
> Signed-off-by: Julian Sun <sunjunchao at bytedance.com>
So I'm not generally opposed the to a superblock inode iterator idea but
what you have looks more complex than I'd expect. Also we shouldn't tie
that to the fix of the lockup in evict_inodes(). So first I'd just
concentrate on fixing that lockup, then we can have a look at the iterator
idea.
You can take some inspiration about inode iteration API from the Dave
Chinner's patch set [1]. That looked more like what I'd expect although
Christoph had some comments on it too.
Regarding the fix I'd just make evict_inodes() do what all other inode
iterators do when they decide to drop s_inode_list_lock - pin an inode in
the list by grabbing refcount and then resume from it.
Honza
[1] https://lore.kernel.org/all/20241002014017.3801899-1-david@fromorbit.com/
> ---
> fs/inode.c | 93 ++++++++++++++++++++++++++++++++++
> fs/super.c | 1 +
> include/linux/fs.h | 15 ++++++
> include/linux/fs/super_types.h | 3 +-
> 4 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index ba7da39be4a3..b4279063a5dd 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -69,6 +69,15 @@ const struct address_space_operations empty_aops = {
> };
> EXPORT_SYMBOL(empty_aops);
>
> +struct inode_iter {
> + struct list_head iters_node; /* sb->s_inodes_iters */
> + struct list_head *next; /* next node going to iterate */
> + unsigned int flags;
> + inode_iter_cb func;
> + void *data;
> + int ret;
> +};
> +
> static DEFINE_PER_CPU(unsigned long, nr_inodes);
> static DEFINE_PER_CPU(unsigned long, nr_unused);
>
> @@ -641,12 +650,96 @@ void inode_sb_list_add(struct inode *inode)
> }
> EXPORT_SYMBOL_GPL(inode_sb_list_add);
>
> +static void inode_sb_iter_start(struct super_block *sb, struct inode_iter *it,
> + unsigned int flags, inode_iter_cb fn, void *data)
> +{
> + it->flags = flags;
> + it->func = fn;
> + it->data = data;
> + it->ret = 0;
> + spin_lock(&sb->s_inode_list_lock);
> + it->next = sb->s_inodes.next;
> + list_add(&it->iters_node, &sb->s_inodes_iters);
> +}
> +
> +static void inode_sb_iter_end(struct inode_iter *it, struct super_block *sb)
> +{
> + list_del(&it->iters_node);
> + spin_unlock(&sb->s_inode_list_lock);
> +}
> +
> +static bool inode_sb_iter_next(struct inode_iter *it, struct super_block *sb)
> +{
> + struct inode *inode = NULL;
> + int ret;
> +
> + while (!inode && it->next != &sb->s_inodes) {
> + inode = list_entry(it->next, struct inode, i_sb_list);
> + if (it->flags & INODE_ITER_UNUSED) {
> + if (icount_read_once(inode)) {
> + it->next = it->next->next;
> + continue;
> + }
> +
> + spin_lock(&inode->i_lock);
> + if (icount_read(inode)) {
> + spin_unlock(&inode->i_lock);
> + it->next = it->next->next;
> + continue;
> + }
> + } else {
> + spin_lock(&inode->i_lock);
> + }
> +
> + if ((it->flags & INODE_ITER_NORMAL) &&
> + (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE))) {
> + spin_unlock(&inode->i_lock);
> + it->next = it->next->next;
> + continue;
> + }
> +
> + it->next = it->next->next;
> + ret = it->func(inode, it->data);
> + if (ret) {
> + it->ret = ret;
> + return false;
> + }
> +
> + if (need_resched()) {
> + spin_unlock(&sb->s_inode_list_lock);
> + cond_resched();
> + spin_lock(&sb->s_inode_list_lock);
> + }
> + }
> +
> + return it->next == &sb->s_inodes ? false : true;
> +}
> +
> +int sb_for_each_inodes(struct super_block *sb, unsigned int flags,
> + inode_iter_cb fn, void *data)
> +{
> + struct inode_iter it;
> +
> + inode_sb_iter_start(sb, &it, flags, fn, data);
> + while (inode_sb_iter_next(&it, sb))
> + ;
> + inode_sb_iter_end(&it, sb);
> +
> + return it.ret;
> +}
> +EXPORT_SYMBOL(sb_for_each_inodes);
> +
> static inline void inode_sb_list_del(struct inode *inode)
> {
> struct super_block *sb = inode->i_sb;
> + struct inode_iter *it;
>
> if (!list_empty(&inode->i_sb_list)) {
> spin_lock(&sb->s_inode_list_lock);
> + list_for_each_entry(it, &sb->s_inodes_iters, iters_node) {
> + if (it->next == &inode->i_sb_list)
> + it->next = inode->i_sb_list.next;
> + }
> list_del_init(&inode->i_sb_list);
> spin_unlock(&sb->s_inode_list_lock);
> }
> diff --git a/fs/super.c b/fs/super.c
> index 05e443173038..3e069150c544 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -382,6 +382,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
> spin_lock_init(&s->s_roots_lock);
> mutex_init(&s->s_sync_lock);
> INIT_LIST_HEAD(&s->s_inodes);
> + INIT_LIST_HEAD(&s->s_inodes_iters);
> spin_lock_init(&s->s_inode_list_lock);
> INIT_LIST_HEAD(&s->s_inodes_wb);
> spin_lock_init(&s->s_inode_wblist_lock);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 09c4db5e9ae0..f3176ab10e65 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -870,6 +870,21 @@ struct inode {
> void *i_private; /* fs or device private pointer */
> } __randomize_layout;
>
> +enum inode_iter_flags_enum {
> + INODE_ITER_NORMAL = (1U << 1), /* Exclude inodes with (I_NEW | I_FREEING | I_WILL_FREE). */
> + INODE_ITER_UNUSED = (1U << 2), /* Only return inodes with (i_count == 0). */
> +};
> +
> +/*
> + * start end
> + * inode->i_lock locked unlocked
> + * sb->s_inode_list_lock locked locked
> + */
> +typedef int (*inode_iter_cb) (struct inode *, void *);
> +
> +int sb_for_each_inodes(struct super_block *sb, unsigned int flags,
> + inode_iter_cb fn, void *data);
> +
> /*
> * i_state handling
> *
> diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
> index ecd96aeb1cee..1f81cc219b8e 100644
> --- a/include/linux/fs/super_types.h
> +++ b/include/linux/fs/super_types.h
> @@ -269,9 +269,10 @@ struct super_block {
> */
> int s_stack_depth;
>
> - /* s_inode_list_lock protects s_inodes */
> + /* s_inode_list_lock protects s_inodes and s_inodes_iters */
> spinlock_t s_inode_list_lock ____cacheline_aligned_in_smp;
> struct list_head s_inodes; /* all inodes */
> + struct list_head s_inodes_iters; /* all iterators */
>
> spinlock_t s_inode_wblist_lock;
> struct list_head s_inodes_wb; /* writeback inodes */
> --
> 2.39.5
>
--
Jan Kara <jack at suse.com>
SUSE Labs, CR
More information about the Linux-security-module-archive
mailing list