* [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops
@ 2026-09-09 9:01 Julian Sun
2026-09-09 9:01 ` [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h Julian Sun
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
[Motivation]
We observed hung tasks in production during disk hotplug operations. A
kernel thread spends a long time in evict_inodes() while holding s_umount,
blocking other users of that lock and causing further stalls.
The problem is that evict_inodes() restarts its walk from the head of
sb->s_inodes every time it reschedules. When a large number of referenced
inodes remain near the head, each restart scans those inodes again without
making progress through that part of the list. The repeated scans can
delay eviction long enough to trigger hung-task reports. This is the same
problem that [1] attempted to address.
[Approach]
This series introduces sb_for_each_inodes() for two purposes:
1. Consolidate open-coded s_inodes walks behind a common entry point.
2. Retain each walk's position across drops of s_inode_list_lock.
The iterator mechanism follows the approach used by cgroup task iteration,
such as css_task_iter_next(). Active iterators are registered on a separate
list, sb->s_inodes_iters. Before removing an inode from s_inodes, the
removal path advances any iterator whose next position points to that
inode. These updates are protected by s_inode_list_lock, so a walker can
drop the lock and later resume from its saved position.
Existing walkers, such as drop_pagecache_sb() and add_dquot_ref(), already
contain their own position-preserving logic: they carry an inode reference
across iterations so that they can resume after dropping the list lock.
Moving that responsibility into sb_for_each_inodes() simplifies these
callers and lets their callbacks focus on the per-inode work.
Patch 1 removes trailing whitespace from include/linux/fs.h.
Patch 2 introduces sb_for_each_inodes().
The remaining patches convert existing walks to the new interface.
remove_dquot_ref() and nr_blockdev_pages() are left unchanged: their
walks are simple and do not require the inode->i_lock locking imposed
by the callback interface. Converting them would add an unnecessary
lock/unlock overhead for every inode.
[Testing]
I tested this series with approximately 20 hours of xfstests case
execution, repeatedly running the auto group on ext4 and XFS, no new
issues were observed. And with this patch applied, the hung task that
previously occurred on every run no longer occurs.
[1] https://lore.kernel.org/all/20241118114508.1405494-1-yebin@huaweicloud.com/
Julian Sun (7):
fs: remove trailing whitespace from include/linux/fs.h
fs: introduce sb_for_each_inodes().
block: use sb_for_each_inodes() in sync_bdevs()
fs: use sb_for_each_inodes() API.
gfs2: use sb_for_each_inodes() for cooperative eviction
quota: use sb_for_each_inodes() in add_dquot_ref()
landlock: use sb_for_each_inodes() when detaching a superblock
block/bdev.c | 85 +++++++++----------
fs/drop_caches.c | 44 +++++-----
fs/gfs2/ops_fstype.c | 38 ++++-----
fs/inode.c | 142 +++++++++++++++++++++++--------
fs/quota/dquot.c | 72 ++++++----------
fs/super.c | 1 +
include/linux/fs.h | 29 +++++--
include/linux/fs/super_types.h | 3 +-
security/landlock/fs.c | 150 +++++++++++++--------------------
9 files changed, 295 insertions(+), 269 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-10 16:52 ` Jan Kara
2026-09-09 9:01 ` [PATCH 2/7] fs: introduce sb_for_each_inodes() Julian Sun
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Remove trailing whitespace before introducing the inode iterator API.
This avoids unrelated whitespace changes from automatic editor cleanup
in the following patches. This also fixes formatting issues in fs.h
reported by scripts/checkpatch.pl.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
include/linux/fs.h | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f9d1e05e8ae6..09c4db5e9ae0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -296,7 +296,7 @@ struct iattr {
*/
#define FILESYSTEM_MAX_STACK_DEPTH 2
-/**
+/**
* enum positive_aop_returns - aop return codes with specific semantics
*
* @AOP_WRITEPAGE_ACTIVATE: Informs the caller that page writeback has
@@ -306,7 +306,7 @@ struct iattr {
* be a candidate for writeback again in the near
* future. Other callers must be careful to unlock
* the page if they get this return. Returned by
- * writepage();
+ * writepage();
*
* @AOP_TRUNCATED_PAGE: The AOP method that was handed a locked page has
* unlocked it and the page might have been truncated.
@@ -1315,8 +1315,10 @@ struct file *get_file_active(struct file **f);
#define MAX_NON_LFS ((1UL<<31) - 1)
-/* Page cache limit. The filesystems should put that into their s_maxbytes
- limits, otherwise bad things can happen in VM. */
+/*
+ * Page cache limit. The filesystems should put that into their s_maxbytes
+ * limits, otherwise bad things can happen in VM.
+ */
#if BITS_PER_LONG==32
#define MAX_LFS_FILESIZE ((loff_t)ULONG_MAX << PAGE_SHIFT)
#elif BITS_PER_LONG==64
@@ -2285,7 +2287,7 @@ int sync_inode_metadata(struct inode *inode, int wait);
struct file_system_type {
const char *name;
int fs_flags;
-#define FS_REQUIRES_DEV 1
+#define FS_REQUIRES_DEV 1
#define FS_BINARY_MOUNTDATA 2
#define FS_HAS_SUBTYPE 4
#define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */
@@ -2907,7 +2909,7 @@ ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos);
extern ssize_t kernel_write(struct file *, const void *, size_t, loff_t *);
extern ssize_t __kernel_write(struct file *, const void *, size_t, loff_t *);
extern struct file * open_exec(const char *);
-
+
/* fs/dcache.c -- generic fs support functions */
extern bool is_subdir(struct dentry *, struct dentry *);
extern bool path_is_under(const struct path *, const struct path *);
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/7] fs: introduce sb_for_each_inodes().
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
2026-09-09 9:01 ` [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-10 17:47 ` Jan Kara
2026-09-09 9:01 ` [PATCH 3/7] block: use sb_for_each_inodes() in sync_bdevs() Julian Sun
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
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@bytedance.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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/7] block: use sb_for_each_inodes() in sync_bdevs()
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
2026-09-09 9:01 ` [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h Julian Sun
2026-09-09 9:01 ` [PATCH 2/7] fs: introduce sb_for_each_inodes() Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-09 9:01 ` [PATCH 4/7] fs: use sb_for_each_inodes() API Julian Sun
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Convert sync_bdevs() to sb_for_each_inodes() and move device writeback
into a callback. The iterator preserves the walk position, allowing the
callback to iput() its inode before reacquiring the list lock and removing
the old_inode reference carried across iterations.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
block/bdev.c | 85 +++++++++++++++++++++++++---------------------------
1 file changed, 41 insertions(+), 44 deletions(-)
diff --git a/block/bdev.c b/block/bdev.c
index cd8323083740..ae472a062e51 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -1336,56 +1336,53 @@ void bdev_mark_dead(struct block_device *bdev, bool surprise)
*/
EXPORT_SYMBOL_GPL(bdev_mark_dead);
-void sync_bdevs(bool wait)
+static int sync_bdevs_inode_iter_cb(struct inode *inode, void *data)
{
- struct inode *inode, *old_inode = NULL;
-
- spin_lock(&blockdev_superblock->s_inode_list_lock);
- list_for_each_entry(inode, &blockdev_superblock->s_inodes, i_sb_list) {
- struct address_space *mapping = inode->i_mapping;
- struct block_device *bdev;
+ bool wait = (bool)data;
+ struct block_device *bdev;
+ struct address_space *mapping = inode->i_mapping;
- spin_lock(&inode->i_lock);
- if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW) ||
- mapping->nrpages == 0) {
- spin_unlock(&inode->i_lock);
- continue;
- }
- __iget(inode);
+ if (mapping->nrpages == 0) {
spin_unlock(&inode->i_lock);
- spin_unlock(&blockdev_superblock->s_inode_list_lock);
+ return 0;
+ }
+
+ /*
+ * We hold a reference to 'inode' so it couldn't have been
+ * removed from s_inodes list while we dropped the
+ * s_inode_list_lock.
+ */
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ spin_unlock(&blockdev_superblock->s_inode_list_lock);
+ bdev = I_BDEV(inode);
+
+ mutex_lock(&bdev->bd_disk->open_mutex);
+ if (!atomic_read(&bdev->bd_openers)) {
+ ; /* skip */
+ } else if (wait) {
/*
- * We hold a reference to 'inode' so it couldn't have been
- * removed from s_inodes list while we dropped the
- * s_inode_list_lock We cannot iput the inode now as we can
- * be holding the last reference and we cannot iput it under
- * s_inode_list_lock. So we keep the reference and iput it
- * later.
+ * We keep the error status of individual mapping so
+ * that applications can catch the writeback error using
+ * fsync(2). See filemap_fdatawait_keep_errors() for
+ * details.
*/
- iput(old_inode);
- old_inode = inode;
- bdev = I_BDEV(inode);
-
- mutex_lock(&bdev->bd_disk->open_mutex);
- if (!atomic_read(&bdev->bd_openers)) {
- ; /* skip */
- } else if (wait) {
- /*
- * We keep the error status of individual mapping so
- * that applications can catch the writeback error using
- * fsync(2). See filemap_fdatawait_keep_errors() for
- * details.
- */
- filemap_fdatawait_keep_errors(inode->i_mapping);
- } else {
- filemap_fdatawrite(inode->i_mapping);
- }
- mutex_unlock(&bdev->bd_disk->open_mutex);
-
- spin_lock(&blockdev_superblock->s_inode_list_lock);
+ filemap_fdatawait_keep_errors(inode->i_mapping);
+ } else {
+ filemap_fdatawrite(inode->i_mapping);
}
- spin_unlock(&blockdev_superblock->s_inode_list_lock);
- iput(old_inode);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
+ iput(inode);
+
+ spin_lock(&blockdev_superblock->s_inode_list_lock);
+
+ return 0;
+}
+
+void sync_bdevs(bool wait)
+{
+ sb_for_each_inodes(blockdev_superblock, INODE_ITER_NORMAL,
+ sync_bdevs_inode_iter_cb, (void *)wait);
}
/*
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/7] fs: use sb_for_each_inodes() API.
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
` (2 preceding siblings ...)
2026-09-09 9:01 ` [PATCH 3/7] block: use sb_for_each_inodes() in sync_bdevs() Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-09 9:01 ` [PATCH 5/7] gfs2: use sb_for_each_inodes() for cooperative eviction Julian Sun
` (3 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Use sb_for_each_inodes() in evict_inodes() to avoid rescanning referenced
inodes after each reschedule. Collect eligible inodes during the walk and
dispose of them after it completes.
Convert drop_pagecache_sb() as well, replacing its deferred toput_inode
reference with the iterator's position tracking.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
fs/drop_caches.c | 44 +++++++++++++++++++------------------------
fs/inode.c | 49 +++++++++++++-----------------------------------
2 files changed, 32 insertions(+), 61 deletions(-)
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 49f56a598ecb..0d475a5ff8cf 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -16,36 +16,30 @@
/* A global variable is a bit ugly, but it keeps the code simple */
static int sysctl_drop_caches;
-static void drop_pagecache_sb(struct super_block *sb, void *unused)
+static int drop_pagecache_inode_iter_cb(struct inode *inode, void *unused)
{
- struct inode *inode, *toput_inode = NULL;
+ struct super_block *sb = inode->i_sb;
- spin_lock(&sb->s_inode_list_lock);
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- spin_lock(&inode->i_lock);
- /*
- * We must skip inodes in unusual state. We may also skip
- * inodes without pages but we deliberately won't in case
- * we need to reschedule to avoid softlockups.
- */
- if ((inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) ||
- (mapping_empty(inode->i_mapping) && !need_resched())) {
- spin_unlock(&inode->i_lock);
- continue;
- }
- __iget(inode);
+ if (mapping_empty(inode->i_mapping)) {
spin_unlock(&inode->i_lock);
- spin_unlock(&sb->s_inode_list_lock);
-
- invalidate_mapping_pages(inode->i_mapping, 0, -1);
- iput(toput_inode);
- toput_inode = inode;
-
- cond_resched();
- spin_lock(&sb->s_inode_list_lock);
+ return 0;
}
+
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&sb->s_inode_list_lock);
- iput(toput_inode);
+
+ invalidate_mapping_pages(inode->i_mapping, 0, -1);
+ iput(inode);
+
+ spin_lock(&sb->s_inode_list_lock);
+
+ return 0;
+}
+
+static void drop_pagecache_sb(struct super_block *sb, void *unused)
+{
+ sb_for_each_inodes(sb, INODE_ITER_NORMAL, drop_pagecache_inode_iter_cb, NULL);
}
static int drop_caches_sysctl_handler(const struct ctl_table *table, int write,
diff --git a/fs/inode.c b/fs/inode.c
index b4279063a5dd..07a5f48641af 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -959,6 +959,17 @@ static void dispose_list(struct list_head *head)
}
}
+static int evict_inodes_inode_iter_cb(struct inode *inode, void *data)
+{
+ struct list_head *dispose = (struct list_head *)data;
+
+ inode_state_set(inode, I_FREEING);
+ inode_lru_list_del(inode);
+ spin_unlock(&inode->i_lock);
+ list_add(&inode->i_lru, dispose);
+ return 0;
+}
+
/**
* evict_inodes - evict all evictable inodes for a superblock
* @sb: superblock to operate on
@@ -970,44 +981,10 @@ static void dispose_list(struct list_head *head)
*/
void evict_inodes(struct super_block *sb)
{
- struct inode *inode;
LIST_HEAD(dispose);
+ unsigned int flags = INODE_ITER_NORMAL | INODE_ITER_UNUSED;
-again:
- spin_lock(&sb->s_inode_list_lock);
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- if (icount_read_once(inode))
- continue;
-
- spin_lock(&inode->i_lock);
- if (icount_read(inode)) {
- spin_unlock(&inode->i_lock);
- continue;
- }
- if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE)) {
- spin_unlock(&inode->i_lock);
- continue;
- }
-
- inode_state_set(inode, I_FREEING);
- inode_lru_list_del(inode);
- spin_unlock(&inode->i_lock);
- list_add(&inode->i_lru, &dispose);
-
- /*
- * We can have a ton of inodes to evict at unmount time given
- * enough memory, check to see if we need to go to sleep for a
- * bit so we don't livelock.
- */
- if (need_resched()) {
- spin_unlock(&sb->s_inode_list_lock);
- cond_resched();
- dispose_list(&dispose);
- goto again;
- }
- }
- spin_unlock(&sb->s_inode_list_lock);
-
+ sb_for_each_inodes(sb, flags, evict_inodes_inode_iter_cb, &dispose);
dispose_list(&dispose);
}
EXPORT_SYMBOL_GPL(evict_inodes);
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/7] gfs2: use sb_for_each_inodes() for cooperative eviction
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
` (3 preceding siblings ...)
2026-09-09 9:01 ` [PATCH 4/7] fs: use sb_for_each_inodes() API Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-09 9:01 ` [PATCH 6/7] quota: use sb_for_each_inodes() in add_dquot_ref() Julian Sun
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Convert gfs2_evict_inodes() to sb_for_each_inodes(). Take and drop the
inode reference within the callback, with iput() outside the list lock,
to retain cooperative eviction. The iterator preserves the walk position
and removes the need for toput_inode.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
fs/gfs2/ops_fstype.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 718e0da7dfce..acdbf9ca44a3 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -996,7 +996,7 @@ static int gfs2_lm_mount(struct gfs2_sbd *sdp, int silent)
switch (token) {
case Opt_jid:
ret = match_int(&tmp[0], &option);
- if (ret || option < 0)
+ if (ret || option < 0)
goto hostdata_error;
if (test_and_clear_bit(SDF_NOJOURNALID, &sdp->sd_flags))
ls->ls_jid = option;
@@ -1719,6 +1719,19 @@ static int gfs2_meta_init_fs_context(struct fs_context *fc)
return 0;
}
+static int gfs2_evict_inode_iter_cb(struct inode *inode, void *unused)
+{
+ struct super_block *sb = inode->i_sb;
+
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ spin_unlock(&sb->s_inode_list_lock);
+
+ iput(inode);
+ spin_lock(&sb->s_inode_list_lock);
+ return 0;
+}
+
/**
* gfs2_evict_inodes - evict inodes cooperatively
* @sb: the superblock
@@ -1741,31 +1754,10 @@ static int gfs2_meta_init_fs_context(struct fs_context *fc)
*/
static void gfs2_evict_inodes(struct super_block *sb)
{
- struct inode *inode, *toput_inode = NULL;
struct gfs2_sbd *sdp = sb->s_fs_info;
set_bit(SDF_EVICTING, &sdp->sd_flags);
-
- spin_lock(&sb->s_inode_list_lock);
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- spin_lock(&inode->i_lock);
- if ((inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) &&
- !need_resched()) {
- spin_unlock(&inode->i_lock);
- continue;
- }
- __iget(inode);
- spin_unlock(&inode->i_lock);
- spin_unlock(&sb->s_inode_list_lock);
-
- iput(toput_inode);
- toput_inode = inode;
-
- cond_resched();
- spin_lock(&sb->s_inode_list_lock);
- }
- spin_unlock(&sb->s_inode_list_lock);
- iput(toput_inode);
+ sb_for_each_inodes(sb, INODE_ITER_NORMAL, gfs2_evict_inode_iter_cb, NULL);
}
static void gfs2_kill_sb(struct super_block *sb)
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/7] quota: use sb_for_each_inodes() in add_dquot_ref()
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
` (4 preceding siblings ...)
2026-09-09 9:01 ` [PATCH 5/7] gfs2: use sb_for_each_inodes() for cooperative eviction Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-09 9:01 ` [PATCH 7/7] landlock: use sb_for_each_inodes() when detaching a superblock Julian Sun
2026-09-09 12:49 ` [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Jan Kara
7 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Convert add_dquot_ref() to sb_for_each_inodes(), passing the quota type
to the callback and propagating initialization errors. This removes the
old_inode reference used to preserve the walk position.
Leave remove_dquot_ref() unchanged to avoid adding per-inode locking to a
walk that never drops s_inode_list_lock.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
fs/quota/dquot.c | 72 +++++++++++++++++++-----------------------------
1 file changed, 28 insertions(+), 44 deletions(-)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 204afc5e984b..77c68149d4ae 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1043,64 +1043,48 @@ static int dqinit_needed(struct inode *inode, int type)
return 0;
}
-/* This routine is guarded by s_umount semaphore */
-static int add_dquot_ref(struct super_block *sb, int type)
+static int add_dquot_ref_inode_iter_cb(struct inode *inode, void *data)
{
- struct inode *inode, *old_inode = NULL;
+ int type = *(int *)data;
+ int err = 0;
+ struct super_block *sb = inode->i_sb;
#ifdef CONFIG_QUOTA_DEBUG
int reserved = 0;
#endif
- int err = 0;
- spin_lock(&sb->s_inode_list_lock);
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- spin_lock(&inode->i_lock);
- if ((inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) ||
- !atomic_read(&inode->i_writecount) ||
- !dqinit_needed(inode, type)) {
- spin_unlock(&inode->i_lock);
- continue;
- }
- __iget(inode);
+ if (!atomic_read(&inode->i_writecount) ||
+ !dqinit_needed(inode, type)) {
spin_unlock(&inode->i_lock);
- spin_unlock(&sb->s_inode_list_lock);
+ return 0;
+ }
+
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ spin_unlock(&sb->s_inode_list_lock);
#ifdef CONFIG_QUOTA_DEBUG
- if (unlikely(inode_get_rsv_space(inode) > 0))
- reserved = 1;
+ if (unlikely(inode_get_rsv_space(inode) > 0))
+ reserved = 1;
#endif
- iput(old_inode);
- err = __dquot_initialize(inode, type);
- if (err) {
- iput(inode);
- goto out;
- }
-
- /*
- * We hold a reference to 'inode' so it couldn't have been
- * removed from s_inodes list while we dropped the
- * s_inode_list_lock. We cannot iput the inode now as we can be
- * holding the last reference and we cannot iput it under
- * s_inode_list_lock. So we keep the reference and iput it
- * later.
- */
- old_inode = inode;
- cond_resched();
- spin_lock(&sb->s_inode_list_lock);
- }
- spin_unlock(&sb->s_inode_list_lock);
- iput(old_inode);
-out:
+ err = __dquot_initialize(inode, type);
#ifdef CONFIG_QUOTA_DEBUG
- if (reserved) {
- quota_error(sb, "Writes happened before quota was turned on "
- "thus quota information is probably inconsistent. "
- "Please run quotacheck(8)");
- }
+ if (reserved)
+ quota_error(sb, "Writes happened before quota was turned "
+ "on thus quota information is probably "
+ "inconsistent. Please run quotacheck(8)");
#endif
+ iput(inode);
+ spin_lock(&sb->s_inode_list_lock);
return err;
}
+/* This routine is guarded by s_umount semaphore */
+static int add_dquot_ref(struct super_block *sb, int type)
+{
+ return sb_for_each_inodes(sb, INODE_ITER_NORMAL,
+ add_dquot_ref_inode_iter_cb, &type);
+}
+
static void remove_dquot_ref(struct super_block *sb, int type)
{
struct inode *inode;
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 7/7] landlock: use sb_for_each_inodes() when detaching a superblock
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
` (5 preceding siblings ...)
2026-09-09 9:01 ` [PATCH 6/7] quota: use sb_for_each_inodes() in add_dquot_ref() Julian Sun
@ 2026-09-09 9:01 ` Julian Sun
2026-09-09 12:49 ` [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Jan Kara
7 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 9:01 UTC (permalink / raw)
To: linux-block, linux-fsdevel, gfs2, linux-security-module
Cc: jack, agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
Convert hook_sb_delete() to sb_for_each_inodes() and release each
callback's inode reference outside s_inode_list_lock. This removes the
prev_inode reference used to preserve the walk position while retaining
the synchronization with release_inode() and the final wait for pending
inode releases.
Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
---
security/landlock/fs.c | 150 +++++++++++++++++------------------------
1 file changed, 60 insertions(+), 90 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 30aa6ce13590..3dcde8cbfb6a 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1369,110 +1369,80 @@ static void hook_inode_free_security_rcu(void *inode_security)
/* Super-block hooks */
-/*
- * Release the inodes used in a security policy.
- *
- * Cf. fsnotify_unmount_inodes() and evict_inodes()
- */
-static void hook_sb_delete(struct super_block *const sb)
+static int hook_sb_delete_inode_iter_cb(struct inode *inode, void *data)
{
- struct inode *inode, *prev_inode = NULL;
+ struct landlock_object *object;
+ struct super_block *sb = inode->i_sb;
- if (!landlock_initialized)
- return;
+ if (!atomic_read(&inode->i_count)) {
+ spin_unlock(&inode->i_lock);
+ return 0;
+ }
- spin_lock(&sb->s_inode_list_lock);
- list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- struct landlock_object *object;
+ rcu_read_lock();
+ object = rcu_dereference(landlock_inode(inode)->object);
+ if (!object) {
+ rcu_read_unlock();
+ spin_unlock(&inode->i_lock);
+ return 0;
+ }
+ /* Keeps a reference to this inode until the next loop walk. */
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
- /* Only handles referenced inodes. */
- if (!icount_read_once(inode))
- continue;
+ /*
+ * If there is no concurrent release_inode() ongoing, then we
+ * are in charge of calling iput() on this inode, otherwise we
+ * will just wait for it to finish.
+ */
+ spin_lock(&object->lock);
+ if (object->underobj == inode) {
+ object->underobj = NULL;
+ spin_unlock(&object->lock);
+ rcu_read_unlock();
/*
- * Protects against concurrent modification of inode (e.g.
- * from get_inode_object()).
+ * Because object->underobj was not NULL,
+ * release_inode() and get_inode_object() guarantee
+ * that it is safe to reset
+ * landlock_inode(inode)->object while it is not NULL.
+ * It is therefore not necessary to lock inode->i_lock.
*/
- spin_lock(&inode->i_lock);
+ rcu_assign_pointer(landlock_inode(inode)->object, NULL);
/*
- * Checks I_FREEING and I_WILL_FREE to protect against a race
- * condition when release_inode() just called iput(), which
- * could lead to a NULL dereference of inode->security or a
- * second call to iput() for the same Landlock object. Also
- * checks I_NEW because such inode cannot be tied to an object.
+ * At this point, we own the ihold() reference that was
+ * originally set up by get_inode_object() and the
+ * __iget() reference that we just set in this loop
+ * walk. Therefore there are at least two references
+ * on the inode.
*/
- if (inode_state_read(inode) &
- (I_FREEING | I_WILL_FREE | I_NEW)) {
- spin_unlock(&inode->i_lock);
- continue;
- }
+ iput_not_last(inode);
+ } else {
+ spin_unlock(&object->lock);
+ rcu_read_unlock();
+ }
- rcu_read_lock();
- object = rcu_dereference(landlock_inode(inode)->object);
- if (!object) {
- rcu_read_unlock();
- spin_unlock(&inode->i_lock);
- continue;
- }
- /* Keeps a reference to this inode until the next loop walk. */
- __iget(inode);
- spin_unlock(&inode->i_lock);
+ spin_unlock(&sb->s_inode_list_lock);
+ iput(inode);
+ spin_lock(&sb->s_inode_list_lock);
- /*
- * If there is no concurrent release_inode() ongoing, then we
- * are in charge of calling iput() on this inode, otherwise we
- * will just wait for it to finish.
- */
- spin_lock(&object->lock);
- if (object->underobj == inode) {
- object->underobj = NULL;
- spin_unlock(&object->lock);
- rcu_read_unlock();
+ return 0;
+}
- /*
- * Because object->underobj was not NULL,
- * release_inode() and get_inode_object() guarantee
- * that it is safe to reset
- * landlock_inode(inode)->object while it is not NULL.
- * It is therefore not necessary to lock inode->i_lock.
- */
- rcu_assign_pointer(landlock_inode(inode)->object, NULL);
- /*
- * At this point, we own the ihold() reference that was
- * originally set up by get_inode_object() and the
- * __iget() reference that we just set in this loop
- * walk. Therefore there are at least two references
- * on the inode.
- */
- iput_not_last(inode);
- } else {
- spin_unlock(&object->lock);
- rcu_read_unlock();
- }
+/*
+ * Release the inodes used in a security policy.
+ *
+ * Cf. fsnotify_unmount_inodes() and evict_inodes()
+ */
+static void hook_sb_delete(struct super_block *const sb)
+{
+ unsigned int flags = INODE_ITER_NORMAL;
- if (prev_inode) {
- /*
- * At this point, we still own the __iget() reference
- * that we just set in this loop walk. Therefore we
- * can drop the list lock and know that the inode won't
- * disappear from under us until the next loop walk.
- */
- spin_unlock(&sb->s_inode_list_lock);
- /*
- * We can now actually put the inode reference from the
- * previous loop walk, which is not needed anymore.
- */
- iput(prev_inode);
- cond_resched();
- spin_lock(&sb->s_inode_list_lock);
- }
- prev_inode = inode;
- }
- spin_unlock(&sb->s_inode_list_lock);
+ if (!landlock_initialized)
+ return;
+
+ sb_for_each_inodes(sb, flags, hook_sb_delete_inode_iter_cb, NULL);
- /* Puts the inode reference from the last loop walk, if any. */
- if (prev_inode)
- iput(prev_inode);
/* Waits for pending iput() in release_inode(). */
wait_var_event(&landlock_superblock(sb)->inode_refs,
!atomic_long_read(&landlock_superblock(sb)->inode_refs));
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
` (6 preceding siblings ...)
2026-09-09 9:01 ` [PATCH 7/7] landlock: use sb_for_each_inodes() when detaching a superblock Julian Sun
@ 2026-09-09 12:49 ` Jan Kara
2026-09-09 13:08 ` [External] " Julian Sun
7 siblings, 1 reply; 14+ messages in thread
From: Jan Kara @ 2026-09-09 12:49 UTC (permalink / raw)
To: Julian Sun
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, jack,
agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
On Wed 09-09-26 17:01:05, Julian Sun wrote:
> [Motivation]
>
> We observed hung tasks in production during disk hotplug operations. A
> kernel thread spends a long time in evict_inodes() while holding s_umount,
> blocking other users of that lock and causing further stalls.
>
> The problem is that evict_inodes() restarts its walk from the head of
> sb->s_inodes every time it reschedules. When a large number of referenced
> inodes remain near the head, each restart scans those inodes again without
> making progress through that part of the list. The repeated scans can
> delay eviction long enough to trigger hung-task reports. This is the same
> problem that [1] attempted to address.
Thanks fro the patch! What's really unexpected is that there are many
inodes with incremented refcount at the time when evict_inodes() is called.
Did you have a look who's holding those references? Or is evict_inodes()
called through the fs_bdev_mark_dead() call (as you mention "disk hotplug
operations")? There the filesystem is in fact in use so what you describe
makes some sense.
Honza
> [Approach]
>
> This series introduces sb_for_each_inodes() for two purposes:
>
> 1. Consolidate open-coded s_inodes walks behind a common entry point.
> 2. Retain each walk's position across drops of s_inode_list_lock.
>
> The iterator mechanism follows the approach used by cgroup task iteration,
> such as css_task_iter_next(). Active iterators are registered on a separate
> list, sb->s_inodes_iters. Before removing an inode from s_inodes, the
> removal path advances any iterator whose next position points to that
> inode. These updates are protected by s_inode_list_lock, so a walker can
> drop the lock and later resume from its saved position.
>
> Existing walkers, such as drop_pagecache_sb() and add_dquot_ref(), already
> contain their own position-preserving logic: they carry an inode reference
> across iterations so that they can resume after dropping the list lock.
> Moving that responsibility into sb_for_each_inodes() simplifies these
> callers and lets their callbacks focus on the per-inode work.
>
> Patch 1 removes trailing whitespace from include/linux/fs.h.
> Patch 2 introduces sb_for_each_inodes().
> The remaining patches convert existing walks to the new interface.
> remove_dquot_ref() and nr_blockdev_pages() are left unchanged: their
> walks are simple and do not require the inode->i_lock locking imposed
> by the callback interface. Converting them would add an unnecessary
> lock/unlock overhead for every inode.
>
> [Testing]
>
> I tested this series with approximately 20 hours of xfstests case
> execution, repeatedly running the auto group on ext4 and XFS, no new
> issues were observed. And with this patch applied, the hung task that
> previously occurred on every run no longer occurs.
>
> [1] https://lore.kernel.org/all/20241118114508.1405494-1-yebin@huaweicloud.com/
>
> Julian Sun (7):
> fs: remove trailing whitespace from include/linux/fs.h
> fs: introduce sb_for_each_inodes().
> block: use sb_for_each_inodes() in sync_bdevs()
> fs: use sb_for_each_inodes() API.
> gfs2: use sb_for_each_inodes() for cooperative eviction
> quota: use sb_for_each_inodes() in add_dquot_ref()
> landlock: use sb_for_each_inodes() when detaching a superblock
>
> block/bdev.c | 85 +++++++++----------
> fs/drop_caches.c | 44 +++++-----
> fs/gfs2/ops_fstype.c | 38 ++++-----
> fs/inode.c | 142 +++++++++++++++++++++++--------
> fs/quota/dquot.c | 72 ++++++----------
> fs/super.c | 1 +
> include/linux/fs.h | 29 +++++--
> include/linux/fs/super_types.h | 3 +-
> security/landlock/fs.c | 150 +++++++++++++--------------------
> 9 files changed, 295 insertions(+), 269 deletions(-)
>
> --
> 2.39.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External] Re: [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops
2026-09-09 12:49 ` [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Jan Kara
@ 2026-09-09 13:08 ` Julian Sun
0 siblings, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-09 13:08 UTC (permalink / raw)
To: Jan Kara
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, agruenba,
mic, gnoack, paul, jmorris, serge, aleksa, legion, djwong,
ebiggers, sandeen
On Wed, Sep 9, 2026 at 8:49 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 09-09-26 17:01:05, Julian Sun wrote:
> > [Motivation]
> >
> > We observed hung tasks in production during disk hotplug operations. A
> > kernel thread spends a long time in evict_inodes() while holding s_umount,
> > blocking other users of that lock and causing further stalls.
> >
> > The problem is that evict_inodes() restarts its walk from the head of
> > sb->s_inodes every time it reschedules. When a large number of referenced
> > inodes remain near the head, each restart scans those inodes again without
> > making progress through that part of the list. The repeated scans can
> > delay eviction long enough to trigger hung-task reports. This is the same
> > problem that [1] attempted to address.
>
> Thanks fro the patch! What's really unexpected is that there are many
> inodes with incremented refcount at the time when evict_inodes() is called.
> Did you have a look who's holding those references? Or is evict_inodes()
> called through the fs_bdev_mark_dead() call (as you mention "disk hotplug
> operations")? There the filesystem is in fact in use so what you describe
> makes some sense.
Yes, the filesystem was still in use, and the disk was unplugged
directly without being unmounted. As a result, `fs_bdev_mark_dead()`
holds the `s_umount` lock and performs a large number of redundant
traversals, which takes a significant amount of time.
>
> Honza
>
> > [Approach]
> >
> > This series introduces sb_for_each_inodes() for two purposes:
> >
> > 1. Consolidate open-coded s_inodes walks behind a common entry point.
> > 2. Retain each walk's position across drops of s_inode_list_lock.
> >
> > The iterator mechanism follows the approach used by cgroup task iteration,
> > such as css_task_iter_next(). Active iterators are registered on a separate
> > list, sb->s_inodes_iters. Before removing an inode from s_inodes, the
> > removal path advances any iterator whose next position points to that
> > inode. These updates are protected by s_inode_list_lock, so a walker can
> > drop the lock and later resume from its saved position.
> >
> > Existing walkers, such as drop_pagecache_sb() and add_dquot_ref(), already
> > contain their own position-preserving logic: they carry an inode reference
> > across iterations so that they can resume after dropping the list lock.
> > Moving that responsibility into sb_for_each_inodes() simplifies these
> > callers and lets their callbacks focus on the per-inode work.
> >
> > Patch 1 removes trailing whitespace from include/linux/fs.h.
> > Patch 2 introduces sb_for_each_inodes().
> > The remaining patches convert existing walks to the new interface.
> > remove_dquot_ref() and nr_blockdev_pages() are left unchanged: their
> > walks are simple and do not require the inode->i_lock locking imposed
> > by the callback interface. Converting them would add an unnecessary
> > lock/unlock overhead for every inode.
> >
> > [Testing]
> >
> > I tested this series with approximately 20 hours of xfstests case
> > execution, repeatedly running the auto group on ext4 and XFS, no new
> > issues were observed. And with this patch applied, the hung task that
> > previously occurred on every run no longer occurs.
> >
> > [1] https://lore.kernel.org/all/20241118114508.1405494-1-yebin@huaweicloud.com/
> >
> > Julian Sun (7):
> > fs: remove trailing whitespace from include/linux/fs.h
> > fs: introduce sb_for_each_inodes().
> > block: use sb_for_each_inodes() in sync_bdevs()
> > fs: use sb_for_each_inodes() API.
> > gfs2: use sb_for_each_inodes() for cooperative eviction
> > quota: use sb_for_each_inodes() in add_dquot_ref()
> > landlock: use sb_for_each_inodes() when detaching a superblock
> >
> > block/bdev.c | 85 +++++++++----------
> > fs/drop_caches.c | 44 +++++-----
> > fs/gfs2/ops_fstype.c | 38 ++++-----
> > fs/inode.c | 142 +++++++++++++++++++++++--------
> > fs/quota/dquot.c | 72 ++++++----------
> > fs/super.c | 1 +
> > include/linux/fs.h | 29 +++++--
> > include/linux/fs/super_types.h | 3 +-
> > security/landlock/fs.c | 150 +++++++++++++--------------------
> > 9 files changed, 295 insertions(+), 269 deletions(-)
> >
> > --
> > 2.39.5
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
Thanks,
--
Julian Sun <sunjunchao@bytedance.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h
2026-09-09 9:01 ` [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h Julian Sun
@ 2026-09-10 16:52 ` Jan Kara
0 siblings, 0 replies; 14+ messages in thread
From: Jan Kara @ 2026-09-10 16:52 UTC (permalink / raw)
To: Julian Sun
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, jack,
agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
On Wed 09-09-26 17:01:06, Julian Sun wrote:
> Remove trailing whitespace before introducing the inode iterator API.
> This avoids unrelated whitespace changes from automatic editor cleanup
> in the following patches. This also fixes formatting issues in fs.h
> reported by scripts/checkpatch.pl.
>
> Signed-off-by: Julian Sun <sunjunchao@bytedance.com>
We generally try to avoid whitespace only or style only changes as they
hurt backportability mostly unnecessarily. But this patch looks harmless
enough so feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> include/linux/fs.h | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f9d1e05e8ae6..09c4db5e9ae0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -296,7 +296,7 @@ struct iattr {
> */
> #define FILESYSTEM_MAX_STACK_DEPTH 2
>
> -/**
> +/**
> * enum positive_aop_returns - aop return codes with specific semantics
> *
> * @AOP_WRITEPAGE_ACTIVATE: Informs the caller that page writeback has
> @@ -306,7 +306,7 @@ struct iattr {
> * be a candidate for writeback again in the near
> * future. Other callers must be careful to unlock
> * the page if they get this return. Returned by
> - * writepage();
> + * writepage();
> *
> * @AOP_TRUNCATED_PAGE: The AOP method that was handed a locked page has
> * unlocked it and the page might have been truncated.
> @@ -1315,8 +1315,10 @@ struct file *get_file_active(struct file **f);
>
> #define MAX_NON_LFS ((1UL<<31) - 1)
>
> -/* Page cache limit. The filesystems should put that into their s_maxbytes
> - limits, otherwise bad things can happen in VM. */
> +/*
> + * Page cache limit. The filesystems should put that into their s_maxbytes
> + * limits, otherwise bad things can happen in VM.
> + */
> #if BITS_PER_LONG==32
> #define MAX_LFS_FILESIZE ((loff_t)ULONG_MAX << PAGE_SHIFT)
> #elif BITS_PER_LONG==64
> @@ -2285,7 +2287,7 @@ int sync_inode_metadata(struct inode *inode, int wait);
> struct file_system_type {
> const char *name;
> int fs_flags;
> -#define FS_REQUIRES_DEV 1
> +#define FS_REQUIRES_DEV 1
> #define FS_BINARY_MOUNTDATA 2
> #define FS_HAS_SUBTYPE 4
> #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */
> @@ -2907,7 +2909,7 @@ ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos);
> extern ssize_t kernel_write(struct file *, const void *, size_t, loff_t *);
> extern ssize_t __kernel_write(struct file *, const void *, size_t, loff_t *);
> extern struct file * open_exec(const char *);
> -
> +
> /* fs/dcache.c -- generic fs support functions */
> extern bool is_subdir(struct dentry *, struct dentry *);
> extern bool path_is_under(const struct path *, const struct path *);
> --
> 2.39.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/7] fs: introduce sb_for_each_inodes().
2026-09-09 9:01 ` [PATCH 2/7] fs: introduce sb_for_each_inodes() Julian Sun
@ 2026-09-10 17:47 ` Jan Kara
2026-09-11 3:34 ` [External] " Julian Sun
2026-09-11 3:35 ` Julian Sun
0 siblings, 2 replies; 14+ messages in thread
From: Jan Kara @ 2026-09-10 17:47 UTC (permalink / raw)
To: Julian Sun
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, jack,
agruenba, mic, gnoack, paul, jmorris, serge, aleksa, legion,
djwong, ebiggers, sandeen
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@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@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External] Re: [PATCH 2/7] fs: introduce sb_for_each_inodes().
2026-09-10 17:47 ` Jan Kara
@ 2026-09-11 3:34 ` Julian Sun
2026-09-11 3:35 ` Julian Sun
1 sibling, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-11 3:34 UTC (permalink / raw)
To: Jan Kara
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, agruenba,
mic, gnoack, paul, jmorris, serge, aleksa, legion, djwong,
ebiggers, sandeen
On 9/11/26 1:47 AM, Jan Kara wrote:
> 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@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.
Thanks for the feedback. I agree that we should fix the lockup separately,
and I'll focus on that first.
I had seen Dave Chinner's series before. I'll revisit it and the review
comments when working on the inode iteration API.>
> 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
>>
Thanks,
--
Julian Sun <sunjunchao@bytedance.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/7] fs: introduce sb_for_each_inodes().
2026-09-10 17:47 ` Jan Kara
2026-09-11 3:34 ` [External] " Julian Sun
@ 2026-09-11 3:35 ` Julian Sun
1 sibling, 0 replies; 14+ messages in thread
From: Julian Sun @ 2026-09-11 3:35 UTC (permalink / raw)
To: Jan Kara
Cc: linux-block, linux-fsdevel, gfs2, linux-security-module, agruenba,
mic, gnoack, paul, jmorris, serge, aleksa, legion, djwong,
ebiggers, sandeen
On 9/11/26 1:47 AM, Jan Kara wrote:
> 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@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.
Thanks for the feedback. I agree that we should fix the lockup separately,
and I'll focus on that first.
I had seen Dave Chinner's series before. I'll revisit it and the review
comments when working on the inode iteration API.>
> 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
>>
Thanks,
--
Julian Sun <sunjunchao@bytedance.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-09-11 3:35 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 9:01 [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Julian Sun
2026-09-09 9:01 ` [PATCH 1/7] fs: remove trailing whitespace from include/linux/fs.h Julian Sun
2026-09-10 16:52 ` Jan Kara
2026-09-09 9:01 ` [PATCH 2/7] fs: introduce sb_for_each_inodes() Julian Sun
2026-09-10 17:47 ` Jan Kara
2026-09-11 3:34 ` [External] " Julian Sun
2026-09-11 3:35 ` Julian Sun
2026-09-09 9:01 ` [PATCH 3/7] block: use sb_for_each_inodes() in sync_bdevs() Julian Sun
2026-09-09 9:01 ` [PATCH 4/7] fs: use sb_for_each_inodes() API Julian Sun
2026-09-09 9:01 ` [PATCH 5/7] gfs2: use sb_for_each_inodes() for cooperative eviction Julian Sun
2026-09-09 9:01 ` [PATCH 6/7] quota: use sb_for_each_inodes() in add_dquot_ref() Julian Sun
2026-09-09 9:01 ` [PATCH 7/7] landlock: use sb_for_each_inodes() when detaching a superblock Julian Sun
2026-09-09 12:49 ` [PATCH 0/7] fs: preserve superblock inode walk positions across lock drops Jan Kara
2026-09-09 13:08 ` [External] " Julian Sun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox