* [PATCH v10 3/5] ext4: convert all EA inode iput() calls to ext4_put_ea_inode()
From: Yun Zhou @ 2026-06-25 15:29 UTC (permalink / raw)
To: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
yi.zhang, viro, brauner
Cc: linux-ext4, linux-kernel, yun.zhou, linux-fsdevel
In-Reply-To: <20260625152941.24788-1-yun.zhou@windriver.com>
Convert all iput() calls on EA inodes in xattr code paths to use
ext4_put_ea_inode(). This establishes a uniform rule: every EA inode
reference release in ext4 xattr code goes through ext4_put_ea_inode(),
eliminating the need to analyze each call site individually for lock
safety.
Converted sites:
- ext4_xattr_inode_get() read path
- ext4_xattr_inode_inc_ref_all() main loop and cleanup path
- ext4_xattr_inode_dec_ref_all() error paths
- ext4_xattr_inode_create() error path
- ext4_xattr_inode_cache_find() mismatch path
- ext4_xattr_inode_lookup_create() out_err
- ext4_xattr_set_entry() old_ea_inode
- ext4_xattr_block_set() new block path, cleanup, and tmp_inode
- ext4_xattr_ibody_set() error and success paths
- ext4_xattr_delete_inode() quota loop
For most of these, iput_if_not_last() will succeed (the EA inode has
other references) making the overhead a single atomic operation.
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
fs/ext4/xattr.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index ecdad5920b14..90b693b78a45 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -569,7 +569,7 @@ ext4_xattr_inode_get(struct inode *inode, struct ext4_xattr_entry *entry,
ea_inode->i_ino, true /* reusable */);
}
out:
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
return err;
}
@@ -1106,10 +1106,10 @@ static int ext4_xattr_inode_inc_ref_all(handle_t *handle, struct inode *parent,
err = ext4_xattr_inode_inc_ref(handle, ea_inode);
if (err) {
ext4_warning_inode(ea_inode, "inc ref error %d", err);
- iput(ea_inode);
+ ext4_put_ea_inode(parent->i_sb, ea_inode);
goto cleanup;
}
- iput(ea_inode);
+ ext4_put_ea_inode(parent->i_sb, ea_inode);
}
return 0;
@@ -1135,7 +1135,7 @@ static int ext4_xattr_inode_inc_ref_all(handle_t *handle, struct inode *parent,
if (err)
ext4_warning_inode(ea_inode, "cleanup dec ref error %d",
err);
- iput(ea_inode);
+ ext4_put_ea_inode(parent->i_sb, ea_inode);
}
return saved_err;
}
@@ -1203,7 +1203,7 @@ ext4_xattr_inode_dec_ref_all(handle_t *handle, struct inode *parent,
if (err) {
ext4_warning_inode(ea_inode,
"Expand inode array err=%d", err);
- iput(ea_inode);
+ ext4_put_ea_inode(parent->i_sb, ea_inode);
continue;
}
@@ -1507,7 +1507,7 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
if (ext4_xattr_inode_dec_ref(handle, ea_inode))
ext4_warning_inode(ea_inode,
"cleanup dec ref error %d", err);
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
return ERR_PTR(err);
}
@@ -1566,7 +1566,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
kvfree(ea_data);
return ea_inode;
}
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
next_entry:
ce = mb_cache_entry_find_next(ea_inode_cache, ce);
}
@@ -1617,7 +1617,7 @@ static struct inode *ext4_xattr_inode_lookup_create(handle_t *handle,
ea_inode->i_ino, true /* reusable */);
return ea_inode;
out_err:
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
ext4_xattr_inode_free_quota(inode, NULL, value_len);
return ERR_PTR(err);
}
@@ -1850,7 +1850,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
ret = 0;
out:
- iput(old_ea_inode);
+ ext4_put_ea_inode(inode->i_sb, old_ea_inode);
return ret;
}
@@ -2012,7 +2012,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
old_ea_inode_quota = le32_to_cpu(
s->here->e_value_size);
}
- iput(tmp_inode);
+ ext4_put_ea_inode(inode->i_sb, tmp_inode);
s->here->e_value_inum = 0;
s->here->e_value_size = 0;
@@ -2152,7 +2152,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
ext4_warning_inode(ea_inode,
"dec ref error=%d",
error);
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
ea_inode = NULL;
}
@@ -2206,7 +2206,7 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
ext4_xattr_inode_free_quota(inode, ea_inode,
i_size_read(ea_inode));
}
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
}
if (ce)
mb_cache_entry_put(ea_block_cache, ce);
@@ -2288,7 +2288,7 @@ int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
ext4_xattr_inode_free_quota(inode, ea_inode,
i_size_read(ea_inode));
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
}
return error;
}
@@ -2300,7 +2300,7 @@ int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
header->h_magic = cpu_to_le32(0);
ext4_clear_inode_state(inode, EXT4_STATE_XATTR);
}
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
return 0;
}
@@ -2989,7 +2989,7 @@ int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
continue;
ext4_xattr_inode_free_quota(inode, ea_inode,
le32_to_cpu(entry->e_value_size));
- iput(ea_inode);
+ ext4_put_ea_inode(inode->i_sb, ea_inode);
}
}
--
2.43.0
^ permalink raw reply related
* [PATCH v10 0/5] ext4: deferred iput framework for EA inodes
From: Yun Zhou @ 2026-06-25 15:29 UTC (permalink / raw)
To: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
yi.zhang, viro, brauner
Cc: linux-ext4, linux-kernel, yun.zhou, linux-fsdevel
This series introduces a deferred-iput framework for EA inodes to
eliminate a class of lock ordering issues in ext4 xattr code.
The problem: iput() on EA inodes while holding xattr_sem or a jbd2
handle can trigger eviction, which may acquire those same locks or
s_writepages_rwsem, creating circular dependencies. The immediate
deadlock (during mount-time orphan cleanup) is fixed by two separate
patches already reviewed and posted:
ext4: skip extra isize expansion during mount to prevent deadlock
ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode
This series provides the structural fix that makes the code safe
regardless of calling context:
Patch 1 adds a VFS helper iput_if_not_last() which drops an inode
reference only if it is not the last one, using atomic_add_unless().
This provides a proper VFS abstraction for filesystems that need to
conditionally defer final iput.
Patch 2 introduces ext4_put_ea_inode() using iput_if_not_last() as
a fast path (single atomic, zero overhead for the common case). If
this is the last reference, the inode is linked onto a per-sb llist
(via i_ea_iput_node embedded in ext4_inode_info, union with xattr_sem
which is unused for EA inodes) and a delayed worker (1 jiffie) performs
the final iput() in a clean context. No per-iput allocation needed.
Also moves init_rwsem(xattr_sem) from init_once to ext4_alloc_inode
to handle slab reuse after the union field has been overwritten.
Patch 3 converts all EA inode iput() calls in xattr code to use
ext4_put_ea_inode() uniformly -- no exceptions to reason about.
Patch 4 removes the now-redundant ea_inode_array mechanism (parameter
threading, struct, expand/free functions), replaced entirely by direct
ext4_put_ea_inode() calls. This is a net code reduction.
Patch 5 prevents a potential ABBA deadlock on corrupted filesystems
where multiple xattr entries reference the same EA inode. It tracks
processed EA inodes on a per-call llist (reusing i_ea_iput_node) and
skips duplicates before iget, deferring the actual ext4_put_ea_inode()
until after the loop completes. This covers both intra-block and
cross ibody/block duplicates in ext4_xattr_delete_inode().
Link: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5
v10:
- New patch 5: prevent deadlock from duplicate EA inode references
on corrupted filesystems. Track processed EA inodes on a per-call
llist to skip duplicates before iget, and defer ext4_put_ea_inode()
until after the loop to avoid queuing an inode for eviction while
the same loop may still iget it.
- Patch 2: move ext4_init_ea_inode_work() before ext4_multi_mount_protect()
so that failed_mount3a drain does not hit an uninitialized delayed_work
when MMP check fails.
v9:
- Add iput_if_not_last() as proper VFS helper (per reviewer: don't
let filesystems manipulate inode refcount without VFS abstraction).
- Use iput_if_not_last() + llist_node embedded in ext4_inode_info
(union with xattr_sem) to avoid per-iput allocation entirely.
- Convert ALL EA inode iput() calls uniformly -- no exceptions.
- Remove entire ea_inode_array mechanism.
- Add WARN_ON_ONCE in ext4_put_ea_inode() to catch misuse on non-EA
inodes (protects the xattr_sem union safety).
- Fix worker re-arm: ext4_drain_ea_inode_work() loops to handle
nested EA inode evictions re-scheduling work.
- Move INIT_DELAYED_WORK before journal loading (fast commit replay
may trigger evictions).
- Drain before ext4_quotas_off() for correct quota accounting.
- Add flush in failed_mount_wq and failed_mount3a error paths for
journal replay case.
- Move init_rwsem(xattr_sem) from init_once to ext4_alloc_inode to
handle slab object reuse after union overwrite.
- Encapsulate worker init into ext4_init_ea_inode_work(), making
ext4_ea_inode_work() static to xattr.c.
Yun Zhou (5):
fs: add iput_if_not_last() helper
ext4: introduce ext4_put_ea_inode() for safe deferred iput
ext4: convert all EA inode iput() calls to ext4_put_ea_inode()
ext4: remove ea_inode_array mechanism in favor of ext4_put_ea_inode()
ext4: prevent deadlock from duplicate EA inode references on corrupted
fs
fs/ext4/ext4.h | 13 ++-
fs/ext4/inode.c | 6 +-
fs/ext4/super.c | 19 +++-
fs/ext4/xattr.c | 214 +++++++++++++++++++++++++++------------------
fs/ext4/xattr.h | 21 +++--
include/linux/fs.h | 13 +++
6 files changed, 185 insertions(+), 101 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v10 2/5] ext4: introduce ext4_put_ea_inode() for safe deferred iput
From: Yun Zhou @ 2026-06-25 15:29 UTC (permalink / raw)
To: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
yi.zhang, viro, brauner
Cc: linux-ext4, linux-kernel, yun.zhou, linux-fsdevel
In-Reply-To: <20260625152941.24788-1-yun.zhou@windriver.com>
Calling iput() on EA inodes while holding xattr_sem or a jbd2 handle
can trigger write_inode_now() -> ext4_writepages() -> s_writepages_rwsem,
creating a lock ordering issue during mount (!SB_ACTIVE).
Add ext4_put_ea_inode() which uses iput_if_not_last() as a fast path.
If this is not the last reference, it is dropped immediately. If this
is the last reference, the inode is linked onto a per-sb lock-free llist
via i_ea_iput_node (embedded in ext4_inode_info, sharing space with the
unused xattr_sem of EA inodes via a union) and a delayed worker
(1 jiffie) performs the final iput() in a clean context. This avoids
per-iput memory allocation.
Convert the first call site: ext4_xattr_block_set()'s "Drop the
previous xattr block" path, which previously called
ext4_xattr_inode_array_free() under xattr_sem + jbd2 handle.
The worker is drained in ext4_put_super() before quota shutdown using
a loop to handle re-arming (evicting an EA inode may queue further EA
inodes). Initialization is placed before journal loading since fast
commit replay may trigger evictions that call ext4_put_ea_inode().
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
Suggested-by: Jan Kara <jack@suse.cz>
---
fs/ext4/ext4.h | 13 ++++++++-
fs/ext4/super.c | 19 ++++++++++++-
fs/ext4/xattr.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
fs/ext4/xattr.h | 14 ++++++++++
4 files changed, 116 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..b9b0ada7774b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1070,8 +1070,14 @@ struct ext4_inode_info {
* between readers of EAs and writers of regular file data, so
* instead we synchronize on xattr_sem when reading or changing
* EAs.
+ *
+ * EA inodes (EXT4_EA_INODE_FL) do not use xattr_sem; they reuse
+ * the space for deferred iput linkage.
*/
- struct rw_semaphore xattr_sem;
+ union {
+ struct rw_semaphore xattr_sem;
+ struct llist_node i_ea_iput_node;
+ };
/*
* Inodes with EXT4_STATE_ORPHAN_FILE use i_orphan_idx. Otherwise
@@ -1770,6 +1776,11 @@ struct ext4_sb_info {
struct ext4_es_stats s_es_stats;
struct mb_cache *s_ea_block_cache;
struct mb_cache *s_ea_inode_cache;
+
+ /* Deferred iput for EA inodes to avoid lock ordering issues */
+ struct llist_head s_ea_inode_to_free;
+ struct delayed_work s_ea_inode_work;
+
spinlock_t s_es_lock ____cacheline_aligned_in_smp;
/* Journal triggers for checksum computation */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 245f67d10ded..ed1d0cad2bc2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1303,6 +1303,8 @@ static void ext4_put_super(struct super_block *sb)
&sb->s_uuid);
ext4_unregister_li_request(sb);
+ /* Drain deferred EA inode iputs while quota is still active. */
+ ext4_drain_ea_inode_work(sbi);
ext4_quotas_off(sb, EXT4_MAXQUOTAS);
destroy_workqueue(sbi->rsv_conversion_wq);
@@ -1423,6 +1425,13 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
memset(&ei->i_dquot, 0, sizeof(ei->i_dquot));
#endif
ei->jinode = NULL;
+ /*
+ * Reinitialize xattr_sem every allocation because EA inodes
+ * share this space with i_ea_iput_node (via union) which may
+ * have overwritten the semaphore when the slab object was
+ * previously used as an EA inode.
+ */
+ init_rwsem(&ei->xattr_sem);
INIT_LIST_HEAD(&ei->i_rsv_conversion_list);
spin_lock_init(&ei->i_completed_io_lock);
ei->i_sync_tid = 0;
@@ -1488,7 +1497,6 @@ static void init_once(void *foo)
struct ext4_inode_info *ei = foo;
INIT_LIST_HEAD(&ei->i_orphan);
- init_rwsem(&ei->xattr_sem);
init_rwsem(&ei->i_data_sem);
inode_init_once(&ei->vfs_inode);
ext4_fc_init_inode(&ei->vfs_inode);
@@ -5497,6 +5505,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
ext4_has_feature_orphan_present(sb) ||
ext4_has_feature_journal_needs_recovery(sb));
+ ext4_init_ea_inode_work(sbi);
+
if (ext4_has_feature_mmp(sb) && !sb_rdonly(sb)) {
err = ext4_multi_mount_protect(sb, le64_to_cpu(es->s_mmp_block));
if (err)
@@ -5508,6 +5518,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
* The first inode we look at is the journal inode. Don't try
* root first: it may be modified in the journal!
*/
+
if (!test_opt(sb, NOLOAD) && ext4_has_feature_journal(sb)) {
err = ext4_load_and_init_journal(sb, es, ctx);
if (err)
@@ -5747,6 +5758,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
return 0;
failed_mount9:
+ /* Drain deferred EA inode iputs before quota shutdown */
+ ext4_drain_ea_inode_work(sbi);
ext4_quotas_off(sb, EXT4_MAXQUOTAS);
failed_mount8: __maybe_unused
ext4_release_orphan_info(sb);
@@ -5767,6 +5780,8 @@ failed_mount8: __maybe_unused
if (EXT4_SB(sb)->rsv_conversion_wq)
destroy_workqueue(EXT4_SB(sb)->rsv_conversion_wq);
failed_mount_wq:
+ /* Drain deferred EA inode iputs before freeing structures */
+ ext4_drain_ea_inode_work(sbi);
ext4_xattr_destroy_cache(sbi->s_ea_inode_cache);
sbi->s_ea_inode_cache = NULL;
@@ -5777,6 +5792,8 @@ failed_mount8: __maybe_unused
ext4_journal_destroy(sbi, sbi->s_journal);
}
failed_mount3a:
+ /* Drain deferred EA inode iputs from journal replay */
+ ext4_drain_ea_inode_work(sbi);
ext4_es_unregister_shrinker(sbi);
failed_mount3:
/* flush s_sb_upd_work before sbi destroy */
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..ecdad5920b14 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -117,6 +117,8 @@ const struct xattr_handler * const ext4_xattr_handlers[] = {
static int
ext4_expand_inode_array(struct ext4_xattr_inode_array **ea_inode_array,
struct inode *inode);
+static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
+ struct ext4_xattr_inode_array *array);
#ifdef CONFIG_LOCKDEP
void ext4_xattr_inode_set_class(struct inode *ea_inode)
@@ -2187,7 +2189,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
ext4_xattr_release_block(handle, inode, bs->bh,
&ea_inode_array,
0 /* extra_credits */);
- ext4_xattr_inode_array_free(ea_inode_array);
+ ext4_xattr_inode_array_free_deferred(inode->i_sb,
+ ea_inode_array);
}
error = 0;
@@ -3025,6 +3028,74 @@ void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *ea_inode_array)
kfree(ea_inode_array);
}
+static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
+ struct ext4_xattr_inode_array *array)
+{
+ int idx;
+
+ if (array == NULL)
+ return;
+
+ for (idx = 0; idx < array->count; ++idx)
+ ext4_put_ea_inode(sb, array->inodes[idx]);
+ kfree(array);
+}
+
+/*
+ * Worker function for deferred EA inode iput. Processes all inodes queued
+ * on s_ea_inode_to_free in a context free of xattr_sem/jbd2 handle locks.
+ */
+static void ext4_ea_inode_work(struct work_struct *work)
+{
+ struct ext4_sb_info *sbi = container_of(to_delayed_work(work),
+ struct ext4_sb_info,
+ s_ea_inode_work);
+ struct llist_node *node = llist_del_all(&sbi->s_ea_inode_to_free);
+ struct llist_node *next;
+
+ while (node) {
+ struct ext4_inode_info *ei = container_of(node,
+ struct ext4_inode_info, i_ea_iput_node);
+ next = node->next;
+ iput(&ei->vfs_inode);
+ node = next;
+ }
+}
+
+/*
+ * Release a VFS reference on an EA inode. Must be used instead of iput()
+ * in any context where xattr_sem or a jbd2 handle is held.
+ *
+ * If this is not the last reference, drops it immediately via
+ * iput_if_not_last() with no further action needed.
+ *
+ * If this is the last reference, the inode is linked onto a per-sb
+ * llist via i_ea_iput_node (embedded in ext4_inode_info, sharing space
+ * with the unused xattr_sem) and a delayed worker performs the final
+ * iput() in a clean context.
+ */
+void ext4_put_ea_inode(struct super_block *sb, struct inode *inode)
+{
+ if (!inode)
+ return;
+ WARN_ON_ONCE(!(EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL));
+ if (iput_if_not_last(inode))
+ return;
+ llist_add(&EXT4_I(inode)->i_ea_iput_node,
+ &EXT4_SB(sb)->s_ea_inode_to_free);
+ /*
+ * Use a short delay to allow multiple EA inodes to accumulate,
+ * reducing workqueue wakeups when several are released together.
+ */
+ schedule_delayed_work(&EXT4_SB(sb)->s_ea_inode_work, 1);
+}
+
+void ext4_init_ea_inode_work(struct ext4_sb_info *sbi)
+{
+ init_llist_head(&sbi->s_ea_inode_to_free);
+ INIT_DELAYED_WORK(&sbi->s_ea_inode_work, ext4_ea_inode_work);
+}
+
/*
* ext4_xattr_block_cache_insert()
*
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index 1fedf44d4fb6..9883ba5569a1 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -190,6 +190,20 @@ extern int ext4_xattr_delete_inode(handle_t *handle, struct inode *inode,
struct ext4_xattr_inode_array **array,
int extra_credits);
extern void ext4_xattr_inode_array_free(struct ext4_xattr_inode_array *array);
+extern void ext4_init_ea_inode_work(struct ext4_sb_info *sbi);
+extern void ext4_put_ea_inode(struct super_block *sb, struct inode *inode);
+
+/*
+ * Drain all pending deferred EA inode iputs. Must be called before
+ * freeing resources that eviction depends on (quota, block allocator).
+ * Loops because worker iput may trigger eviction that re-queues.
+ */
+static inline void ext4_drain_ea_inode_work(struct ext4_sb_info *sbi)
+{
+ while (flush_delayed_work(&sbi->s_ea_inode_work) ||
+ !llist_empty(&sbi->s_ea_inode_to_free))
+ ;
+}
extern int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
struct ext4_inode *raw_inode, handle_t *handle);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] ext4: clear stale xarray tags on folios skipped during writeback
From: Gerald Yang @ 2026-06-25 13:38 UTC (permalink / raw)
To: Jan Kara; +Cc: tytso, linux-ext4, gerald.yang.tw
In-Reply-To: <asahstosrqlboeyjqyyzvwgne7qucw4y5mvoos346seqgrvwcd@wtuocifmom6c>
Thanks for the review, I will send a v2 based on the suggestion.
On Thu, Jun 25, 2026 at 7:33 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 23-06-26 16:02:38, Gerald Yang wrote:
> > In data=journal mode, the writeback thread can hit the
> > WARN_ON_ONCE(sb_rdonly(sb)) in ext4_journal_check_start() while the
> > superblock is being remounted read-only during reboot:
> >
> > Workqueue: writeback wb_workfn (flush-253:0)
> > RIP: 0010:ext4_journal_check_start+0x8b/0xd0
> > Call Trace:
> > __ext4_journal_start_sb+0x3c/0x1e0
> > mpage_prepare_extent_to_map+0x4af/0x580
> > ext4_do_writepages+0x3c0/0x1080
> > ext4_writepages+0xc8/0x1a0
> > do_writepages+0xc4/0x180
> > __writeback_single_inode+0x45/0x2f0
> > writeback_sb_inodes+0x26b/0x5d0
> > __writeback_inodes_wb+0x54/0x100
> > wb_writeback+0x1ac/0x320
> > wb_workfn+0x394/0x470
> >
> > And followed by the warning:
> > EXT4-fs warning (device vda1): ext4_evict_inode:195: inode #6263:
> > comm (sd-umount): data will be lost
> >
> > This issue is not reproduced every time, but frequently.
> > The reproduction step is to create a VM with 8 CPUs, 16G memory and
> > setup data=journal:
> > sudo tune2fs -o journal_data /dev/vda1
> > Run fio:
> > rm -f fiotest
> > fio --name=fiotest --rw=randwrite --bs=4k --runtime=6 --ioengine=libaio
> > --iodepth=256 --numjobs=8 --filename=fiotest --filesize=30G
> > --group_reporting
> > Reboot the VM, and check the console output from:
> > virsh console testvm
> >
> > But there is no dirty inode, folio_clear_dirty_for_io clears PG_dirty
> > but leaves tags PAGECACHE_TAG_DIRTY and PAGECACHE_TAG_TOWRITE set which
> > are only cleared by __folio_start_writeback.
> > In data=journal mode, jbd2 checkpoints the journalled data to its final
> > location and clears its own dirty flag without touching folio PG_dirty
> > or xarray dirty flags.
> > The commit f4a2b42e7891 ("ext4: fix stale xarray tags after writeback")
> > fixes when PG_dirty is still set but there is no dirty page.
> > Another case is PG_dirty is cleared, but PAGECACHE_TAG_DIRTY and
> > PAGECACHE_TAG_TOWRITE is still set. In this case, writeback thread
> > checks clean folio and skips it in mpage_prepare_extent_to_map:
> > if (!folio_test_dirty(folio) ||
> > ...
> > folio_unlcok(folio);
> > continue
> >
> > And never reaches ext4_bio_write_folio where the commit f4a2b42e7891
> > clears the stale xarray tags. Print debug logs after the filesystem
> > is remounted read-only:
> > writepages RDONLY nrpages=2048 dirtytag=1 wbtag=0 towrite=1 sync=0
> > And all folios are actually clean:
> > folio idx=3 dirty=0 wb=0 checked=0 dirtybuf=0 jbddirty=0 mapped=1
> > ...
> >
> > We need to clear the xarray stale tags for such clean folios by
> > cycling them through writeback in the skip path, the same way
> > f4a2b42e7891 does in ext4_bio_write_folio.
> >
> > Fixes: dff4ac75eeee ("ext4: move keep_towrite handling to ext4_bio_write_page()")
> > Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
>
> Good spotting. One comment to the patch:
>
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index ce99807c5f5b..40da611b0831 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -2698,6 +2698,28 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
> > (folio_test_writeback(folio) &&
> > (mpd->wbc->sync_mode == WB_SYNC_NONE)) ||
> > unlikely(folio->mapping != mapping)) {
> > + /*
> > + * A clean folio (PG_dirty = 0) can still carry
> > + * stale xarray tags: PAGECACHE_TAG_DIRTY /
> > + * PAGECACHE_TAG_TOWRITE.
> > + * In data=journal mode, the data may have been
> > + * checkpointed to its final location by jbd2
> > + * but these tags are only cleared by
> > + * __folio_start_writeback() later.
> > + * These tags keep the inode on the writeback
> > + * list and make the writeback thread calls
> > + * ext4_journal_start on a read-only sb during
> > + * remounting fs to read-only, and return
> > + * -EROFS from ext4_journal_check_start.
> > + * Cycle the clean folio through writeback to
> > + * drop the stale xarray tags.
> > + */
> > + if (folio->mapping == mapping &&
> > + !folio_test_dirty(folio) &&
> > + !folio_test_writeback(folio)) {
> > + __folio_start_writeback(folio, false);
> > + folio_end_writeback(folio);
> > + }
>
> Instead of this check, just split the top level condition to make things
> more obvious:
> if ((folio_test_writeback(folio) &&
> mpd->wbc->sync_mode == WB_SYNC_NONE) ||
> unlikely(folio->mapping != mapping)) {
> folio_unlock(folio);
> continue;
> }
> /*
> * If the folio is clean, skip writing it back.
> * Cycle the folio through the writeback state
> * though, to clear stale xarray tags.
> */
> if (!folio_test_dirty(folio)) {
> if (!folio_test_writeback(folio)) {
> __folio_start_writeback(folio,
> false);
> folio_end_writeback(folio);
> }
> folio_unlock(folio);
> continue;
> }
>
> Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 09/23] ext4: implement writeback path using iomap
From: Jan Kara @ 2026-06-25 12:58 UTC (permalink / raw)
To: Zhang Yi
Cc: Jan Kara, Zhang Yi, linux-ext4, linux-fsdevel, linux-kernel,
tytso, adilger.kernel, libaokun, ojaswin, ritesh.list, djwong,
hch, yi.zhang, yangerkun, yukuai
In-Reply-To: <881a714b-12ad-4678-81d1-f92dcc126567@huaweicloud.com>
On Thu 25-06-26 11:33:36, Zhang Yi wrote:
> On 6/25/2026 1:16 AM, Jan Kara wrote:
> > On Mon 22-06-26 20:36:02, Zhang Yi wrote:
> >>> then I'd
> >>> probably prefer coming up with an ext4_get_blocks flag which tells it to
> >>> start a transaction on its own if we need to allocate blocks... That would
> >>> be much simpler than opencoding all this.
> >>
> >> Additionally, there is a key point here. The reason I open-coded
> >> ext4_iomap_map_writeback_range() is that we must ensure extent query
> >> and allocation are performed atomically under i_data_sem. Otherwise,
> >> concurrent truncate could lead to quota leaks.
> >>
> >> Specifically, consider the following scenario: we call
> >> ext4_map_blocks() to allocate blocks. Suppose there is a delalloc
> >> extent covering blocks [0,3). While writeback is submitting block 0, a
> >> concurrent truncate(block 1) occurs:
> >>
> >> wb truncate
> >> ext4_es_lookup_extent() ext4_truncate_down()
> >> //get [0,3)
> >> truncate_inode_pages_range()
> >> //clear page 1&2
> >> ext4_truncate()
> >> down_write(i_data_sem)
> >> ext4_es_remove_extent()
> >> //drop extent [1,3)
> >> //i_reserved_data_blocks: 3->1
> >> up_write(i_data_sem)
> >> down_write(i_data_sem)
> >> ext4_map_create_blocks()
> >> //alloc 3 blocks
> >> ext4_es_insert_extent()
> >> //only reclaim 1 block，stale 2 blocks
> >> up_write(i_data_sem)
> >>
> >> Therefore, If we don't open-coding this part, we would need to
> >> significantly rework ext4_map_blocks(), which might have a larger
> >> impact at this point. What do you think?
> >
> > Hum, is something like this really possible? I mean iomap_writepages() will
> > lookup and lock folio. Only then it calls ->iomap_begin to map it to
> > underlying blocks. And folio lock synchronizes against
> > truncate_inode_pages_range() so how would writeback end up trying to
> > allocate something underlying pages 1 or 2?
>
> I believe this scenario can indeed occur, and folio lock alone is
> insufficient to protect against this concurrency issue. The main reason
> is that the iomap writeback framework processes folios one by one. For
> each folio, it follows the "lock -> map -> unlock" sequence. If each
> iteration only mapped blocks covering no more than one folio in length,
> performance would be severely degraded. Therefore, both XFS and the
> current ext4 iomap implementation choose to map up to the minimum of the
> writeback length and the delalloc extent length. This means that when
> processing folio 0, if an extent of length 3 is found, the ranges
> corresponding to the subsequent folios are also mapped and cached. As a
> result, holding only the folio lock of folio 0 is insufficient to
> protect against truncation concurrency with the latter two folios.
Yes, I know about the behavior that iomap_writepages() can cache extent
longer than what folio_lock covers. But after truncate_inode_pages_range()
completes in the truncate path, writeback_iter will never attempt to map
anything for folios beyond i_size. If we had anything cached from before
truncate, iomap_valid() check takes care it isn't used.
But I think you might be concerned about the following race:
In the writeback path iomap_writeback_folio() is called for folio 0.
iomap_writeback_handle_eof() still sees old i_size so end_pos is kept
large, we go down to ext4_map_blocks() which runs ext4_map_query_blocks()
and still sees larger delalloc extent. Then truncate to folio 1 comes. It
can fully run and complete because folio 0 is not affected by it. Then
ext4_map_blocks() resumes and calls ext4_map_create_blocks() with the (now
stale) long delalloc extent and allocates blocks under already truncated
area.
Frankly what I think needs to happen is to fix ext4_map_blocks() so that
after reacquiring i_data_sem, we recheck m_seq still matches i_seq and if
not, redo the extent status tree lookup which will as a side effect also
trim the length to map to appropriate size. What do you think?
> >>> For now, I'd prefer to do what XFS does and offload everything. Then you
> >>> don't have to export iomap_finish_ioend() (which would need to be in a
> >>> separate patch and acked by iomap maintainers) and the code is more
> >>> standard. There's a patchset in the works which adds general ioend offloading
> >>> infrastructure into iomap [1] and when that lands we should get all these
> >> ^^^^^ block layer?
> >>
> >>> bells and whistles (even better ones with percpu work queues, batching,
> >>> etc.) for free.
> >>>
> >>> [1] https://lore.kernel.org/all/20260514-blk-dontcache-v6-0-782e2fa7477b@columbia.edu/
> >>>
> >>> Honza
> >>
> >> Ha, I've noticed this patchset, so I haven't implemented
> >> uncached I/O handling for now. As a side note, I have a question:
> >> if we convert all endio processing to worker threads, IIRC, my
> >> recollection from previous performance tests is that pure overwrite
> >> scenarios would see at least a 20% degradation. Is that acceptable?
> >
> > No, but the latest version of the patches exactly does IO completion in the
> > interrupt unless the bio is flagged as needing IO completion from process
> > context or unless end_io handler returns a particular error - which means
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Please let me confirm whether my understanding is correct. The latest(v6)
> modification to bio_endio() is as follows:
>
> @@ -1791,7 +1865,9 @@ void bio_endio(struct bio *bio)
> }
> #endif
>
> - if (bio->bi_end_io)
> + if (bio_flagged(bio, BIO_COMPLETE_IN_TASK) && bio_in_atomic())
> + __bio_complete_in_task(bio);
> + else if (bio->bi_end_io)
> bio->bi_end_io(bio);
> }
>
> Currently, __bio_complete_in_task() is only called in bio_endio() when
> BIO_COMPLETE_IN_TASK is explicitly set on the bio. It is not called
> again based on a specific error return from bio->bi_end_io(). So I
> believe what you meant is that each filesystem's own ->bi_end_io()
> should call it to convert the endio processing to process context.
> Is my understanding correct?
Right yes. I remembered wrong what we ended up with. But I was certain
the usecase of offloading from ->bi_end_io is handled. :)
> >> The reason I kept ext4_iomap_end_bio() handling I/O completion in
> >> interrupt context is for overwrite performance. XFS also handles
> >> overwrites in interrupt context (via ioend_writeback_end_bio()).
> >> However, ext4 has the data_error=abort mount option — when this mode
> >> is set and an I/O error occurs, we must abort the journal in a
> >> worker. Since we cannot predict I/O errors at submission time, we
> >> can't directly use ioend_writeback_end_bio() and must instead bind
> >> our own ext4_iomap_end_bio(). At the same time, I want to avoid
> >> spawning a worker for pure overwrites when no I/O error occurs, so I
> >> exported iomap_finish_ioend(). What do you think?
> >
> > So data_error=abort handling can exactly use the new generic framework - if
> > we detect during processing IO completion we cannot actually do it in the
> > interrupt (like in case of error), we just return appropriately from the
> > handler and the generic code will handle offloading and call the ->end_io
> > callback again.
>
> If my understanding above is correct, what you are suggesting here is
> that in ext4_iomap_end_bio(), we should call __bio_complete_in_task()
> to switch to process context when data_error=abort is set and the I/O
> returns an error. This way, we could drop ext4's own
> i_iomap_ioend_work / i_rsv_conversion_work handling. Is that right?
Exactly.
> But even with that, it seems we would still need to export
> iomap_finish_ioend(). Because if the I/O does not fail,
> ext4_iomap_end_bio() would need to complete the IO processing in
> interrupt context for pure overwrite and would not switch to tasks
> context. Because only iomap_finish_ioend() is safe to call in interrupt
> context. Am I misunderstanding something?
Hmm, yes, something like that. Basically iomap ioend processing will need
to be updated to play well with the generic IO completion offloading
infrastructure. But that's a separate topic. Just keep things simple
(possibly not perfect in terms of performance) since we should transition
to the generic infrastructure sooner rather than later.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] ext4: clear stale xarray tags on folios skipped during writeback
From: Jan Kara @ 2026-06-25 11:33 UTC (permalink / raw)
To: Gerald Yang; +Cc: tytso, jack, linux-ext4, gerald.yang.tw
In-Reply-To: <20260623080312.2713478-1-gerald.yang@canonical.com>
On Tue 23-06-26 16:02:38, Gerald Yang wrote:
> In data=journal mode, the writeback thread can hit the
> WARN_ON_ONCE(sb_rdonly(sb)) in ext4_journal_check_start() while the
> superblock is being remounted read-only during reboot:
>
> Workqueue: writeback wb_workfn (flush-253:0)
> RIP: 0010:ext4_journal_check_start+0x8b/0xd0
> Call Trace:
> __ext4_journal_start_sb+0x3c/0x1e0
> mpage_prepare_extent_to_map+0x4af/0x580
> ext4_do_writepages+0x3c0/0x1080
> ext4_writepages+0xc8/0x1a0
> do_writepages+0xc4/0x180
> __writeback_single_inode+0x45/0x2f0
> writeback_sb_inodes+0x26b/0x5d0
> __writeback_inodes_wb+0x54/0x100
> wb_writeback+0x1ac/0x320
> wb_workfn+0x394/0x470
>
> And followed by the warning:
> EXT4-fs warning (device vda1): ext4_evict_inode:195: inode #6263:
> comm (sd-umount): data will be lost
>
> This issue is not reproduced every time, but frequently.
> The reproduction step is to create a VM with 8 CPUs, 16G memory and
> setup data=journal:
> sudo tune2fs -o journal_data /dev/vda1
> Run fio:
> rm -f fiotest
> fio --name=fiotest --rw=randwrite --bs=4k --runtime=6 --ioengine=libaio
> --iodepth=256 --numjobs=8 --filename=fiotest --filesize=30G
> --group_reporting
> Reboot the VM, and check the console output from:
> virsh console testvm
>
> But there is no dirty inode, folio_clear_dirty_for_io clears PG_dirty
> but leaves tags PAGECACHE_TAG_DIRTY and PAGECACHE_TAG_TOWRITE set which
> are only cleared by __folio_start_writeback.
> In data=journal mode, jbd2 checkpoints the journalled data to its final
> location and clears its own dirty flag without touching folio PG_dirty
> or xarray dirty flags.
> The commit f4a2b42e7891 ("ext4: fix stale xarray tags after writeback")
> fixes when PG_dirty is still set but there is no dirty page.
> Another case is PG_dirty is cleared, but PAGECACHE_TAG_DIRTY and
> PAGECACHE_TAG_TOWRITE is still set. In this case, writeback thread
> checks clean folio and skips it in mpage_prepare_extent_to_map:
> if (!folio_test_dirty(folio) ||
> ...
> folio_unlcok(folio);
> continue
>
> And never reaches ext4_bio_write_folio where the commit f4a2b42e7891
> clears the stale xarray tags. Print debug logs after the filesystem
> is remounted read-only:
> writepages RDONLY nrpages=2048 dirtytag=1 wbtag=0 towrite=1 sync=0
> And all folios are actually clean:
> folio idx=3 dirty=0 wb=0 checked=0 dirtybuf=0 jbddirty=0 mapped=1
> ...
>
> We need to clear the xarray stale tags for such clean folios by
> cycling them through writeback in the skip path, the same way
> f4a2b42e7891 does in ext4_bio_write_folio.
>
> Fixes: dff4ac75eeee ("ext4: move keep_towrite handling to ext4_bio_write_page()")
> Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
Good spotting. One comment to the patch:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ce99807c5f5b..40da611b0831 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2698,6 +2698,28 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
> (folio_test_writeback(folio) &&
> (mpd->wbc->sync_mode == WB_SYNC_NONE)) ||
> unlikely(folio->mapping != mapping)) {
> + /*
> + * A clean folio (PG_dirty = 0) can still carry
> + * stale xarray tags: PAGECACHE_TAG_DIRTY /
> + * PAGECACHE_TAG_TOWRITE.
> + * In data=journal mode, the data may have been
> + * checkpointed to its final location by jbd2
> + * but these tags are only cleared by
> + * __folio_start_writeback() later.
> + * These tags keep the inode on the writeback
> + * list and make the writeback thread calls
> + * ext4_journal_start on a read-only sb during
> + * remounting fs to read-only, and return
> + * -EROFS from ext4_journal_check_start.
> + * Cycle the clean folio through writeback to
> + * drop the stale xarray tags.
> + */
> + if (folio->mapping == mapping &&
> + !folio_test_dirty(folio) &&
> + !folio_test_writeback(folio)) {
> + __folio_start_writeback(folio, false);
> + folio_end_writeback(folio);
> + }
Instead of this check, just split the top level condition to make things
more obvious:
if ((folio_test_writeback(folio) &&
mpd->wbc->sync_mode == WB_SYNC_NONE) ||
unlikely(folio->mapping != mapping)) {
folio_unlock(folio);
continue;
}
/*
* If the folio is clean, skip writing it back.
* Cycle the folio through the writeback state
* though, to clear stale xarray tags.
*/
if (!folio_test_dirty(folio)) {
if (!folio_test_writeback(folio)) {
__folio_start_writeback(folio,
false);
folio_end_writeback(folio);
}
folio_unlock(folio);
continue;
}
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH] ext4: cancel dirty accounting for folios without buffers
From: Jan Kara @ 2026-06-25 11:18 UTC (permalink / raw)
To: Zhang Yi
Cc: Jan Kara, Zhu Jia, Zhang Yi, tytso, adilger.kernel, libaokun,
ojaswin, ritesh.list, linux-ext4, linux-kernel, stable
In-Reply-To: <7471b9cb-158a-4ed4-a1ce-95270ef38974@gmail.com>
On Wed 24-06-26 21:29:58, Zhang Yi wrote:
> On 6/24/2026 8:32 PM, Jan Kara wrote:
> > On Wed 24-06-26 17:52:06, Zhu Jia wrote:
> > > Hi Yi,
> > >
> > > Thanks for taking a look.
> > >
> > > Yes, clearing PAGECACHE_TAG_DIRTY/TOWRITE would make the page-cache state
> > > cleaner. I had a version that did this by adding a helper around
> > > folio_cancel_dirty() and clearing the xarray tags after confirming the
> > > folio was still the same clean page-cache entry.
> > >
> > > It looked like this:
> > >
> > > static void ext4_cancel_dirty_folio(struct address_space *mapping,
> > > struct folio *folio)
> > > {
> > > XA_STATE(xas, &mapping->i_pages, folio->index);
> > > unsigned long flags;
> > >
> > > folio_cancel_dirty(folio);
> > >
> > > xas_lock_irqsave(&xas, flags);
> > > if (xas_load(&xas) == folio && !folio_test_dirty(folio)) {
> > > xas_clear_mark(&xas, PAGECACHE_TAG_DIRTY);
> > > xas_clear_mark(&xas, PAGECACHE_TAG_TOWRITE);
> > > }
> > > xas_unlock_irqrestore(&xas, flags);
> > > }
> > >
> > > The reason I left the tags unchanged in this version is that I was not sure
> > > whether it is appropriate for ext4 to open-code xarray tag cleanup directly.
> > >
> > > If you think this is the right direction, I can add the helper back and
> > > send a v2.
> >
> > That was a good judgement! Playing with xarray tags like this in filesystem
> > code is certainly not a good thing. For now, I'd leave the xarray tags
> > dangling - they will be eventually synced with reality on next writeback
> > attempt. If this inconsistency of tags needs to be fixed, the fix belongs
> > to the generic code (so that it can be used in other places as well).
> >
> > Honza
>
> Yes, I agree. Directly clearing the tag via open code is not a good
> approach. However, I took a look at the !nr_to_submit branch in
> ext4_bio_write_folio(), and it seems to have a similar simple handling
> pattern—it directly calls __folio_start_writeback() and
> folio_end_writeback(), which appears to be an elegant way to clear them.
> Could we also call these two helpers just after folio_cancel_dirty()
> here?
Right, that would be actually doable there and would keep things more
consistent so I think that's a good idea! Thanks!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v5] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Aditya Prakash Srivastava @ 2026-06-25 11:09 UTC (permalink / raw)
To: Jan Kara
Cc: tytso, adilger.kernel, libaokun, ritesh.list, yi.zhang,
linux-ext4, linux-kernel, Colin Ian King
In-Reply-To: <emhyf7w5rac5e2bgaxnbphgo5tlbpud24utygg22dobqnhuacx@2t76kf3yooca>
Hi Jan,
My sincere apologies for the rapid succession of patches and the noise
on the list. You are completely right; I got over-excited trying to
quickly address bugs flagged by the Sashiko-AI bot in previous
iterations, and I should have thought them through more carefully. I
will be more careful going forward.
Regarding your feedback:
You are entirely correct that 'is_freeing' is redundant. Since
find_inode_nowait() already returns NULL if the match callback returns
-1, we can simply drop that tracking variable and check !inode.
I also agree that returning -ENOENT on cache misses and unhashed checks
is much more semantically accurate than -ESTALE.
Thank you again for your patience, invaluable reviews, and guidance.
Best regards,
Aditya
On Thu, Jun 25, 2026 at 3:53 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 25-06-26 06:50:09, Aditya Srivastava wrote:
> > From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
> >
> > Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
> > concurrent xattr workloads (using the ea_inode mount/format option).
> >
> > The deadlock occurs between the running transaction and the eviction
> > thread:
> > - Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
> > and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
> > the corresponding EA inode. Since the EA inode is currently being
> > evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
> > eviction to complete.
> > - Task 2 (eviction thread): Currently evicting the same EA inode in
> > ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
> > blocks waiting for Task 1 to release the reference to the mbcache_entry.
> >
> > To break this deadlock, implement a new ext4_iget() configuration flag
> > named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
> > inode via VFS's find_inode_nowait() API.
> >
> > If the inode is currently being evicted (marked with I_FREEING or
> > I_WILL_FREE) or created (I_CREATING), or if it is not present in the VFS
> > inode cache (cache miss), simply skip it (returning -ESTALE) rather than
> > waiting for eviction/creation to complete, breaking the ABBA cycle.
> >
> > Since we return -ESTALE immediately on a cache miss, we never attempt to
> > allocate a new inode or call iget_locked(), completely eliminating any
> > TOCTOU race window.
> >
> > If the returned inode is I_NEW, wait for its initialization to clear via
> > wait_on_new_inode(). If initialization fails and the inode is unhashed
> > during wait_on_new_inode() waking up (e.g., due to an I/O read error in
> > another thread), safely drop the reference and return -ESTALE. This
> > unhashed check is executed unconditionally on all cache-hit pathways to
> > properly handle concurrent initialization failures.
> >
> > Finally, standard validation checks (including is_bad_inode,
> > EXT4_EA_INODE_FL, file_acl, and xattr flags) are executed as normal inside
> > check_igot_inode() to fully guarantee VFS-layer safety.
> >
> > In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
> > EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
> >
> > Suggested-by: Jan Kara <jack@suse.cz>
> > Reported-by: Colin Ian King <colin.i.king@gmail.com>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
> > Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
> > Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
>
> Three patch submissions in a day? I think you are overdoing it. Please test
> more, think more about what you do (i.e., really understand what the code
> does, not just whatever some AI agent suggested) and submit less.
>
> > +static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
> > +{
> > + bool *is_freeing = data;
> > +
> > + if (inode->i_ino != ino)
> > + return 0;
> > + spin_lock(&inode->i_lock);
> > + if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
> > + if (is_freeing)
> > + *is_freeing = true;
> > + spin_unlock(&inode->i_lock);
> > + return -1;
> > + }
> > + __iget(inode);
> > + spin_unlock(&inode->i_lock);
> > + return 1;
> > +}
> > +
> > struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> > ext4_iget_flags flags, const char *function,
> > unsigned int line)
> > @@ -5298,9 +5316,26 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> > return ERR_PTR(-EFSCORRUPTED);
> > }
> >
> > - inode = iget_locked(sb, ino);
> > - if (!inode)
> > - return ERR_PTR(-ENOMEM);
> > + if (flags & EXT4_IGET_NOWAIT) {
> > + bool is_freeing = false;
> > +
> > + inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
> > + if (is_freeing || !inode)
>
> is_freeing is pointless. Just drop it and use !inode.
>
> > + return ERR_PTR(-ESTALE);
>
> I'd prefer -ENOENT here instead of -ESTALE. It's kind of more specific.
>
> > + if (inode_state_read_once(inode) & I_NEW)
> > + wait_on_new_inode(inode);
> > +
> > + if (unlikely(inode_unhashed(inode))) {
> > + iput(inode);
> > + return ERR_PTR(-ESTALE);
>
> Here -ENOENT as well please.
>
> Otherwise the patch looks good to me.
>
> Honza
>
> > + }
> > + } else {
> > + inode = iget_locked(sb, ino);
> > + if (!inode)
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > if (!(inode_state_read_once(inode) & I_NEW)) {
> > ret = check_igot_inode(inode, flags, function, line);
> > if (ret) {
> > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> > index 982a1f831e22..21b5670d8503 100644
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
> >
> > while (ce) {
> > ea_inode = ext4_iget(inode->i_sb, ce->e_value,
> > - EXT4_IGET_EA_INODE);
> > + EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
> > if (IS_ERR(ea_inode))
> > goto next_entry;
> > ext4_xattr_inode_set_class(ea_inode);
> > --
> > 2.47.3
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 0/2] tracing: Move non-trace_printk prototypes into trace_controls.h
From: Jani Nikula @ 2026-06-25 11:05 UTC (permalink / raw)
To: Steven Rostedt, linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Linus Torvalds, Sebastian Andrzej Siewior, John Ogness,
Thomas Gleixner, Peter Zijlstra, Julia Lawall, Yury Norov,
linux-doc, linux-kbuild, linuxppc-dev, dri-devel, linux-stm32,
linux-arm-kernel, linux-rdma, linux-usb, linux-ext4, linux-nfs,
kvm, intel-gfx
In-Reply-To: <20260625104007.041432666@kernel.org>
On Thu, 25 Jun 2026, Steven Rostedt <rostedt@kernel.org> wrote:
> Remove trace_printk.h by creating a trace_controls.h for those places that
> need access to tracing prototypes like tracing_off() and for the places that
> need trace_printk() directly, to have it included directly.
>
> Changse since v3: https://lore.kernel.org/all/20260624081806.120105649@kernel.org/
>
> - Always include trace_controls.h in rcu.h (kernel test robot)
>
> There are other configs that may include tracing_off() in rcu.h besides
> the one that had the include of trace_controls.h. Just always include
> it in that header to be safe.
>
> Steven Rostedt (2):
> tracing: Move non-trace_printk prototypes into trace_controls.h
> tracing: Remove trace_printk.h from kernel.h
>
> ----
> arch/powerpc/kvm/book3s_xics.c | 1 +
> arch/powerpc/xmon/xmon.c | 1 +
> arch/s390/kernel/ipl.c | 1 +
> arch/s390/kernel/machine_kexec.c | 1 +
> drivers/gpu/drm/i915/gt/intel_gtt.h | 1 +
> drivers/gpu/drm/i915/i915_gem.h | 2 ++
For the i915 parts,
Acked-by: Jani Nikula <jani.nikula@intel.com>
for merging via whichever tree.
> drivers/hwtracing/stm/dummy_stm.c | 1 +
> drivers/infiniband/hw/hfi1/trace_dbg.h | 1 +
> drivers/tty/sysrq.c | 1 +
> drivers/usb/early/xhci-dbc.c | 1 +
> fs/ext4/inline.c | 1 +
> include/linux/ftrace.h | 2 ++
> include/linux/kernel.h | 1 -
> include/linux/sunrpc/debug.h | 1 +
> include/linux/trace_controls.h | 54 ++++++++++++++++++++++++++++++++
> include/linux/trace_printk.h | 56 ++--------------------------------
> kernel/debug/debug_core.c | 1 +
> kernel/panic.c | 1 +
> kernel/rcu/rcu.h | 1 +
> kernel/rcu/rcutorture.c | 1 +
> kernel/trace/ring_buffer_benchmark.c | 1 +
> kernel/trace/trace.h | 1 +
> kernel/trace/trace_benchmark.c | 1 +
> lib/sys_info.c | 1 +
> samples/fprobe/fprobe_example.c | 1 +
> samples/ftrace/ftrace-direct-too.c | 1 -
> samples/trace_printk/trace-printk.c | 1 +
> 27 files changed, 82 insertions(+), 55 deletions(-)
> create mode 100644 include/linux/trace_controls.h
--
Jani Nikula, Intel
^ permalink raw reply
* [PATCH v4 1/2] tracing: Move non-trace_printk prototypes into trace_controls.h
From: Steven Rostedt @ 2026-06-25 10:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Linus Torvalds, Sebastian Andrzej Siewior, John Ogness,
Thomas Gleixner, Peter Zijlstra, Julia Lawall, Yury Norov,
linux-doc, linux-kbuild, linuxppc-dev, dri-devel, linux-stm32,
linux-arm-kernel, linux-rdma, linux-usb, linux-ext4, linux-nfs,
kvm, intel-gfx
In-Reply-To: <20260625104007.041432666@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Remove the prototypes of the code that is not associated with
trace_printk() from trace_printk.h.
These control functions as well as ftrace_dump() and trace_dump_stack()
are used in cases where things go wrong. The main use case is to do a
trace_dump_stack(); tracing_off(); ftrace_dump(); in a place that detected
that something went wrong, whereas, trace_printk() is added to normal code
during debugging and removed before committing upstream. The dump code is
fine to keep in production.
Suggested-by: Yury Norov <yury.norov@gmail.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
Changes since v3: https://patch.msgid.link/20260624081948.147764194@kernel.org
- Move include out of #if statement in rcu.h
kernel test robot found other configs that could require the
control functions in rcu.h. Just always include it in that file.
arch/powerpc/xmon/xmon.c | 1 +
arch/s390/kernel/ipl.c | 1 +
arch/s390/kernel/machine_kexec.c | 1 +
drivers/gpu/drm/i915/i915_gem.h | 1 +
drivers/tty/sysrq.c | 1 +
include/linux/trace_controls.h | 54 ++++++++++++++++++++++++++++++++
include/linux/trace_printk.h | 51 ------------------------------
kernel/debug/debug_core.c | 1 +
kernel/panic.c | 1 +
kernel/rcu/rcu.h | 1 +
kernel/rcu/rcutorture.c | 1 +
kernel/trace/trace.h | 1 +
kernel/trace/trace_benchmark.c | 1 +
lib/sys_info.c | 1 +
14 files changed, 66 insertions(+), 51 deletions(-)
create mode 100644 include/linux/trace_controls.h
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index cb3a3244ae6f..2135f319e0dd 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -27,6 +27,7 @@
#include <linux/highmem.h>
#include <linux/security.h>
#include <linux/debugfs.h>
+#include <linux/trace_controls.h>
#include <asm/ptrace.h>
#include <asm/smp.h>
diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index 3c346b02ceb9..baac66cc4de4 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -22,6 +22,7 @@
#include <linux/debug_locks.h>
#include <linux/vmalloc.h>
#include <linux/secure_boot.h>
+#include <linux/trace_controls.h>
#include <asm/asm-extable.h>
#include <asm/machine.h>
#include <asm/diag.h>
diff --git a/arch/s390/kernel/machine_kexec.c b/arch/s390/kernel/machine_kexec.c
index baeb3dcfc1c8..33f9a89eb3ad 100644
--- a/arch/s390/kernel/machine_kexec.c
+++ b/arch/s390/kernel/machine_kexec.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/reboot.h>
#include <linux/ftrace.h>
+#include <linux/trace_controls.h>
#include <linux/debug_locks.h>
#include <linux/cpufeature.h>
#include <asm/guarded_storage.h>
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 20b3cb29cfff..1da8fb61c09e 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -116,6 +116,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
#endif
#if IS_ENABLED(CONFIG_DRM_I915_TRACE_GEM)
+#include <linux/trace_controls.h>
#define GEM_TRACE(...) trace_printk(__VA_ARGS__)
#define GEM_TRACE_ERR(...) do { \
pr_err(__VA_ARGS__); \
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c2e4b31b699a..d3f72dc430b8 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -324,6 +324,7 @@ static const struct sysrq_key_op sysrq_showstate_blocked_op = {
};
#ifdef CONFIG_TRACING
+#include <linux/trace_controls.h>
#include <linux/ftrace.h>
static void sysrq_ftrace_dump(u8 key)
diff --git a/include/linux/trace_controls.h b/include/linux/trace_controls.h
new file mode 100644
index 000000000000..995b97e963b4
--- /dev/null
+++ b/include/linux/trace_controls.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_TRACE_CONTROLS_H
+#define _LINUX_TRACE_CONTROLS_H
+
+
+/*
+ * General tracing related utility functions - trace_printk(),
+ * tracing_on/tracing_off and tracing_start()/tracing_stop
+ *
+ * Use tracing_on/tracing_off when you want to quickly turn on or off
+ * tracing. It simply enables or disables the recording of the trace events.
+ * This also corresponds to the user space /sys/kernel/tracing/tracing_on
+ * file, which gives a means for the kernel and userspace to interact.
+ * Place a tracing_off() in the kernel where you want tracing to end.
+ * From user space, examine the trace, and then echo 1 > tracing_on
+ * to continue tracing.
+ *
+ * tracing_stop/tracing_start has slightly more overhead. It is used
+ * by things like suspend to ram where disabling the recording of the
+ * trace is not enough, but tracing must actually stop because things
+ * like calling smp_processor_id() may crash the system.
+ *
+ * Most likely, you want to use tracing_on/tracing_off.
+ */
+enum ftrace_dump_mode {
+ DUMP_NONE,
+ DUMP_ALL,
+ DUMP_ORIG,
+ DUMP_PARAM,
+};
+
+#ifdef CONFIG_TRACING
+void tracing_on(void);
+void tracing_off(void);
+int tracing_is_on(void);
+void tracing_snapshot(void);
+void tracing_snapshot_alloc(void);
+void tracing_start(void);
+void tracing_stop(void);
+void trace_dump_stack(int skip);
+void ftrace_dump(enum ftrace_dump_mode oops_dump_mode);
+#else
+static inline void tracing_start(void) { }
+static inline void tracing_stop(void) { }
+static inline void tracing_on(void) { }
+static inline void tracing_off(void) { }
+static inline int tracing_is_on(void) { return 0; }
+static inline void tracing_snapshot(void) { }
+static inline void tracing_snapshot_alloc(void) { }
+static inline void trace_dump_stack(int skip) { }
+static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
+#endif
+
+#endif /* _LINUX_TRACE_CONTROLS_H */
diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 3d54f440dccf..a488ea9e9f85 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -7,43 +7,7 @@
#include <linux/stddef.h>
#include <linux/stringify.h>
-/*
- * General tracing related utility functions - trace_printk(),
- * tracing_on/tracing_off and tracing_start()/tracing_stop
- *
- * Use tracing_on/tracing_off when you want to quickly turn on or off
- * tracing. It simply enables or disables the recording of the trace events.
- * This also corresponds to the user space /sys/kernel/tracing/tracing_on
- * file, which gives a means for the kernel and userspace to interact.
- * Place a tracing_off() in the kernel where you want tracing to end.
- * From user space, examine the trace, and then echo 1 > tracing_on
- * to continue tracing.
- *
- * tracing_stop/tracing_start has slightly more overhead. It is used
- * by things like suspend to ram where disabling the recording of the
- * trace is not enough, but tracing must actually stop because things
- * like calling smp_processor_id() may crash the system.
- *
- * Most likely, you want to use tracing_on/tracing_off.
- */
-
-enum ftrace_dump_mode {
- DUMP_NONE,
- DUMP_ALL,
- DUMP_ORIG,
- DUMP_PARAM,
-};
-
#ifdef CONFIG_TRACING
-void tracing_on(void);
-void tracing_off(void);
-int tracing_is_on(void);
-void tracing_snapshot(void);
-void tracing_snapshot_alloc(void);
-
-extern void tracing_start(void);
-extern void tracing_stop(void);
-
static inline __printf(1, 2)
void ____trace_printk_check_format(const char *fmt, ...)
{
@@ -149,8 +113,6 @@ int __trace_printk(unsigned long ip, const char *fmt, ...);
extern int __trace_bputs(unsigned long ip, const char *str);
extern int __trace_puts(unsigned long ip, const char *str);
-extern void trace_dump_stack(int skip);
-
/*
* The double __builtin_constant_p is because gcc will give us an error
* if we try to allocate the static variable to fmt if it is not a
@@ -173,19 +135,7 @@ __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap);
extern __printf(2, 0) int
__ftrace_vprintk(unsigned long ip, const char *fmt, va_list ap);
-
-extern void ftrace_dump(enum ftrace_dump_mode oops_dump_mode);
#else
-static inline void tracing_start(void) { }
-static inline void tracing_stop(void) { }
-static inline void trace_dump_stack(int skip) { }
-
-static inline void tracing_on(void) { }
-static inline void tracing_off(void) { }
-static inline int tracing_is_on(void) { return 0; }
-static inline void tracing_snapshot(void) { }
-static inline void tracing_snapshot_alloc(void) { }
-
static inline __printf(1, 2)
int trace_printk(const char *fmt, ...)
{
@@ -196,7 +146,6 @@ ftrace_vprintk(const char *fmt, va_list ap)
{
return 0;
}
-static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
#endif /* CONFIG_TRACING */
#endif
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index b276504c1c6b..f9c83a470c98 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -27,6 +27,7 @@
#define pr_fmt(fmt) "KGDB: " fmt
+#include <linux/trace_controls.h>
#include <linux/pid_namespace.h>
#include <linux/clocksource.h>
#include <linux/serial_core.h>
diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..1415e910371d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -9,6 +9,7 @@
* This function is used through-out the kernel (including mm and fs)
* to indicate a major problem.
*/
+#include <linux/trace_controls.h>
#include <linux/debug_locks.h>
#include <linux/sched/debug.h>
#include <linux/interrupt.h>
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index fa6d30ce73d1..735a80df0b30 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <trace/events/rcu.h>
+#include <linux/trace_controls.h>
/*
* Grace-period counter management.
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 882a158ada7b..76bf0184b267 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -39,6 +39,7 @@
#include <linux/srcu.h>
#include <linux/slab.h>
#include <linux/trace_clock.h>
+#include <linux/trace_controls.h>
#include <asm/byteorder.h>
#include <linux/torture.h>
#include <linux/vmalloc.h>
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..2537c33ddd49 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -22,6 +22,7 @@
#include <linux/ctype.h>
#include <linux/once_lite.h>
#include <linux/ftrace_regs.h>
+#include <linux/trace_controls.h>
#include <linux/llist.h>
#include "pid_list.h"
diff --git a/kernel/trace/trace_benchmark.c b/kernel/trace/trace_benchmark.c
index e19c32f2a938..69cc39008c36 100644
--- a/kernel/trace/trace_benchmark.c
+++ b/kernel/trace/trace_benchmark.c
@@ -3,6 +3,7 @@
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/trace_clock.h>
+#include <linux/trace_controls.h>
#define CREATE_TRACE_POINTS
#include "trace_benchmark.h"
diff --git a/lib/sys_info.c b/lib/sys_info.c
index f32a06ec9ed4..e3c9ca05601b 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -8,6 +8,7 @@
#include <linux/ftrace.h>
#include <linux/nmi.h>
#include <linux/sched/debug.h>
+#include <linux/trace_controls.h>
#include <linux/string.h>
#include <linux/sysctl.h>
--
2.53.0
^ permalink raw reply related
* [PATCH v4 2/2] tracing: Remove trace_printk.h from kernel.h
From: Steven Rostedt @ 2026-06-25 10:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Linus Torvalds, Sebastian Andrzej Siewior, John Ogness,
Thomas Gleixner, Peter Zijlstra, Julia Lawall, Yury Norov,
linux-doc, linux-kbuild, linuxppc-dev, dri-devel, linux-stm32,
linux-arm-kernel, linux-rdma, linux-usb, linux-ext4, linux-nfs,
kvm, intel-gfx
In-Reply-To: <20260625104007.041432666@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
There have been complaints about trace_printk.h causing more build time
for being in kernel.h if it changes. There is also an effort to clean up
kernel.h to have it not include unneeded header files. Move trace_printk.h
out of kernel.h and place it in the headers and C files that use it.
Link: https://lore.kernel.org/all/CAHk-=wikCBeVFjVXiY4o-oepdbjAoir5+TcAgtL12c4u1TpZLQ@mail.gmail.com/
Suggested-by: Yury Norov <yury.norov@gmail.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
arch/powerpc/kvm/book3s_xics.c | 1 +
drivers/gpu/drm/i915/gt/intel_gtt.h | 1 +
drivers/gpu/drm/i915/i915_gem.h | 1 +
drivers/hwtracing/stm/dummy_stm.c | 1 +
drivers/infiniband/hw/hfi1/trace_dbg.h | 1 +
drivers/usb/early/xhci-dbc.c | 1 +
fs/ext4/inline.c | 1 +
include/linux/ftrace.h | 2 ++
include/linux/kernel.h | 1 -
include/linux/sunrpc/debug.h | 1 +
include/linux/trace_printk.h | 5 +++--
kernel/trace/ring_buffer_benchmark.c | 1 +
samples/fprobe/fprobe_example.c | 1 +
samples/ftrace/ftrace-direct-too.c | 1 -
samples/trace_printk/trace-printk.c | 1 +
15 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 74a44fa702b0..ef5eb596a56e 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -26,6 +26,7 @@
#if 1
#define XICS_DBG(fmt...) do { } while (0)
#else
+#include <linux/trace_printk.h>
#define XICS_DBG(fmt...) trace_printk(fmt)
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index b54ee4f25af1..f6f223090760 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -35,6 +35,7 @@
#define I915_GFP_ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
#if IS_ENABLED(CONFIG_DRM_I915_TRACE_GTT)
+#include <linux/trace_printk.h>
#define GTT_TRACE(...) trace_printk(__VA_ARGS__)
#else
#define GTT_TRACE(...)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 1da8fb61c09e..f490052e8964 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -117,6 +117,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
#if IS_ENABLED(CONFIG_DRM_I915_TRACE_GEM)
#include <linux/trace_controls.h>
+#include <linux/trace_printk.h>
#define GEM_TRACE(...) trace_printk(__VA_ARGS__)
#define GEM_TRACE_ERR(...) do { \
pr_err(__VA_ARGS__); \
diff --git a/drivers/hwtracing/stm/dummy_stm.c b/drivers/hwtracing/stm/dummy_stm.c
index 38528ffdc0b3..7c5e48ebfb9f 100644
--- a/drivers/hwtracing/stm/dummy_stm.c
+++ b/drivers/hwtracing/stm/dummy_stm.c
@@ -8,6 +8,7 @@
*/
#undef DEBUG
+#include <linux/trace_printk.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
diff --git a/drivers/infiniband/hw/hfi1/trace_dbg.h b/drivers/infiniband/hw/hfi1/trace_dbg.h
index 58304b91380f..30df5e246586 100644
--- a/drivers/infiniband/hw/hfi1/trace_dbg.h
+++ b/drivers/infiniband/hw/hfi1/trace_dbg.h
@@ -103,6 +103,7 @@ __hfi1_trace_def(IOCTL);
*/
#ifdef HFI1_EARLY_DBG
+#include <linux/trace_printk.h>
#define hfi1_dbg_early(fmt, ...) \
trace_printk(fmt, ##__VA_ARGS__)
#else
diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index 41118bba9197..955c73bd601f 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -30,6 +30,7 @@ static struct xdbc_state xdbc;
static bool early_console_keep;
#ifdef XDBC_TRACE
+#include <linux/trace_printk.h>
#define xdbc_trace trace_printk
#else
static inline void xdbc_trace(const char *fmt, ...) { }
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 8045e4ff270c..0eff4a0c6a6c 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -934,6 +934,7 @@ static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
}
#ifdef INLINE_DIR_DEBUG
+#include <linux/trace_printk.h>
void ext4_show_inline_dir(struct inode *dir, struct buffer_head *bh,
void *inline_start, int inline_size)
{
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 02bc5027523a..b5336a81e619 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -8,6 +8,8 @@
#define _LINUX_FTRACE_H
#include <linux/trace_recursion.h>
+#include <linux/trace_controls.h>
+#include <linux/trace_printk.h>
#include <linux/trace_clock.h>
#include <linux/jump_label.h>
#include <linux/kallsyms.h>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index e5570a16cbb1..e87a40fbd152 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -31,7 +31,6 @@
#include <linux/build_bug.h>
#include <linux/sprintf.h>
#include <linux/static_call_types.h>
-#include <linux/trace_printk.h>
#include <linux/util_macros.h>
#include <linux/wordpart.h>
diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index ab61bed2f7af..7524f5d82fba 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -29,6 +29,7 @@ extern unsigned int nlm_debug;
# define ifdebug(fac) if (unlikely(rpc_debug & RPCDBG_##fac))
# if IS_ENABLED(CONFIG_SUNRPC_DEBUG_TRACE)
+# include <linux/trace_printk.h>
# define __sunrpc_printk(fmt, ...) trace_printk(fmt, ##__VA_ARGS__)
# else
# define __sunrpc_printk(fmt, ...) printk(KERN_DEFAULT fmt, ##__VA_ARGS__)
diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index a488ea9e9f85..74ce4f8995c4 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -1,11 +1,12 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef _LINUX_TRACE_PRINTK_H
#define _LINUX_TRACE_PRINTK_H
+#if !defined(__ASSEMBLY__) && !defined(__GENKSYMS__) && !defined(BUILD_VDSO)
-#include <linux/compiler_attributes.h>
#include <linux/instruction_pointer.h>
#include <linux/stddef.h>
#include <linux/stringify.h>
+#include <linux/stdarg.h>
#ifdef CONFIG_TRACING
static inline __printf(1, 2)
@@ -147,5 +148,5 @@ ftrace_vprintk(const char *fmt, va_list ap)
return 0;
}
#endif /* CONFIG_TRACING */
-
+#endif /* !defined(__ASSEMBLY__) && !defined(__GENKSYMS__) && !defined(BUILD_VDSO) */
#endif
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 593e3b59e42e..2bb25caebb75 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -5,6 +5,7 @@
* Copyright (C) 2009 Steven Rostedt <srostedt@redhat.com>
*/
#include <linux/ring_buffer.h>
+#include <linux/trace_printk.h>
#include <linux/completion.h>
#include <linux/kthread.h>
#include <uapi/linux/sched/types.h>
diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
index bfe98ce826f3..de81b9b4ca7d 100644
--- a/samples/fprobe/fprobe_example.c
+++ b/samples/fprobe/fprobe_example.c
@@ -12,6 +12,7 @@
#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <linux/trace_printk.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/fprobe.h>
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index bf2411aa6fd7..159190f4103f 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/module.h>
-
#include <linux/mm.h> /* for handle_mm_fault() */
#include <linux/ftrace.h>
#if !defined(CONFIG_ARM64) && !defined(CONFIG_PPC32)
diff --git a/samples/trace_printk/trace-printk.c b/samples/trace_printk/trace-printk.c
index cfc159580263..ff37aeb8523e 100644
--- a/samples/trace_printk/trace-printk.c
+++ b/samples/trace_printk/trace-printk.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/trace_printk.h>
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/irq_work.h>
--
2.53.0
^ permalink raw reply related
* [PATCH v4 0/2] tracing: Move non-trace_printk prototypes into trace_controls.h
From: Steven Rostedt @ 2026-06-25 10:40 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Linus Torvalds, Sebastian Andrzej Siewior, John Ogness,
Thomas Gleixner, Peter Zijlstra, Julia Lawall, Yury Norov,
linux-doc, linux-kbuild, linuxppc-dev, dri-devel, linux-stm32,
linux-arm-kernel, linux-rdma, linux-usb, linux-ext4, linux-nfs,
kvm, intel-gfx
Remove trace_printk.h by creating a trace_controls.h for those places that
need access to tracing prototypes like tracing_off() and for the places that
need trace_printk() directly, to have it included directly.
Changse since v3: https://lore.kernel.org/all/20260624081806.120105649@kernel.org/
- Always include trace_controls.h in rcu.h (kernel test robot)
There are other configs that may include tracing_off() in rcu.h besides
the one that had the include of trace_controls.h. Just always include
it in that header to be safe.
Steven Rostedt (2):
tracing: Move non-trace_printk prototypes into trace_controls.h
tracing: Remove trace_printk.h from kernel.h
----
arch/powerpc/kvm/book3s_xics.c | 1 +
arch/powerpc/xmon/xmon.c | 1 +
arch/s390/kernel/ipl.c | 1 +
arch/s390/kernel/machine_kexec.c | 1 +
drivers/gpu/drm/i915/gt/intel_gtt.h | 1 +
drivers/gpu/drm/i915/i915_gem.h | 2 ++
drivers/hwtracing/stm/dummy_stm.c | 1 +
drivers/infiniband/hw/hfi1/trace_dbg.h | 1 +
drivers/tty/sysrq.c | 1 +
drivers/usb/early/xhci-dbc.c | 1 +
fs/ext4/inline.c | 1 +
include/linux/ftrace.h | 2 ++
include/linux/kernel.h | 1 -
include/linux/sunrpc/debug.h | 1 +
include/linux/trace_controls.h | 54 ++++++++++++++++++++++++++++++++
include/linux/trace_printk.h | 56 ++--------------------------------
kernel/debug/debug_core.c | 1 +
kernel/panic.c | 1 +
kernel/rcu/rcu.h | 1 +
kernel/rcu/rcutorture.c | 1 +
kernel/trace/ring_buffer_benchmark.c | 1 +
kernel/trace/trace.h | 1 +
kernel/trace/trace_benchmark.c | 1 +
lib/sys_info.c | 1 +
samples/fprobe/fprobe_example.c | 1 +
samples/ftrace/ftrace-direct-too.c | 1 -
samples/trace_printk/trace-printk.c | 1 +
27 files changed, 82 insertions(+), 55 deletions(-)
create mode 100644 include/linux/trace_controls.h
^ permalink raw reply
* Re: [PATCH v5] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Jan Kara @ 2026-06-25 10:23 UTC (permalink / raw)
To: Aditya Srivastava
Cc: tytso, jack, adilger.kernel, libaokun, ritesh.list, yi.zhang,
linux-ext4, linux-kernel, Colin Ian King
In-Reply-To: <20260625065010.2332-1-aditya.ansh182@gmail.com>
On Thu 25-06-26 06:50:09, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
>
> Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
> concurrent xattr workloads (using the ea_inode mount/format option).
>
> The deadlock occurs between the running transaction and the eviction
> thread:
> - Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
> and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
> the corresponding EA inode. Since the EA inode is currently being
> evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
> eviction to complete.
> - Task 2 (eviction thread): Currently evicting the same EA inode in
> ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
> blocks waiting for Task 1 to release the reference to the mbcache_entry.
>
> To break this deadlock, implement a new ext4_iget() configuration flag
> named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
> inode via VFS's find_inode_nowait() API.
>
> If the inode is currently being evicted (marked with I_FREEING or
> I_WILL_FREE) or created (I_CREATING), or if it is not present in the VFS
> inode cache (cache miss), simply skip it (returning -ESTALE) rather than
> waiting for eviction/creation to complete, breaking the ABBA cycle.
>
> Since we return -ESTALE immediately on a cache miss, we never attempt to
> allocate a new inode or call iget_locked(), completely eliminating any
> TOCTOU race window.
>
> If the returned inode is I_NEW, wait for its initialization to clear via
> wait_on_new_inode(). If initialization fails and the inode is unhashed
> during wait_on_new_inode() waking up (e.g., due to an I/O read error in
> another thread), safely drop the reference and return -ESTALE. This
> unhashed check is executed unconditionally on all cache-hit pathways to
> properly handle concurrent initialization failures.
>
> Finally, standard validation checks (including is_bad_inode,
> EXT4_EA_INODE_FL, file_acl, and xattr flags) are executed as normal inside
> check_igot_inode() to fully guarantee VFS-layer safety.
>
> In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
> EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Reported-by: Colin Ian King <colin.i.king@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
> Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
Three patch submissions in a day? I think you are overdoing it. Please test
more, think more about what you do (i.e., really understand what the code
does, not just whatever some AI agent suggested) and submit less.
> +static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
> +{
> + bool *is_freeing = data;
> +
> + if (inode->i_ino != ino)
> + return 0;
> + spin_lock(&inode->i_lock);
> + if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
> + if (is_freeing)
> + *is_freeing = true;
> + spin_unlock(&inode->i_lock);
> + return -1;
> + }
> + __iget(inode);
> + spin_unlock(&inode->i_lock);
> + return 1;
> +}
> +
> struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> ext4_iget_flags flags, const char *function,
> unsigned int line)
> @@ -5298,9 +5316,26 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> return ERR_PTR(-EFSCORRUPTED);
> }
>
> - inode = iget_locked(sb, ino);
> - if (!inode)
> - return ERR_PTR(-ENOMEM);
> + if (flags & EXT4_IGET_NOWAIT) {
> + bool is_freeing = false;
> +
> + inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
> + if (is_freeing || !inode)
is_freeing is pointless. Just drop it and use !inode.
> + return ERR_PTR(-ESTALE);
I'd prefer -ENOENT here instead of -ESTALE. It's kind of more specific.
> + if (inode_state_read_once(inode) & I_NEW)
> + wait_on_new_inode(inode);
> +
> + if (unlikely(inode_unhashed(inode))) {
> + iput(inode);
> + return ERR_PTR(-ESTALE);
Here -ENOENT as well please.
Otherwise the patch looks good to me.
Honza
> + }
> + } else {
> + inode = iget_locked(sb, ino);
> + if (!inode)
> + return ERR_PTR(-ENOMEM);
> + }
> +
> if (!(inode_state_read_once(inode) & I_NEW)) {
> ret = check_igot_inode(inode, flags, function, line);
> if (ret) {
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 982a1f831e22..21b5670d8503 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
>
> while (ce) {
> ea_inode = ext4_iget(inode->i_sb, ce->e_value,
> - EXT4_IGET_EA_INODE);
> + EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
> if (IS_ERR(ea_inode))
> goto next_entry;
> ext4_xattr_inode_set_class(ea_inode);
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v3] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Jan Kara @ 2026-06-25 10:14 UTC (permalink / raw)
To: Aditya Srivastava
Cc: tytso, jack, adilger.kernel, libaokun, ritesh.list, yi.zhang,
linux-ext4, linux-kernel, Colin Ian King
In-Reply-To: <20260625040935.2244-1-aditya.ansh182@gmail.com>
On Thu 25-06-26 04:09:35, Aditya Srivastava wrote:
> From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
>
> Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
> concurrent xattr workloads (using the ea_inode mount/format option).
>
> The deadlock occurs between the running transaction and the eviction
> thread:
> - Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
> and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
> the corresponding EA inode. Since the EA inode is currently being
> evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
> eviction to complete.
> - Task 2 (eviction thread): Currently evicting the same EA inode in
> ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
> blocks waiting for Task 1 to release the reference to the mbcache_entry.
>
> To break this deadlock, implement a new ext4_iget() configuration flag
> named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
> inode via VFS's find_inode_nowait() API.
>
> If the inode is currently being evicted (marked with I_FREEING or
> I_WILL_FREE) or created (I_CREATING), simply skip it (returning -ESTALE)
> rather than waiting for eviction/creation to complete, breaking the ABBA
> cycle. If the returned inode is I_NEW, wait for its initialization to
> clear via wait_on_new_inode(). Finally, standard validation checks
> (including is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr flags) are
> executed as normal inside check_igot_inode() to fully guarantee VFS-layer
> safety.
>
> In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
> EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Reported-by: Colin Ian King <colin.i.king@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
> Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
> Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
...
> +static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
> +{
> + bool *is_freeing = data;
> +
> + if (inode->i_ino != ino)
> + return 0;
> + spin_lock(&inode->i_lock);
> + if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
> + if (is_freeing)
> + *is_freeing = true;
> + spin_unlock(&inode->i_lock);
> + return -1;
> + }
> + __iget(inode);
> + spin_unlock(&inode->i_lock);
> + return 1;
> +}
> +
> struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> ext4_iget_flags flags, const char *function,
> unsigned int line)
> @@ -5298,9 +5316,26 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> return ERR_PTR(-EFSCORRUPTED);
> }
>
> - inode = iget_locked(sb, ino);
> - if (!inode)
> - return ERR_PTR(-ENOMEM);
> + if (flags & EXT4_IGET_NOWAIT) {
> + bool is_freeing = false;
> +
> + inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
> + if (is_freeing)
> + return ERR_PTR(-ESTALE);
In case of EXT4_IGET_NOWAIT I just would not bother with returning errors
like this. Just if the inode was not found or cannot be grabbed, I'd return
say -ENOENT from __ext4_iget() and be done with it. Due to the nature of
EXT4_IGET_NOWAIT it is unreliable and the caller thus has to decide what to
do in case the inode cannot be grabbed.
> + if (!inode) {
> + inode = iget_locked(sb, ino);
> + if (!inode)
> + return ERR_PTR(-ENOMEM);
Hu, what's the point of this? It defeats the purpose of EXT4_IGET_NOWAIT as
it can block...
Honza
> + } else {
> + if (inode_state_read_once(inode) & I_NEW)
> + wait_on_new_inode(inode);
> + }
> + } else {
> + inode = iget_locked(sb, ino);
> + if (!inode)
> + return ERR_PTR(-ENOMEM);
> + }
> +
> if (!(inode_state_read_once(inode) & I_NEW)) {
> ret = check_igot_inode(inode, flags, function, line);
> if (ret) {
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 982a1f831e22..21b5670d8503 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
>
> while (ce) {
> ea_inode = ext4_iget(inode->i_sb, ce->e_value,
> - EXT4_IGET_EA_INODE);
> + EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
> if (IS_ERR(ea_inode))
> goto next_entry;
> ext4_xattr_inode_set_class(ea_inode);
> --
> 2.47.3
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v4 18/23] ext4: wait for ordered I/O in the iomap buffered I/O path
From: Jan Kara @ 2026-06-25 9:42 UTC (permalink / raw)
To: Zhang Yi
Cc: Jan Kara, Zhang Yi, linux-ext4, linux-fsdevel, linux-kernel,
tytso, adilger.kernel, libaokun, ojaswin, ritesh.list, djwong,
hch, yi.zhang, yangerkun, yukuai
In-Reply-To: <9ab9281d-c5dc-4183-8bc9-b53b5a4ac9c0@gmail.com>
On Mon 22-06-26 11:32:16, Zhang Yi wrote:
> On 6/18/2026 9:48 PM, Jan Kara wrote:
> > On Mon 11-05-26 15:23:38, Zhang Yi wrote:
> > > From: Zhang Yi <yi.zhang@huawei.com>
> > >
> > > For append writes, wait for ordered I/O to complete before updating
> > > i_disksize. This ensures that zeroed data is flushed to disk before the
> > > metadata update, preventing stale data from being exposed during
> > > unaligned post-EOF append writes.
> > >
> > > Suggested-by: Jan Kara <jack@suse.cz>
> > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> >
> > Frankly, this all looks too complex to me. Plus your are adding 32-bytes to
> > struct ext4_inode_info which isn't great either. Why don't you just do
> > filemap_fdatawait() for the byte at old i_disksize and be done with it?
> >
> > I believe we have to simplify this. All this complexity (and thus
> > maintenance burden) across several patches for the corner case of zeroing
> > tail block on extention is in my opinion difficult to justify.
> >
> > Honza
>
> Hi, Jan!
>
> Thanks for the review. I understand the concern about complexity and the
> 32-byte increase to ext4_inode_info. I tried using
> filemap_fdatawait_range() as you suggested, but found two issues where
> this solution doesn't work.
>
> 1. ioend worker deadlock
>
> Since worker concurrency resources are limited, we cannot wait for
> another ioend worker to complete within one ioend worker with the same
> work_struct. If the worker calls
> filemap_fdatawait_range(byte_at_old_disksize) to wait for the zeroed
> block's folio writeback to complete, it sleeps holding the only worker
> slot. If the folio contains blocks requiring extent conversion, its
> writeback bit is cleared by iomap_finish_ioends() running inside
> another worker -- which can only run after the current worker finishes
> its batch.
>
> Concretely:
> - Worker W1 processes ioend A, calls filemap_fdatawait_range() on
> the old EOF byte, sleeps.
> - The zeroed data is in ioend B. bio_endio defers it to
> i_iomap_ioend_list and calls queue_work().
> - queue_work() on i_iomap_ioend_work is idempotent: it returns false
> because the work is currently executing (even though sleeping).
> - ioend B sits in the list, never gets processed.
> - The folio writeback bit is only cleared by processing ioend B.
> - W1 sleeps forever -> deadlock.
Yes, good point. We cannot wait for folio writeback completion from end_io
processing for another folio.
> Therefore, I think we have to put the wakeup logic in
> ext4_iomap_end_bio() that runs in interrupt context without consuming
> a worker thread. The ordered range tracking and wait queue are what
> make that possible.
>
> 2. Truncate-up needs an accurate state query
>
> In the follow-up patch 19, ext4_set_inode_size() must make a precise
> decision when updating i_disksize during truncate up.
>
> This needs a state query: "is there ordered zero I/O in flight right
> now?" If yes, the i_disksize update is deferred to
> ext4_iomap_wb_update_disksize(is_ordered=true), which advances
> i_disksize to i_size when the ordered I/O completes. If no, we must
> advance i_disksize immediately, otherwise we will lose the updating
> forever.
>
> Therefore, we need to track the state of the ordered range. Simply
> using filemap_fdatawait_range() doesn't work. i_ordered_len serves as
> a maintained state flag that both the ioend completion path and the
> setattr path can read atomically without sleeping.
>
> Suggestions?
I see. Thanks for explanation. I went back to our discussion back from
February to remind myself about the constraints on the tail block zeroing
and the i_disksize update mechanism. And in the light of complexity of the
current mechanism, I think we've discarded the following possibility too
easily:
* On file extend / truncate up just zero tail folio in the page cache, mark
it dirty, keep i_disksize at old value, update i_size to the new value,
add inode to orphan list.
If the i_disksize was block aligned (and so we skip zeroing), we just
update i_disksize rightaway.
* In io end processing if the folio for which we end io has a block which
straddles i_disksize, we update i_disksize to current i_size. We defer
removing inode from orphan list e.g. to file close time (doing it from
end_io processing is problematic locking wise as we need i_rwsem for it).
This is a very simple scheme with very good performace. It makes sure stale
data in the tail block cannot be seen on disk after a crash.
Potential problems with this:
* We need to do i_disksize update from io end processing which means starting a
transaction. So this mechanism has to be restricted to buffered IO iomap
path due to locking constraints. If the inode uses old ordered mode, it
has to keep using it also for handling of the tail block zeroing.
I think that's acceptable as we want to transition everything to iomap
anyway so this duplicity will eventually go away.
* After a crash we can see i_disksize already updated but file content will
show zeros. This is not breaking any guarantees, it just changes how ext4
behaves after a crash. Again, I think this is acceptable tradeoff for the
simplicity.
What do you think? Any problems I have missed? I'm sorry for poking into
this mechanism again and again but I want to keep the code as simple as
possible to make our life easier in the future...
> Regarding the bloat of ext4_inode_info, perhaps we can drop the
> wait_queue_head_t (24 bytes) and use wait_var_event()/ wake_up_var()
> instead. Would this be acceptable?
Yes, that would be a good way to reduce the bloat if we still need this.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 10/16] fs/buffer: Remove fs-layer decryption code
From: Christian Brauner @ 2026-06-25 7:01 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
linux-block, Christoph Hellwig, Theodore Ts'o, Andreas Dilger,
Baokun Li, Jan Kara, Ojaswin Mujoo, Ritesh Harjani, Zhang Yi,
Jaegeuk Kim, Chao Yu
In-Reply-To: <20260624050334.124606-11-ebiggers@kernel.org>
On 2026-06-23 22:03 -0700, Eric Biggers wrote:
> Now that fscrypt's file contents en/decryption is always implemented
> using blk-crypto when the filesystem is block-based, the fs-layer
> decryption code in fs/buffer.c is unused code. Remove it.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
^ permalink raw reply
* [PATCH v5] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Aditya Srivastava @ 2026-06-25 6:50 UTC (permalink / raw)
To: tytso, jack
Cc: adilger.kernel, libaokun, ritesh.list, yi.zhang, linux-ext4,
linux-kernel, Aditya Prakash Srivastava, Colin Ian King
From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
concurrent xattr workloads (using the ea_inode mount/format option).
The deadlock occurs between the running transaction and the eviction
thread:
- Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
the corresponding EA inode. Since the EA inode is currently being
evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
eviction to complete.
- Task 2 (eviction thread): Currently evicting the same EA inode in
ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
blocks waiting for Task 1 to release the reference to the mbcache_entry.
To break this deadlock, implement a new ext4_iget() configuration flag
named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
inode via VFS's find_inode_nowait() API.
If the inode is currently being evicted (marked with I_FREEING or
I_WILL_FREE) or created (I_CREATING), or if it is not present in the VFS
inode cache (cache miss), simply skip it (returning -ESTALE) rather than
waiting for eviction/creation to complete, breaking the ABBA cycle.
Since we return -ESTALE immediately on a cache miss, we never attempt to
allocate a new inode or call iget_locked(), completely eliminating any
TOCTOU race window.
If the returned inode is I_NEW, wait for its initialization to clear via
wait_on_new_inode(). If initialization fails and the inode is unhashed
during wait_on_new_inode() waking up (e.g., due to an I/O read error in
another thread), safely drop the reference and return -ESTALE. This
unhashed check is executed unconditionally on all cache-hit pathways to
properly handle concurrent initialization failures.
Finally, standard validation checks (including is_bad_inode,
EXT4_EA_INODE_FL, file_acl, and xattr flags) are executed as normal inside
check_igot_inode() to fully guarantee VFS-layer safety.
In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
Suggested-by: Jan Kara <jack@suse.cz>
Reported-by: Colin Ian King <colin.i.king@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
Changes in v5:
- Address two critical issues flagged by the Sashiko AI bot in v4:
1. Resolve the Time-Of-Check to Time-Of-Use (TOCTOU) race window between
find_inode_nowait() and iget_locked() by returning -ESTALE immediately
on a VFS cache miss. This completely bypasses fallback to iget_locked()
and prevents potential ABBA deadlocks.
2. Fix the improperly nested inode_unhashed() safety check by moving it
outside the I_NEW condition block, ensuring it runs unconditionally
on all cache-hit pathways to prevent false-positive filesystem
corruption errors during concurrent initialization failures.
Changes in v4:
- Check if the inode was unhashed during wait_on_new_inode() waking up
to handle transient initialization failures (like I/O read errors)
gracefully. Dropping the reference and returning -ESTALE prevents
false filesystem corruption errors (__ext4_error), as found by the
Sashiko AI bot.
Changes in v3:
- Implement a new ext4_iget() configuration flag named EXT4_IGET_NOWAIT to
fully contain the non-blocking lookup and VFS-level validations within
inode.c, as requested by Jan Kara.
- Skip inodes currently being created (I_CREATING), following Jan Kara's
direct feedback.
- Remove all open-coded match helpers and VFS state-checks from xattr.c.
Changes in v2:
- Read inode state locklessly using inode_state_read_once() to resolve
a lockdep assertion on cache hit.
- Manually restore essential inode/ea_inode validations on the retrieved
inode (is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr checks) to
match VFS safety guarantees and prevent using corrupted/failed inodes.
fs/ext4/ext4.h | 3 ++-
fs/ext4/inode.c | 41 ++++++++++++++++++++++++++++++++++++++---
fs/ext4/xattr.c | 2 +-
3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..c76dd0bdd3d8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3144,7 +3144,8 @@ typedef enum {
EXT4_IGET_SPECIAL = 0x0001, /* OK to iget a system inode */
EXT4_IGET_HANDLE = 0x0002, /* Inode # is from a handle */
EXT4_IGET_BAD = 0x0004, /* Allow to iget a bad inode */
- EXT4_IGET_EA_INODE = 0x0008 /* Inode should contain an EA value */
+ EXT4_IGET_EA_INODE = 0x0008, /* Inode should contain an EA value */
+ EXT4_IGET_NOWAIT = 0x0010 /* Non-blocking lookup (skip if freeing) */
} ext4_iget_flags;
extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..f6b681320358 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5270,6 +5270,24 @@ void ext4_set_inode_mapping_order(struct inode *inode)
mapping_set_folio_order_range(inode->i_mapping, min_order, max_order);
}
+static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
+{
+ bool *is_freeing = data;
+
+ if (inode->i_ino != ino)
+ return 0;
+ spin_lock(&inode->i_lock);
+ if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
+ if (is_freeing)
+ *is_freeing = true;
+ spin_unlock(&inode->i_lock);
+ return -1;
+ }
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ return 1;
+}
+
struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
ext4_iget_flags flags, const char *function,
unsigned int line)
@@ -5298,9 +5316,26 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
return ERR_PTR(-EFSCORRUPTED);
}
- inode = iget_locked(sb, ino);
- if (!inode)
- return ERR_PTR(-ENOMEM);
+ if (flags & EXT4_IGET_NOWAIT) {
+ bool is_freeing = false;
+
+ inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
+ if (is_freeing || !inode)
+ return ERR_PTR(-ESTALE);
+
+ if (inode_state_read_once(inode) & I_NEW)
+ wait_on_new_inode(inode);
+
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ } else {
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ }
+
if (!(inode_state_read_once(inode) & I_NEW)) {
ret = check_igot_inode(inode, flags, function, line);
if (ret) {
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..21b5670d8503 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
while (ce) {
ea_inode = ext4_iget(inode->i_sb, ce->e_value,
- EXT4_IGET_EA_INODE);
+ EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
if (IS_ERR(ea_inode))
goto next_entry;
ext4_xattr_inode_set_class(ea_inode);
--
2.47.3
^ permalink raw reply related
* [PATCH v4] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Aditya Srivastava @ 2026-06-25 6:03 UTC (permalink / raw)
To: tytso, jack
Cc: adilger.kernel, libaokun, ritesh.list, yi.zhang, linux-ext4,
linux-kernel, Aditya Prakash Srivastava, Colin Ian King
From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
concurrent xattr workloads (using the ea_inode mount/format option).
The deadlock occurs between the running transaction and the eviction
thread:
- Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
the corresponding EA inode. Since the EA inode is currently being
evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
eviction to complete.
- Task 2 (eviction thread): Currently evicting the same EA inode in
ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
blocks waiting for Task 1 to release the reference to the mbcache_entry.
To break this deadlock, implement a new ext4_iget() configuration flag
named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
inode via VFS's find_inode_nowait() API.
If the inode is currently being evicted (marked with I_FREEING or
I_WILL_FREE) or created (I_CREATING), simply skip it (returning -ESTALE)
rather than waiting for eviction/creation to complete, breaking the ABBA
cycle. If the returned inode is I_NEW, wait for its initialization to
clear via wait_on_new_inode().
If initialization fails and the inode is unhashed during the waking up of
wait_on_new_inode() (e.g., due to an I/O read error in another thread),
safely drop the reference and return -ESTALE to cleanly bypass the xattr
cache entry. Finally, standard validation checks (including is_bad_inode,
EXT4_EA_INODE_FL, file_acl, and xattr flags) are executed as normal inside
check_igot_inode() to fully guarantee VFS-layer safety.
In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
Suggested-by: Jan Kara <jack@suse.cz>
Reported-by: Colin Ian King <colin.i.king@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
Changes in v4:
- Check if the inode was unhashed during wait_on_new_inode() waking up
to handle transient initialization failures (like I/O read errors)
gracefully. Dropping the reference and returning -ESTALE prevents
false filesystem corruption errors (__ext4_error), as found by the
Sashiko AI bot.
Changes in v3:
- Implement a new ext4_iget() configuration flag named EXT4_IGET_NOWAIT to
fully contain the non-blocking lookup and VFS-level validations within
inode.c, as requested by Jan Kara.
- Skip inodes currently being created (I_CREATING), following Jan Kara's
direct feedback.
- Remove all open-coded match helpers and VFS state-checks from xattr.c.
Changes in v2:
- Read inode state locklessly using inode_state_read_once() to resolve
a lockdep assertion on cache hit.
- Manually restore essential inode/ea_inode validations on the retrieved
inode (is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr checks) to
match VFS safety guarantees and prevent using corrupted/failed inodes.
fs/ext4/ext4.h | 3 ++-
fs/ext4/inode.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
fs/ext4/xattr.c | 2 +-
3 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..c76dd0bdd3d8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3144,7 +3144,8 @@ typedef enum {
EXT4_IGET_SPECIAL = 0x0001, /* OK to iget a system inode */
EXT4_IGET_HANDLE = 0x0002, /* Inode # is from a handle */
EXT4_IGET_BAD = 0x0004, /* Allow to iget a bad inode */
- EXT4_IGET_EA_INODE = 0x0008 /* Inode should contain an EA value */
+ EXT4_IGET_EA_INODE = 0x0008, /* Inode should contain an EA value */
+ EXT4_IGET_NOWAIT = 0x0010 /* Non-blocking lookup (skip if freeing) */
} ext4_iget_flags;
extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..75ed467f5abf 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5270,6 +5270,24 @@ void ext4_set_inode_mapping_order(struct inode *inode)
mapping_set_folio_order_range(inode->i_mapping, min_order, max_order);
}
+static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
+{
+ bool *is_freeing = data;
+
+ if (inode->i_ino != ino)
+ return 0;
+ spin_lock(&inode->i_lock);
+ if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
+ if (is_freeing)
+ *is_freeing = true;
+ spin_unlock(&inode->i_lock);
+ return -1;
+ }
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ return 1;
+}
+
struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
ext4_iget_flags flags, const char *function,
unsigned int line)
@@ -5298,9 +5316,31 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
return ERR_PTR(-EFSCORRUPTED);
}
- inode = iget_locked(sb, ino);
- if (!inode)
- return ERR_PTR(-ENOMEM);
+ if (flags & EXT4_IGET_NOWAIT) {
+ bool is_freeing = false;
+
+ inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
+ if (is_freeing)
+ return ERR_PTR(-ESTALE);
+ if (!inode) {
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ } else {
+ if (inode_state_read_once(inode) & I_NEW) {
+ wait_on_new_inode(inode);
+ if (unlikely(inode_unhashed(inode))) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ }
+ }
+ } else {
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ }
+
if (!(inode_state_read_once(inode) & I_NEW)) {
ret = check_igot_inode(inode, flags, function, line);
if (ret) {
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..21b5670d8503 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
while (ce) {
ea_inode = ext4_iget(inode->i_sb, ce->e_value,
- EXT4_IGET_EA_INODE);
+ EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
if (IS_ERR(ea_inode))
goto next_entry;
ext4_xattr_inode_set_class(ea_inode);
--
2.47.3
^ permalink raw reply related
* [PATCH v3] ext4: fix ABBA deadlock in ext4_xattr_inode_cache_find()
From: Aditya Srivastava @ 2026-06-25 4:09 UTC (permalink / raw)
To: tytso, jack
Cc: adilger.kernel, libaokun, ritesh.list, yi.zhang, linux-ext4,
linux-kernel, Aditya Prakash Srivastava, Colin Ian King
From: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
Syzbot/stress-ng reported an ABBA deadlock in ext4 when exercising
concurrent xattr workloads (using the ea_inode mount/format option).
The deadlock occurs between the running transaction and the eviction
thread:
- Task 1 (stress-ng): Holds a reference to a shared mbcache_entry (ce)
and calls ext4_xattr_inode_cache_find() -> ext4_iget() to retrieve
the corresponding EA inode. Since the EA inode is currently being
evicted, ext4_iget() blocks in __wait_on_freeing_inode() waiting for
eviction to complete.
- Task 2 (eviction thread): Currently evicting the same EA inode in
ext4_evict_ea_inode(). It calls mb_cache_entry_wait_unused(oe) which
blocks waiting for Task 1 to release the reference to the mbcache_entry.
To break this deadlock, implement a new ext4_iget() configuration flag
named EXT4_IGET_NOWAIT. When set, perform a non-blocking lookup of the
inode via VFS's find_inode_nowait() API.
If the inode is currently being evicted (marked with I_FREEING or
I_WILL_FREE) or created (I_CREATING), simply skip it (returning -ESTALE)
rather than waiting for eviction/creation to complete, breaking the ABBA
cycle. If the returned inode is I_NEW, wait for its initialization to
clear via wait_on_new_inode(). Finally, standard validation checks
(including is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr flags) are
executed as normal inside check_igot_inode() to fully guarantee VFS-layer
safety.
In ext4_xattr_inode_cache_find(), invoke ext4_iget() with the new
EXT4_IGET_NOWAIT flag to perform the non-blocking cache search.
Suggested-by: Jan Kara <jack@suse.cz>
Reported-by: Colin Ian King <colin.i.king@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219283
Fixes: 0a46ef234756 ("ext4: do not create EA inode under buffer lock")
Signed-off-by: Aditya Prakash Srivastava <aditya.ansh182@gmail.com>
---
Changes in v3:
- Implement a new ext4_iget() configuration flag named EXT4_IGET_NOWAIT to
fully contain the non-blocking lookup and VFS-level validations within
inode.c, as requested by Jan Kara.
- Skip inodes currently being created (I_CREATING), following Jan Kara's
direct feedback.
- Remove all open-coded match helpers and VFS state-checks from xattr.c.
Changes in v2:
- Read inode state locklessly using inode_state_read_once() to resolve
a lockdep assertion on cache hit.
- Manually restore essential inode/ea_inode validations on the retrieved
inode (is_bad_inode, EXT4_EA_INODE_FL, file_acl, and xattr checks) to
match VFS safety guarantees and prevent using corrupted/failed inodes.
fs/ext4/ext4.h | 3 ++-
fs/ext4/inode.c | 41 ++++++++++++++++++++++++++++++++++++++---
fs/ext4/xattr.c | 2 +-
3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..c76dd0bdd3d8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3144,7 +3144,8 @@ typedef enum {
EXT4_IGET_SPECIAL = 0x0001, /* OK to iget a system inode */
EXT4_IGET_HANDLE = 0x0002, /* Inode # is from a handle */
EXT4_IGET_BAD = 0x0004, /* Allow to iget a bad inode */
- EXT4_IGET_EA_INODE = 0x0008 /* Inode should contain an EA value */
+ EXT4_IGET_EA_INODE = 0x0008, /* Inode should contain an EA value */
+ EXT4_IGET_NOWAIT = 0x0010 /* Non-blocking lookup (skip if freeing) */
} ext4_iget_flags;
extern struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..42a798f333d3 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5270,6 +5270,24 @@ void ext4_set_inode_mapping_order(struct inode *inode)
mapping_set_folio_order_range(inode->i_mapping, min_order, max_order);
}
+static int ext4_iget_match(struct inode *inode, u64 ino, void *data)
+{
+ bool *is_freeing = data;
+
+ if (inode->i_ino != ino)
+ return 0;
+ spin_lock(&inode->i_lock);
+ if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_CREATING)) {
+ if (is_freeing)
+ *is_freeing = true;
+ spin_unlock(&inode->i_lock);
+ return -1;
+ }
+ __iget(inode);
+ spin_unlock(&inode->i_lock);
+ return 1;
+}
+
struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
ext4_iget_flags flags, const char *function,
unsigned int line)
@@ -5298,9 +5316,26 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
return ERR_PTR(-EFSCORRUPTED);
}
- inode = iget_locked(sb, ino);
- if (!inode)
- return ERR_PTR(-ENOMEM);
+ if (flags & EXT4_IGET_NOWAIT) {
+ bool is_freeing = false;
+
+ inode = find_inode_nowait(sb, ino, ext4_iget_match, &is_freeing);
+ if (is_freeing)
+ return ERR_PTR(-ESTALE);
+ if (!inode) {
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ } else {
+ if (inode_state_read_once(inode) & I_NEW)
+ wait_on_new_inode(inode);
+ }
+ } else {
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ }
+
if (!(inode_state_read_once(inode) & I_NEW)) {
ret = check_igot_inode(inode, flags, function, line);
if (ret) {
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..21b5670d8503 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1550,7 +1550,7 @@ ext4_xattr_inode_cache_find(struct inode *inode, const void *value,
while (ce) {
ea_inode = ext4_iget(inode->i_sb, ce->e_value,
- EXT4_IGET_EA_INODE);
+ EXT4_IGET_EA_INODE | EXT4_IGET_NOWAIT);
if (IS_ERR(ea_inode))
goto next_entry;
ext4_xattr_inode_set_class(ea_inode);
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v4 09/23] ext4: implement writeback path using iomap
From: Zhang Yi @ 2026-06-25 3:33 UTC (permalink / raw)
To: Jan Kara, Zhang Yi
Cc: linux-ext4, linux-fsdevel, linux-kernel, tytso, adilger.kernel,
libaokun, ojaswin, ritesh.list, djwong, hch, yi.zhang, yangerkun,
yukuai
In-Reply-To: <i536qqwj5eyulec3r2ki2ycnelqdd4bkpat2drn7t72t6p622k@ktueynysgo3j>
On 6/25/2026 1:16 AM, Jan Kara wrote:
> On Mon 22-06-26 20:36:02, Zhang Yi wrote:
>> On 6/16/2026 7:47 PM, Jan Kara wrote:
>>> On Mon 11-05-26 15:23:29, Zhang Yi wrote:
>>>> From: Zhang Yi <yi.zhang@huawei.com>
>>>>
>>>> Add the iomap writeback path for ext4 buffered I/O. This introduces:
>>>>
>>>> - ext4_iomap_writepages(): the main writeback entry point.
>>>> - ext4_writeback_ops: a new iomap_writeback_ops instance to handle
>>>> block mapping and I/O submission.
>>>> - A new end I/O worker for converting unwritten extents, updating file
>>>> size, and handling DATA_ERR_ABORT after I/O completion.
>>>>
>>>> Core implementation details:
>>>>
>>>> - ->writeback_range() callback
>>>> Calls ext4_iomap_map_writeback_range() to query the longest range of
>>>> existing mapped extents. For performance, when a block range is not
>>>> yet allocated, it allocates based on the writeback length and delalloc
>>>> extent length, rather than allocating for a single folio at a time.
>>>> The folio is then added to an iomap_ioend instance.
>>>>
>>>> - ->writeback_submit() callback
>>>> Registers ext4_iomap_end_bio() as the end bio callback. This callback
>>>> schedules a worker to handle:
>>>> - Unwritten extent conversion.
>>>> - i_disksize update after data is written back.
>>>> - Journal abort on writeback I/O failure.
>>>>
>>>> Key changes and considerations:
>>>>
>>>> - Append write and unwritten extents
>>>> Since data=ordered mode is not used to prevent stale data exposure
>>>> during append writebacks, new blocks are always allocated as unwritten
>>>> extents (i.e. always enable dioread_nolock), and i_disksize update is
>>>> postponed until I/O completion. Additionally, the deadlock that the
>>>> reserve handle was expected to resolve does not occur anymore.
>>>> Therefore, the end I/O worker can start a normal journal handle
>>>> instead of a reserve handle when converting unwritten extents.
>>>>
>>>> - Lock ordering
>>>> The ->writeback_range() callback runs under the folio lock, requiring
>>>> the journal handle to be started under that same lock. This reverses
>>>> the order compared to the buffer_head writeback path. The lock ordering
>>>> documentation in super.c has been updated accordingly.
>>>>
>>>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>>>> ---
>>>> fs/ext4/ext4.h | 4 +
>>>> fs/ext4/inode.c | 208 +++++++++++++++++++++++++++++++++++++++++-
>>>> fs/ext4/page-io.c | 126 +++++++++++++++++++++++++
>>>> fs/ext4/super.c | 7 +-
>>>> fs/iomap/ioend.c | 3 +-
>>>> include/linux/iomap.h | 1 +
>>>> 6 files changed, 346 insertions(+), 3 deletions(-)
>>>>
[...]
>> There are actually two reasons for this. First, we want to avoid
>> starting a journal handle in overwrite scenarios. Second, we want to
>> be able to query the extent locklessly without holding i_data_sem in
>> overwrite cases as well (note that ext4_es_lookup_extent() in
>> ext4_iomap_map_one_extent() is called with i_data_sem held).
>>
>> I ran a set of benchmark tests in my VM, performing the following FIO
>> overwrite test on a 500GB ramdisk:
>>
>> $fio -filename=/test_dir/foo -direct=0 -iodepth=8 -fsync=0 -rw=write \
>> -numjobs=1 -bs=4k -ioengine=io_uring -size=20G -uncached=1 \
>> -runtime=30 --ramp_time=5s -time_based -norandommap=0 \
>> -fallocate=none -overwrite=1 \
>> -group_reportin -name=test --output=/tmp/log
>>
>> The results are as follows:
>>
>> a: on a non-fragmented file
>> A: on a fragmented file [1]
>> b: no background metadata pressure
>> B: with background metadata pressure [2]
>>
>> buffer_head | iomap pre-map w/o journal | iomap directly map
>> a+b: 680 691 690
>> a+B: 560 568 567
>> A+b: 637 633 579
>> A+B: 540 571 495
>>
>> [1] The file is pre-fragmented such that each block occupies a separate
>> extent.
>> [2] A background fsstress process is running (only contains metadata
>> ops):
>> taskset -c 2 fsstress -c -d /test_dir -l 0 -n 1000 -f clonerange=0 \
>> -f copyrange=0 -f awrite=0 -f aread=0 -f dread=0 \
>> -f dwrite=0 -f mread=0 -f mwrite=0 -f readv=0 -f write=0 \
>> -f writev=0 -f read=0 -f sync=0 -f afsync=0 -f fsync=0
>>
>> As can be seen, for large contiguous files, the performance impact is
>> minimal. However, in heavily fragmented scenarios or under other
>> metadata pressure, pre-querying the mapping brings noticeable gains.
>> However, this is testing the most extreme case — I'm not sure about
>> the real-world impact, so I don't have a strong preference either way.
>> But I suppose faster is better, at least not slower than the old
>> buffer_head path. :)
>
> OK, thanks for the test! So for fragmented files the optimization of not
> starting a transaction seems indeed worth it. I still dislike the
> opencoding :) So given we have the reversed lock ordering now, why don't we
> teach ext4_map_blocks() to start a transaction (if not provided) just
> before it acquires i_data_sem for writing? This should be quite elegant. I
> know you have some concerns about possible races below so let's discuss
> that separately but at least in terms of performance and code complexity
> this would look ideal :).
>
Yeah, I agree with you on this point. But let's discuss the below race
issue first. :)
>>> then I'd
>>> probably prefer coming up with an ext4_get_blocks flag which tells it to
>>> start a transaction on its own if we need to allocate blocks... That would
>>> be much simpler than opencoding all this.
>>
>> Additionally, there is a key point here. The reason I open-coded
>> ext4_iomap_map_writeback_range() is that we must ensure extent query
>> and allocation are performed atomically under i_data_sem. Otherwise,
>> concurrent truncate could lead to quota leaks.
>>
>> Specifically, consider the following scenario: we call
>> ext4_map_blocks() to allocate blocks. Suppose there is a delalloc
>> extent covering blocks [0,3). While writeback is submitting block 0, a
>> concurrent truncate(block 1) occurs:
>>
>> wb truncate
>> ext4_es_lookup_extent() ext4_truncate_down()
>> //get [0,3)
>> truncate_inode_pages_range()
>> //clear page 1&2
>> ext4_truncate()
>> down_write(i_data_sem)
>> ext4_es_remove_extent()
>> //drop extent [1,3)
>> //i_reserved_data_blocks: 3->1
>> up_write(i_data_sem)
>> down_write(i_data_sem)
>> ext4_map_create_blocks()
>> //alloc 3 blocks
>> ext4_es_insert_extent()
>> //only reclaim 1 block，stale 2 blocks
>> up_write(i_data_sem)
>>
>> Therefore, If we don't open-coding this part, we would need to
>> significantly rework ext4_map_blocks(), which might have a larger
>> impact at this point. What do you think?
>
> Hum, is something like this really possible? I mean iomap_writepages() will
> lookup and lock folio. Only then it calls ->iomap_begin to map it to
> underlying blocks. And folio lock synchronizes against
> truncate_inode_pages_range() so how would writeback end up trying to
> allocate something underlying pages 1 or 2?
I believe this scenario can indeed occur, and folio lock alone is
insufficient to protect against this concurrency issue. The main reason
is that the iomap writeback framework processes folios one by one. For
each folio, it follows the "lock -> map -> unlock" sequence. If each
iteration only mapped blocks covering no more than one folio in length,
performance would be severely degraded. Therefore, both XFS and the
current ext4 iomap implementation choose to map up to the minimum of the
writeback length and the delalloc extent length. This means that when
processing folio 0, if an extent of length 3 is found, the ranges
corresponding to the subsequent folios are also mapped and cached. As a
result, holding only the folio lock of folio 0 is insufficient to
protect against truncation concurrency with the latter two folios.
This issue does not occur in the original buffer_head writeback path,
ext4_do_writepages(). In that path, a batch of consecutive folios to be
mapped are locked upfront before the mapping operation. Therefore, the
blocks within the corresponding range are protected by folio locks
during mapping, making it impossible for truncation to race with
writeback.
[...]
>>>> +void ext4_iomap_end_bio(struct bio *bio)
>>>> +{
>>>> + struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
>>>> + struct inode *inode = ioend->io_inode;
>>>> + struct ext4_inode_info *ei = EXT4_I(inode);
>>>> + struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>>>> + unsigned long flags;
>>>> +
>>>> + /* Needs to convert unwritten extents or update the i_disksize. */
>>>> + if ((ioend->io_flags & IOMAP_IOEND_UNWRITTEN) ||
>>>> + ioend->io_offset + ioend->io_size > READ_ONCE(ei->i_disksize))
>>>> + goto defer;
>>>> +
>>>> + /* Needs to abort the journal on data_err=abort. */
>>>> + if (unlikely(ioend->io_bio.bi_status))
>>>> + goto defer;
>>>> +
>>>> + iomap_finish_ioend(ioend, 0);
>>>> + return;
>>>> +defer:
>>>> + spin_lock_irqsave(&ei->i_completed_io_lock, flags);
>>>> + if (list_empty(&ei->i_iomap_ioend_list))
>>>> + queue_work(sbi->rsv_conversion_wq, &ei->i_iomap_ioend_work);
>>>> + list_add_tail(&ioend->io_list, &ei->i_iomap_ioend_list);
>>>> + spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
>>>> +}
>>>
>>> For now, I'd prefer to do what XFS does and offload everything. Then you
>>> don't have to export iomap_finish_ioend() (which would need to be in a
>>> separate patch and acked by iomap maintainers) and the code is more
>>> standard. There's a patchset in the works which adds general ioend offloading
>>> infrastructure into iomap [1] and when that lands we should get all these
>> ^^^^^ block layer?
>>
>>> bells and whistles (even better ones with percpu work queues, batching,
>>> etc.) for free.
>>>
>>> [1] https://lore.kernel.org/all/20260514-blk-dontcache-v6-0-782e2fa7477b@columbia.edu/
>>>
>>> Honza
>>
>> Ha, I've noticed this patchset, so I haven't implemented
>> uncached I/O handling for now. As a side note, I have a question:
>> if we convert all endio processing to worker threads, IIRC, my
>> recollection from previous performance tests is that pure overwrite
>> scenarios would see at least a 20% degradation. Is that acceptable?
>
> No, but the latest version of the patches exactly does IO completion in the
> interrupt unless the bio is flagged as needing IO completion from process
> context or unless end_io handler returns a particular error - which means
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Please let me confirm whether my understanding is correct. The latest(v6)
modification to bio_endio() is as follows:
@@ -1791,7 +1865,9 @@ void bio_endio(struct bio *bio)
}
#endif
- if (bio->bi_end_io)
+ if (bio_flagged(bio, BIO_COMPLETE_IN_TASK) && bio_in_atomic())
+ __bio_complete_in_task(bio);
+ else if (bio->bi_end_io)
bio->bi_end_io(bio);
}
Currently, __bio_complete_in_task() is only called in bio_endio() when
BIO_COMPLETE_IN_TASK is explicitly set on the bio. It is not called
again based on a specific error return from bio->bi_end_io(). So I
believe what you meant is that each filesystem's own ->bi_end_io()
should call it to convert the endio processing to process context.
Is my understanding correct?
> IO completion wasn't actually done and needs offloading into process
> context instead.
>
>> I understand why uncached I/O might need the entire completion path
>> in a worker, but can we complete the I/O in interrupt context for
>> pure overwrite and then release the page cache in a worker? Must
>> page cache invalidation and I/O completion be synchronous?
>
> Strictly speaking no, we can first complete the IO and evict page cache
> independently later. But it would be quite tricky locking wise (folios and
> the mapping containing them can get evicted once folio writeback bit gets
> cleared) so the whole uncached writes handling would have to be reworked. I
> don't think it's worth it at this point.
Ah, I see. Thanks for the clarification. :)
>
>> The reason I kept ext4_iomap_end_bio() handling I/O completion in
>> interrupt context is for overwrite performance. XFS also handles
>> overwrites in interrupt context (via ioend_writeback_end_bio()).
>> However, ext4 has the data_error=abort mount option — when this mode
>> is set and an I/O error occurs, we must abort the journal in a
>> worker. Since we cannot predict I/O errors at submission time, we
>> can't directly use ioend_writeback_end_bio() and must instead bind
>> our own ext4_iomap_end_bio(). At the same time, I want to avoid
>> spawning a worker for pure overwrites when no I/O error occurs, so I
>> exported iomap_finish_ioend(). What do you think?
>
> So data_error=abort handling can exactly use the new generic framework - if
> we detect during processing IO completion we cannot actually do it in the
> interrupt (like in case of error), we just return appropriately from the
> handler and the generic code will handle offloading and call the ->end_io
> callback again.
>
> Honza
If my understanding above is correct, what you are suggesting here is
that in ext4_iomap_end_bio(), we should call __bio_complete_in_task()
to switch to process context when data_error=abort is set and the I/O
returns an error. This way, we could drop ext4's own
i_iomap_ioend_work / i_rsv_conversion_work handling. Is that right?
But even with that, it seems we would still need to export
iomap_finish_ioend(). Because if the I/O does not fail,
ext4_iomap_end_bio() would need to complete the IO processing in
interrupt context for pure overwrite and would not switch to tasks
context. Because only iomap_finish_ioend() is safe to call in interrupt
context. Am I misunderstanding something?
Thanks,
Yi.
^ permalink raw reply
* Re: [PATCH v4] iomap: add simple read path for small direct I/O
From: Fengnan @ 2026-06-25 2:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: brauner, djwong, ojaswin, dgc, linux-xfs, linux-fsdevel,
linux-ext4, linux-kernel, lidiangang
In-Reply-To: <ajv5pqNureiK80Eu@infradead.org>
在 2026/6/24 23:37, Christoph Hellwig 写道:
> Sorry for the delay in getting back to this, I'm a bit overloaded at
> the moment.
>
>> -static inline bool should_report_dio_fserror(const struct iomap_dio *dio)
>> +static inline bool should_report_dio_fserror(int error)
> Can you split all the refactoring into prep patches?
off course
>
>> +/*
>> + * In the async simple read path, we need to prevent bio_endio() from
>> + * triggering iocb->ki_complete() before the submitter has returned
>> + * -EIOCBQUEUED. Otherwise, the caller might free the iocb concurrently.
>> + *
>> + * We use a three-state rendezvous to synchronize the submitter and end_io:
>> + *
>> + * IOMAP_DIO_SIMPLE_SUBMITTING: Initial state set before submitting the bio.
>> + *
>> + * IOMAP_DIO_SIMPLE_QUEUED: The submitter has safely queued the IO and will
>> + * return -EIOCBQUEUED. If end_io sees this state, it takes over and calls
>> + * ki_complete().
>> + *
>> + * IOMAP_DIO_SIMPLE_DONE: end_io fired before the submitter finished the
>> + * submit path. end_io sets this state and does nothing else. The submitter
>> + * will see this state and handle the completion synchronously (bypassing
>> + * ki_complete() and returning the actual result).
>> + */
> I don't think we actually need any of this. For the sync case we
> can just use submit_bio_wait, and for async just always complete
> from the end_io handler. This will simplify the implementation a lot,
> and also avoid the atomic.
I was wrong before, in simple read path, we won't use sr after submit bio.
>
>> +static void iomap_dio_simple_read_async_done(struct iomap_dio_simple_read *sr)
> Btw, I'd drop the _read in the name. Most of this would work as-is
> for trivial overwrites if we figure out when to use them.
ok, let's rename to iomap_dio_simple_xxx
>
>> + if (dio_flags & IOMAP_DIO_BOUNCE)
>> + nr_pages = bio_iov_bounce_nr_vecs(iter, REQ_OP_READ);
>> + else
>> + nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS);
> Bounce buffering requires dops, so all this can be dropped.
Get .
>
>> + ret = ops->iomap_begin(inode, iomi.pos, count, iomi.flags,
>> + &iomi.iomap, &iomi.srcmap);
>> + if (ret) {
>> + inode_dio_end(inode);
>> + return ret;
>> + }
>> +
>> + if (iomi.iomap.type != IOMAP_MAPPED ||
>> + iomi.iomap.offset > iomi.pos ||
> I don't think offset > pos can happen
>
>> + iomi.iomap.offset + iomi.iomap.length < iomi.pos + count ||
>> + (iomi.iomap.flags & IOMAP_F_INTEGRITY)) {
>> + ret = -ENOTBLK;
>> + goto out_iomap_end;
>> + }
> Given that we already have a fallback here, I'm not sure why this is
> limited to a single file system block. Anything that:
>
> a) fits into the iomap
> b) fits into a single bio
>
> can be easily supported. The first condition is a trivial, and for
> the second we could just check if iter->nr_segs is larger than
> BIO_MAX_VECS.
The reason I only added a simple path for 4K reads is that, in current
NVMe, 4K random reads suffer from a significant bottleneck, whereas
8K reads and 4K writes do not.
Considering that if an 8K or larger block size does not fit within a single
BIO, two iomap_begin/iomap_end calls would be required, resulting in
additional overhead.
Of course, even without considering this scenario, there would still be
some benefit, it’s just not as significant.
How do you approach this issue?
>
>> + if (user_backed_iter(iter))
>> + dio_flags |= IOMAP_DIO_USER_BACKED;
>> + bio->bi_iter.bi_sector = iomap_sector(&iomi.iomap, iomi.pos);
>> + bio->bi_ioprio = iocb->ki_ioprio;
>> + bio->bi_private = sr;
>> + bio->bi_end_io = iomap_dio_simple_read_end_io;
>> +
>> + if ((dio_flags & IOMAP_DIO_USER_BACKED) &&
>> + !(dio_flags & IOMAP_DIO_BOUNCE))
>> + bio_set_pages_dirty(bio);
>> +
>> + if (iocb->ki_flags & IOCB_NOWAIT)
>> + bio->bi_opf |= REQ_NOWAIT;
>> + if ((iocb->ki_flags & IOCB_HIPRI) && !wait_for_completion) {
>> + bio->bi_opf |= REQ_POLLED;
>> + bio_set_polled(bio, iocb);
>> + WRITE_ONCE(iocb->private, bio);
>> + }
> Can you check if sone more of this can be factored into a shared
> helper?
I'll try.
>
> Below is a completely untested patch implementing my suggestion
> for the completion simplification. It compiles, but that's about
> the guarantees I can give for it:
I'll apply and do some test.
>
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 3cb179752612..c785512e5339 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -909,11 +909,7 @@ struct iomap_dio_simple_read {
> struct kiocb *iocb;
> size_t size;
> unsigned int dio_flags;
> - atomic_t state;
> - union {
> - struct task_struct *waiter;
> - struct work_struct work;
> - };
> + struct work_struct work;
> /*
> * Align @bio to a cacheline boundary so that, combined with the
> * front_pad passed to bioset_init(), the bio sits at the start of
> @@ -926,35 +922,12 @@ struct iomap_dio_simple_read {
>
> static struct bio_set iomap_dio_simple_read_pool;
>
> -/*
> - * In the async simple read path, we need to prevent bio_endio() from
> - * triggering iocb->ki_complete() before the submitter has returned
> - * -EIOCBQUEUED. Otherwise, the caller might free the iocb concurrently.
> - *
> - * We use a three-state rendezvous to synchronize the submitter and end_io:
> - *
> - * IOMAP_DIO_SIMPLE_SUBMITTING: Initial state set before submitting the bio.
> - *
> - * IOMAP_DIO_SIMPLE_QUEUED: The submitter has safely queued the IO and will
> - * return -EIOCBQUEUED. If end_io sees this state, it takes over and calls
> - * ki_complete().
> - *
> - * IOMAP_DIO_SIMPLE_DONE: end_io fired before the submitter finished the
> - * submit path. end_io sets this state and does nothing else. The submitter
> - * will see this state and handle the completion synchronously (bypassing
> - * ki_complete() and returning the actual result).
> - */
> -enum {
> - IOMAP_DIO_SIMPLE_SUBMITTING = 0,
> - IOMAP_DIO_SIMPLE_QUEUED,
> - IOMAP_DIO_SIMPLE_DONE,
> -};
> -
> -static ssize_t iomap_dio_simple_read_finish(struct kiocb *iocb,
> - struct bio *bio, ssize_t ret)
> +static ssize_t iomap_dio_simple_read_complete(struct iomap_dio_simple_read *sr)
> {
> + struct bio *bio = &sr->bio;
> + struct kiocb *iocb = sr->iocb;
> struct inode *inode = file_inode(iocb->ki_filp);
> - struct iomap_dio_simple_read *sr = bio->bi_private;
> + ssize_t ret = blk_status_to_errno(bio->bi_status);
>
> if (likely(!ret)) {
> ret = sr->size;
> @@ -965,21 +938,6 @@ static ssize_t iomap_dio_simple_read_finish(struct kiocb *iocb,
> }
>
> iomap_dio_bio_release_pages(bio, sr->dio_flags, ret < 0);
> -
> - return ret;
> -}
> -
> -static ssize_t iomap_dio_simple_read_complete(struct kiocb *iocb,
> - struct bio *bio)
> -{
> - struct inode *inode = file_inode(iocb->ki_filp);
> - ssize_t ret;
> -
> - WRITE_ONCE(iocb->private, NULL);
> -
> - ret = iomap_dio_simple_read_finish(iocb, bio,
> - blk_status_to_errno(bio->bi_status));
> -
> inode_dio_end(inode);
> trace_iomap_dio_complete(iocb, ret < 0 ? ret : 0, ret > 0 ? ret : 0);
> return ret;
> @@ -988,45 +946,26 @@ static ssize_t iomap_dio_simple_read_complete(struct kiocb *iocb,
> static void iomap_dio_simple_read_complete_work(struct work_struct *work)
> {
> struct iomap_dio_simple_read *sr =
> - container_of(work, struct iomap_dio_simple_read, work);
> - struct kiocb *iocb = sr->iocb;
> - ssize_t ret;
> + container_of(work, struct iomap_dio_simple_read, work);
>
> - ret = iomap_dio_simple_read_complete(iocb, &sr->bio);
> - iocb->ki_complete(iocb, ret);
> + WRITE_ONCE(sr->iocb->private, NULL);
> + sr->iocb->ki_complete(sr->iocb, iomap_dio_simple_read_complete(sr));
> }
>
> -static void iomap_dio_simple_read_async_done(struct iomap_dio_simple_read *sr)
> +static void iomap_dio_simple_read_end_io(struct bio *bio)
> {
> - struct kiocb *iocb = sr->iocb;
> + struct iomap_dio_simple_read *sr =
> + container_of(bio, struct iomap_dio_simple_read, bio);
>
> if (unlikely(sr->bio.bi_status)) {
> - struct inode *inode = file_inode(iocb->ki_filp);
> + struct inode *inode = file_inode(sr->iocb->ki_filp);
>
> INIT_WORK(&sr->work, iomap_dio_simple_read_complete_work);
> queue_work(inode->i_sb->s_dio_done_wq, &sr->work);
> return;
> }
>
> - iomap_dio_simple_read_complete_work(&sr->work);
> -}
> -
> -static void iomap_dio_simple_read_end_io(struct bio *bio)
> -{
> - struct iomap_dio_simple_read *sr = bio->bi_private;
> -
> - if (sr->waiter) {
> - struct task_struct *waiter = sr->waiter;
> -
> - WRITE_ONCE(sr->waiter, NULL);
> - blk_wake_io_task(waiter);
> - return;
> - }
> -
> - if (likely(atomic_read(&sr->state) == IOMAP_DIO_SIMPLE_QUEUED) ||
> - atomic_cmpxchg(&sr->state, IOMAP_DIO_SIMPLE_SUBMITTING,
> - IOMAP_DIO_SIMPLE_DONE) == IOMAP_DIO_SIMPLE_QUEUED)
> - iomap_dio_simple_read_async_done(sr);
> + sr->iocb->ki_complete(sr->iocb, iomap_dio_simple_read_complete(sr));
> }
>
> static inline bool iomap_dio_simple_read_supported(struct kiocb *iocb,
> @@ -1046,11 +985,13 @@ static inline bool iomap_dio_simple_read_supported(struct kiocb *iocb,
> */
> if (count > inode->i_sb->s_blocksize)
> return false;
> - if (dio_flags & (IOMAP_DIO_FORCE_WAIT | IOMAP_DIO_PARTIAL))
> + if (dio_flags & (IOMAP_DIO_FORCE_WAIT | IOMAP_DIO_PARTIAL |
> + IOMAP_DIO_BOUNCE))
> return false;
> if (iocb->ki_pos + count > i_size_read(inode))
> return false;
>
> + // XXX: reject fscrypt
> return true;
> }
>
> @@ -1060,7 +1001,6 @@ static ssize_t iomap_dio_simple_read(struct kiocb *iocb,
> {
> struct inode *inode = file_inode(iocb->ki_filp);
> size_t count = iov_iter_count(iter);
> - int nr_pages;
> struct iomap_dio_simple_read *sr;
> unsigned int alignment;
> struct iomap_iter iomi = {
> @@ -1074,11 +1014,6 @@ static ssize_t iomap_dio_simple_read(struct kiocb *iocb,
> bool wait_for_completion = is_sync_kiocb(iocb);
> ssize_t ret;
>
> - if (dio_flags & IOMAP_DIO_BOUNCE)
> - nr_pages = bio_iov_bounce_nr_vecs(iter, REQ_OP_READ);
> - else
> - nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS);
> -
> if (iocb->ki_flags & IOCB_NOWAIT)
> iomi.flags |= IOMAP_NOWAIT;
>
> @@ -1120,24 +1055,18 @@ static ssize_t iomap_dio_simple_read(struct kiocb *iocb,
> if (user_backed_iter(iter))
> dio_flags |= IOMAP_DIO_USER_BACKED;
>
> - bio = bio_alloc_bioset(iomi.iomap.bdev, nr_pages,
> - REQ_OP_READ | REQ_SYNC | REQ_IDLE,
> - GFP_KERNEL, &iomap_dio_simple_read_pool);
> + bio = bio_alloc_bioset(iomi.iomap.bdev,
> + bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
> + REQ_OP_READ | REQ_SYNC | REQ_IDLE,
> + GFP_KERNEL, &iomap_dio_simple_read_pool);
> sr = container_of(bio, struct iomap_dio_simple_read, bio);
> -
> - fscrypt_set_bio_crypt_ctx(bio, inode, iomi.pos, GFP_KERNEL);
> sr->iocb = iocb;
> sr->dio_flags = dio_flags;
>
> bio->bi_iter.bi_sector = iomap_sector(&iomi.iomap, iomi.pos);
> bio->bi_ioprio = iocb->ki_ioprio;
> - bio->bi_private = sr;
> - bio->bi_end_io = iomap_dio_simple_read_end_io;
>
> - if (dio_flags & IOMAP_DIO_BOUNCE)
> - ret = bio_iov_iter_bounce(bio, iter, count);
> - else
> - ret = bio_iov_iter_get_pages(bio, iter, alignment - 1);
> + ret = bio_iov_iter_get_pages(bio, iter, alignment - 1);
> if (unlikely(ret))
> goto out_bio_put;
>
> @@ -1161,49 +1090,22 @@ static ssize_t iomap_dio_simple_read(struct kiocb *iocb,
> WRITE_ONCE(iocb->private, bio);
> }
>
> - if (wait_for_completion) {
> - sr->waiter = current;
> - blk_crypto_submit_bio(bio);
> - } else {
> - atomic_set(&sr->state, IOMAP_DIO_SIMPLE_SUBMITTING);
> - sr->waiter = NULL;
> - blk_crypto_submit_bio(bio);
> - ret = -EIOCBQUEUED;
> - }
> -
> if (ops->iomap_end)
> ops->iomap_end(inode, iomi.pos, count, count, iomi.flags,
> &iomi.iomap);
>
> - if (wait_for_completion) {
> - for (;;) {
> - set_current_state(TASK_UNINTERRUPTIBLE);
> - if (!READ_ONCE(sr->waiter))
> - break;
> - blk_io_schedule();
> - }
> - __set_current_state(TASK_RUNNING);
> -
> - ret = iomap_dio_simple_read_finish(iocb, bio,
> - blk_status_to_errno(bio->bi_status));
> - inode_dio_end(inode);
> - trace_iomap_dio_complete(iocb, ret < 0 ? ret : 0,
> - ret > 0 ? ret : 0);
> - } else if (atomic_cmpxchg(&sr->state, IOMAP_DIO_SIMPLE_SUBMITTING,
> - IOMAP_DIO_SIMPLE_QUEUED) ==
> - IOMAP_DIO_SIMPLE_DONE) {
> - ret = iomap_dio_simple_read_complete(iocb, bio);
> - } else {
> + if (!wait_for_completion) {
> + bio->bi_end_io = iomap_dio_simple_read_end_io;
> + submit_bio(bio);
> trace_iomap_dio_rw_queued(inode, iomi.pos, count);
> + return -EIOCBQUEUED;
> }
>
> - return ret;
> + submit_bio_wait(bio);
> + return iomap_dio_simple_read_complete(sr);
>
> out_bio_release_pages:
> - if (dio_flags & IOMAP_DIO_BOUNCE)
> - bio_iov_iter_unbounce(bio, true, false);
> - else
> - bio_release_pages(bio, false);
> + bio_release_pages(bio, false);
> out_bio_put:
> bio_put(bio);
> out_iomap_end:
^ permalink raw reply
* [syzbot ci] Re: Data in direntry (dirdata) feature
From: syzbot ci @ 2026-06-24 23:18 UTC (permalink / raw)
To: ablagodarenko, adilger.kernel, adilger, adilger,
artem.blagodarenko, linux-ext4, pravin.shelar, xiaowu.417
Cc: syzbot, syzkaller-bugs
In-Reply-To: <20260624133642.18438-1-ablagodarenko@thelustrecollective.com>
syzbot ci has tested the following series
[v4] Data in direntry (dirdata) feature
https://lore.kernel.org/all/20260624133642.18438-1-ablagodarenko@thelustrecollective.com
* [PATCH v4 01/11] ext4: validate count against limit in ext4_dx_csum_verify/_set
* [PATCH v4 02/11] ext4: replace ext4_dir_entry with ext4_dir_entry_2
* [PATCH v4 03/11] ext4: add ext4_dir_entry_is_tail()
* [PATCH v4 04/11] ext4: refactor dx_root to support variable dirent sizes
* [PATCH v4 05/11] ext4: add dirdata format definitions and access helpers
* [PATCH v4 06/11] ext4: preserve dirdata bits in get_dtype()
* [PATCH v4 07/11] ext4: add ext4_dir_entry_len() and harden dirdata parsing
* [PATCH v4 08/11] ext4: rename ext4_dir_rec_len() and clarify dirdata usage
* [PATCH v4 09/11] ext4: dirdata feature
* [PATCH v4 10/11] ext4: add dirdata set/get helpers
* [PATCH v4 11/11] ext4: Add EXT4_IOC_SET_LUFID ioctl for setting LUFID on directory entries
and found the following issues:
* KASAN: slab-use-after-free Read in ext4_inlinedir_to_tree
* KASAN: use-after-free Read in ext4_inlinedir_to_tree
Full report is available here:
https://ci.syzbot.org/series/7075f9f8-5dad-4e13-83ee-2f76e1e06dcf
***
KASAN: slab-use-after-free Read in ext4_inlinedir_to_tree
tree: torvalds
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base: 840ef6c78e6a2f694b578ecb9063241c992aaa9e
arch: amd64
compiler: Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config: https://ci.syzbot.org/builds/c9c607fc-012f-4e4c-88e7-89d5bade9f75/config
syz repro: https://ci.syzbot.org/findings/3badb95c-16ec-4f87-adf6-da2aca94c39c/syz_repro
EXT4-fs (loop1): mounted filesystem 00000000-0000-0000-0000-000000000000 r/w without journal. Quota mode: none.
==================================================================
BUG: KASAN: slab-use-after-free in ext4_dirent_get_data_len fs/ext4/ext4.h:4156 [inline]
BUG: KASAN: slab-use-after-free in ext4_dir_entry_len fs/ext4/ext4.h:4189 [inline]
BUG: KASAN: slab-use-after-free in ext4_inlinedir_to_tree+0x864/0x1030 fs/ext4/inline.c:1339
Read of size 1 at addr ffff888108ff7c19 by task syz.1.18/5891
CPU: 0 UID: 0 PID: 5891 Comm: syz.1.18 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_address_description+0x55/0x1e0 mm/kasan/report.c:378
print_report+0x58/0x70 mm/kasan/report.c:482
kasan_report+0x117/0x150 mm/kasan/report.c:595
ext4_dirent_get_data_len fs/ext4/ext4.h:4156 [inline]
ext4_dir_entry_len fs/ext4/ext4.h:4189 [inline]
ext4_inlinedir_to_tree+0x864/0x1030 fs/ext4/inline.c:1339
ext4_htree_fill_tree+0x4b9/0x2140 fs/ext4/namei.c:1206
ext4_dx_readdir fs/ext4/dir.c:600 [inline]
ext4_readdir+0x2e2a/0x3720 fs/ext4/dir.c:146
iterate_dir+0x2e2/0x4d0 fs/readdir.c:110
ovl_dir_read+0x141/0x4a0 fs/overlayfs/readdir.c:388
ovl_check_d_type_supported+0xc5/0x150 fs/overlayfs/readdir.c:1167
ovl_make_workdir fs/overlayfs/super.c:695 [inline]
ovl_get_workdir fs/overlayfs/super.c:836 [inline]
ovl_fill_super_creds fs/overlayfs/super.c:1449 [inline]
ovl_fill_super+0x3a43/0x5d40 fs/overlayfs/super.c:1560
vfs_get_super fs/super.c:1267 [inline]
get_tree_nodev+0xbb/0x150 fs/super.c:1286
vfs_get_tree+0x92/0x2a0 fs/super.c:1694
fc_mount fs/namespace.c:1198 [inline]
do_new_mount_fc fs/namespace.c:3765 [inline]
do_new_mount+0x319/0xdc0 fs/namespace.c:3841
do_mount fs/namespace.c:4174 [inline]
__do_sys_mount fs/namespace.c:4390 [inline]
__se_sys_mount+0x31d/0x420 fs/namespace.c:4367
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f09e159ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f09e24a9028 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007f09e1815fa0 RCX: 00007f09e159ce59
RDX: 0000200000000000 RSI: 0000200000000100 RDI: 0000000000000000
RBP: 00007f09e1632e6f R08: 00002000000000c0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f09e1816038 R14: 00007f09e1815fa0 R15: 00007fff17222ac8
</TASK>
Allocated by task 5642:
kasan_save_stack mm/kasan/common.c:57 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
__kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:415
kasan_kmalloc include/linux/kasan.h:263 [inline]
__do_kmalloc_node mm/slub.c:5362 [inline]
__kmalloc_node_track_caller_noprof+0x4c3/0x730 mm/slub.c:5497
kmemdup_noprof+0x2b/0x70 mm/util.c:138
kmemdup_noprof include/linux/fortify-string.h:715 [inline]
xfrm6_net_sysctl_init net/ipv6/xfrm6_policy.c:206 [inline]
xfrm6_net_init+0x86/0x180 net/ipv6/xfrm6_policy.c:261
ops_init+0x35d/0x5d0 net/core/net_namespace.c:137
setup_net+0x118/0x350 net/core/net_namespace.c:446
copy_net_ns+0x4f9/0x720 net/core/net_namespace.c:579
create_new_namespaces+0x3f0/0x6b0 kernel/nsproxy.c:132
unshare_nsproxy_namespaces+0x149/0x190 kernel/nsproxy.c:234
ksys_unshare+0x57d/0xa00 kernel/fork.c:3267
__do_sys_unshare kernel/fork.c:3341 [inline]
__se_sys_unshare kernel/fork.c:3339 [inline]
__x64_sys_unshare+0x38/0x50 kernel/fork.c:3339
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Freed by task 12:
kasan_save_stack mm/kasan/common.c:57 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:584
poison_slab_object mm/kasan/common.c:253 [inline]
__kasan_slab_free+0x5c/0x80 mm/kasan/common.c:285
kasan_slab_free include/linux/kasan.h:235 [inline]
slab_free_hook mm/slub.c:2705 [inline]
slab_free mm/slub.c:6405 [inline]
kfree+0x1c5/0x640 mm/slub.c:6720
xfrm6_net_sysctl_exit net/ipv6/xfrm6_policy.c:238 [inline]
xfrm6_net_exit+0x79/0xa0 net/ipv6/xfrm6_policy.c:270
ops_exit_list net/core/net_namespace.c:199 [inline]
ops_undo_list+0x43d/0x8d0 net/core/net_namespace.c:252
cleanup_net+0x572/0x810 net/core/net_namespace.c:702
process_one_work kernel/workqueue.c:3322 [inline]
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
worker_thread+0xa47/0xfb0 kernel/workqueue.c:3486
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
The buggy address belongs to the object at ffff888108ff7c00
which belongs to the cache kmalloc-64 of size 64
The buggy address is located 25 bytes inside of
freed 64-byte region [ffff888108ff7c00, ffff888108ff7c40)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x108ff7
flags: 0x17ff00000000000(node=0|zone=2|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 017ff00000000000 ffff8881000418c0 dead000000000100 dead000000000122
raw: 0000000000000000 0000000800200020 00000000f5000000 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0xd2cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 1, tgid 1 (swapper/0), ts 14581267674, free_ts 0
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f9/0x250 mm/page_alloc.c:1859
prep_new_page mm/page_alloc.c:1867 [inline]
get_page_from_freelist+0x21fa/0x2270 mm/page_alloc.c:3946
__alloc_frozen_pages_noprof+0x18d/0x380 mm/page_alloc.c:5304
alloc_slab_page mm/slub.c:3294 [inline]
allocate_slab+0x79/0x5e0 mm/slub.c:3408
new_slab mm/slub.c:3454 [inline]
refill_objects+0x2d5/0x350 mm/slub.c:7338
refill_sheaf mm/slub.c:2832 [inline]
__pcs_replace_empty_main+0x2bf/0x6b0 mm/slub.c:4703
alloc_from_pcs mm/slub.c:4801 [inline]
slab_alloc_node mm/slub.c:4933 [inline]
__do_kmalloc_node mm/slub.c:5361 [inline]
__kmalloc_noprof+0x485/0x720 mm/slub.c:5387
_kmalloc_noprof include/linux/slab.h:973 [inline]
_kzalloc_noprof include/linux/slab.h:1290 [inline]
kobject_get_path+0xc5/0x2f0 lib/kobject.c:161
kobject_uevent_env+0x29e/0x9e0 lib/kobject_uevent.c:548
device_add+0x544/0xb80 drivers/base/core.c:3738
device_create_groups_vargs drivers/base/core.c:4454 [inline]
device_create+0x269/0x300 drivers/base/core.c:4493
mon_bin_add+0xb6/0x130 drivers/usb/mon/mon_bin.c:1371
mon_bus_init+0x162/0x2a0 drivers/usb/mon/mon_main.c:291
mon_bus_add drivers/usb/mon/mon_main.c:188 [inline]
mon_notify+0x10c/0x3f0 drivers/usb/mon/mon_main.c:219
notifier_call_chain+0x1a5/0x3d0 kernel/notifier.c:85
blocking_notifier_call_chain+0x6a/0x90 kernel/notifier.c:380
page_owner free stack trace missing
Memory state around the buggy address:
ffff888108ff7b00: 00 00 00 00 00 00 00 04 fc fc fc fc fc fc fc fc
ffff888108ff7b80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>ffff888108ff7c00: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
^
ffff888108ff7c80: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
ffff888108ff7d00: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
==================================================================
***
KASAN: use-after-free Read in ext4_inlinedir_to_tree
tree: torvalds
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base: 840ef6c78e6a2f694b578ecb9063241c992aaa9e
arch: amd64
compiler: Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config: https://ci.syzbot.org/builds/c9c607fc-012f-4e4c-88e7-89d5bade9f75/config
syz repro: https://ci.syzbot.org/findings/560b0247-7e29-4a4c-91b8-c73d275cb34f/syz_repro
loop0: lost filesystem error report for type 5 error -117
EXT4-fs (loop0): mounted filesystem 00000000-0000-0000-0000-000000000000 r/w without journal. Quota mode: none.
==================================================================
BUG: KASAN: use-after-free in ext4_dirent_get_data_len fs/ext4/ext4.h:4156 [inline]
BUG: KASAN: use-after-free in ext4_dir_entry_len fs/ext4/ext4.h:4189 [inline]
BUG: KASAN: use-after-free in ext4_inlinedir_to_tree+0x864/0x1030 fs/ext4/inline.c:1339
Read of size 1 at addr ffff888113752019 by task syz.0.17/5794
CPU: 0 UID: 0 PID: 5794 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_address_description+0x55/0x1e0 mm/kasan/report.c:378
print_report+0x58/0x70 mm/kasan/report.c:482
kasan_report+0x117/0x150 mm/kasan/report.c:595
ext4_dirent_get_data_len fs/ext4/ext4.h:4156 [inline]
ext4_dir_entry_len fs/ext4/ext4.h:4189 [inline]
ext4_inlinedir_to_tree+0x864/0x1030 fs/ext4/inline.c:1339
ext4_htree_fill_tree+0x4b9/0x2140 fs/ext4/namei.c:1206
ext4_dx_readdir fs/ext4/dir.c:600 [inline]
ext4_readdir+0x2e2a/0x3720 fs/ext4/dir.c:146
iterate_dir+0x2e2/0x4d0 fs/readdir.c:110
ovl_dir_read+0x141/0x4a0 fs/overlayfs/readdir.c:388
ovl_check_d_type_supported+0xc5/0x150 fs/overlayfs/readdir.c:1167
ovl_make_workdir fs/overlayfs/super.c:695 [inline]
ovl_get_workdir fs/overlayfs/super.c:836 [inline]
ovl_fill_super_creds fs/overlayfs/super.c:1449 [inline]
ovl_fill_super+0x3a43/0x5d40 fs/overlayfs/super.c:1560
vfs_get_super fs/super.c:1267 [inline]
get_tree_nodev+0xbb/0x150 fs/super.c:1286
vfs_get_tree+0x92/0x2a0 fs/super.c:1694
fc_mount fs/namespace.c:1198 [inline]
do_new_mount_fc fs/namespace.c:3765 [inline]
do_new_mount+0x319/0xdc0 fs/namespace.c:3841
do_mount fs/namespace.c:4174 [inline]
__do_sys_mount fs/namespace.c:4390 [inline]
__se_sys_mount+0x31d/0x420 fs/namespace.c:4367
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f427399ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffce99c0f38 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007f4273c15fa0 RCX: 00007f427399ce59
RDX: 0000200000000000 RSI: 0000200000000100 RDI: 0000000000000000
RBP: 00007f4273a32e6f R08: 00002000000000c0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f4273c15fac R14: 00007f4273c15fa0 R15: 00007f4273c15fa0
</TASK>
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888113752e00 pfn:0x113752
flags: 0x17ff00000000000(node=0|zone=2|lastcpupid=0x7ff)
raw: 017ff00000000000 ffffea0004296d08 ffffea0004501b08 0000000000000000
raw: ffff888113752e00 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as freed
page last allocated via order 1, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 5462, tgid 5462 (rm), ts 46014589837, free_ts 77936825094
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1f9/0x250 mm/page_alloc.c:1859
prep_new_page mm/page_alloc.c:1867 [inline]
get_page_from_freelist+0x21fa/0x2270 mm/page_alloc.c:3946
__alloc_frozen_pages_noprof+0x18d/0x380 mm/page_alloc.c:5304
alloc_slab_page mm/slub.c:3294 [inline]
allocate_slab+0x79/0x5e0 mm/slub.c:3408
new_slab mm/slub.c:3454 [inline]
refill_objects+0x2d5/0x350 mm/slub.c:7338
refill_sheaf mm/slub.c:2832 [inline]
__prefill_sheaf_pfmemalloc mm/slub.c:5035 [inline]
kmem_cache_prefill_sheaf+0x2fb/0x550 mm/slub.c:5123
mt_get_sheaf lib/maple_tree.c:154 [inline]
mas_alloc_nodes+0x1c2/0x350 lib/maple_tree.c:1119
mas_preallocate+0x2cf/0x630 lib/maple_tree.c:4961
vma_iter_prealloc mm/vma.h:577 [inline]
__split_vma+0x318/0xa50 mm/vma.c:529
vms_gather_munmap_vmas+0x322/0x1370 mm/vma.c:1427
__mmap_setup mm/vma.c:2439 [inline]
__mmap_region mm/vma.c:2756 [inline]
mmap_region+0x8f9/0x2310 mm/vma.c:2860
do_mmap+0xc3b/0x10c0 mm/mmap.c:560
vm_mmap_pgoff+0x272/0x4e0 mm/util.c:581
ksys_mmap_pgoff+0x4dc/0x760 mm/mmap.c:606
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
page last free pid 5736 tgid 5736 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
__free_pages_prepare mm/page_alloc.c:1406 [inline]
__free_frozen_pages+0xc1e/0xd10 mm/page_alloc.c:2950
__slab_free+0x274/0x2c0 mm/slub.c:5767
qlink_free mm/kasan/quarantine.c:163 [inline]
qlist_free_all+0x99/0x100 mm/kasan/quarantine.c:179
kasan_quarantine_reduce+0x148/0x160 mm/kasan/quarantine.c:286
__kasan_slab_alloc+0x22/0x80 mm/kasan/common.c:350
kasan_slab_alloc include/linux/kasan.h:253 [inline]
slab_post_alloc_hook mm/slub.c:4612 [inline]
slab_alloc_node mm/slub.c:4945 [inline]
__kmalloc_cache_noprof+0x2ab/0x660 mm/slub.c:5511
_kmalloc_noprof include/linux/slab.h:969 [inline]
_kzalloc_noprof include/linux/slab.h:1290 [inline]
ref_tracker_alloc+0x15b/0x4b0 lib/ref_tracker.c:270
__netdev_tracker_alloc include/linux/netdevice.h:4489 [inline]
netdev_hold include/linux/netdevice.h:4518 [inline]
rx_queue_add_kobject net/core/net-sysfs.c:1236 [inline]
net_rx_queue_update_kobjects+0x1c4/0x780 net/core/net-sysfs.c:1301
register_queue_kobjects net/core/net-sysfs.c:2093 [inline]
netdev_register_kobject+0x21f/0x310 net/core/net-sysfs.c:2341
register_netdevice+0x1433/0x1eb0 net/core/dev.c:11439
ipvlan_link_new+0x3e3/0xa90 drivers/net/ipvlan/ipvlan_main.c:593
rtnl_newlink_create+0x310/0xb00 net/core/rtnetlink.c:3905
__rtnl_newlink net/core/rtnetlink.c:4036 [inline]
rtnl_newlink+0x167f/0x1bd0 net/core/rtnetlink.c:4151
rtnetlink_rcv_msg+0x802/0xc00 net/core/rtnetlink.c:7068
netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2556
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7bb/0x940 net/netlink/af_netlink.c:1345
Memory state around the buggy address:
ffff888113751f00: 00 00 00 00 00 00 00 04 fc fc fc fc fc fc fc fc
ffff888113751f80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>ffff888113752000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff888113752080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888113752100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).
The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.
^ permalink raw reply
* Re: [PATCH RFC v2 17/18] fs: look up the superblock via the device table in user_get_super()
From: Gao Xiang @ 2026-06-24 22:48 UTC (permalink / raw)
To: Darrick J. Wong, Christian Brauner
Cc: Jan Kara, Christoph Hellwig, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260624175417.GU6078@frogsfrogsfrogs>
On 2026/6/25 01:54, Darrick J. Wong wrote:
> On Tue, Jun 16, 2026 at 04:08:33PM +0200, Christian Brauner wrote:
>> user_get_super() still finds the superblock for a device number by
>> walking the global super_blocks list under sb_lock. Every superblock is
>> registered in the device table under its s_dev since sget_fc() inserts
>> it there, including superblocks on anonymous devices, so use the table
>> instead.
>>
>> The refcount-pinning cursor helpers super_dev_{get,first,next}() only
>> touch table state and do not depend on CONFIG_BLOCK, so drop the
>> CONFIG_BLOCK guard around them: their new caller serves anonymous
>> devices as well (ustat() on e.g. tmpfs) and is built without
>> CONFIG_BLOCK. The guard falls in this patch rather than separately
>> since without this caller the helpers would be unused without
>> CONFIG_BLOCK.
>>
>> The pinned entry holds a passive reference on the superblock so
>> super_lock() can be called directly; once the superblock is locked grab
>> a passive reference for the caller before dropping the pin.
>>
>> The device table contains more than the old walk could find: a
>> superblock is also registered for every additional device it claims
>> (the xfs log and realtime devices, btrfs member devices, the ext4
>> external journal, erofs blob devices). Don't filter those out:
>> specifying any device a filesystem uses now resolves to that
>> filesystem, so ustat() and quotactl() work on e.g. the xfs log device
>> or a btrfs member device (the latter used to fail outright as btrfs
>> superblocks carry an anonymous s_dev that never matches a member
>> device). When several superblocks share a device (erofs blob devices)
>> the first live superblock wins.
>
> Does erofs have a means to find the other superblocks that share a
> device given a notification coming in on one of them?
Nope, erofs currently doesn't have a way to find the other
superblocks (it doesn't maintain the relationship). My previous
thought is that because it's a read-only filesystem, IMHO, there
is not a must to implement shutdown or notification mechanism in
erofs itself, just because it's strictly immutable (no local
write or dirty journals), and block layer can return io error
on dead bdevs directly even it's a shared block dev. But I may
be wrong if there are reason that we should maintain the
relationship.
Currently it only uses sb->s_type as the holder for bdev sharing,
I think Christian meant that.
Thanks,
Gao Xiang
^ permalink raw reply
* Re: [PATCH RFC v2 17/18] fs: look up the superblock via the device table in user_get_super()
From: Darrick J. Wong @ 2026-06-24 17:54 UTC (permalink / raw)
To: Christian Brauner
Cc: Jan Kara, Christoph Hellwig, Jens Axboe, Alexander Viro,
linux-block, linux-kernel, linux-fsdevel, Carlos Maiolino,
linux-xfs, Chris Mason, David Sterba, linux-btrfs,
Theodore Ts'o, linux-ext4, Gao Xiang, linux-erofs
In-Reply-To: <20260616-work-super-bdev_holder_global-v2-17-7df6b864028e@kernel.org>
On Tue, Jun 16, 2026 at 04:08:33PM +0200, Christian Brauner wrote:
> user_get_super() still finds the superblock for a device number by
> walking the global super_blocks list under sb_lock. Every superblock is
> registered in the device table under its s_dev since sget_fc() inserts
> it there, including superblocks on anonymous devices, so use the table
> instead.
>
> The refcount-pinning cursor helpers super_dev_{get,first,next}() only
> touch table state and do not depend on CONFIG_BLOCK, so drop the
> CONFIG_BLOCK guard around them: their new caller serves anonymous
> devices as well (ustat() on e.g. tmpfs) and is built without
> CONFIG_BLOCK. The guard falls in this patch rather than separately
> since without this caller the helpers would be unused without
> CONFIG_BLOCK.
>
> The pinned entry holds a passive reference on the superblock so
> super_lock() can be called directly; once the superblock is locked grab
> a passive reference for the caller before dropping the pin.
>
> The device table contains more than the old walk could find: a
> superblock is also registered for every additional device it claims
> (the xfs log and realtime devices, btrfs member devices, the ext4
> external journal, erofs blob devices). Don't filter those out:
> specifying any device a filesystem uses now resolves to that
> filesystem, so ustat() and quotactl() work on e.g. the xfs log device
> or a btrfs member device (the latter used to fail outright as btrfs
> superblocks carry an anonymous s_dev that never matches a member
> device). When several superblocks share a device (erofs blob devices)
> the first live superblock wins.
Does erofs have a means to find the other superblocks that share a
device given a notification coming in on one of them? As hch says, it
feels weird to have a lookup mechanism when there's also an upcall
mechanism.
<shrug> I've been on vacation for a while so maybe I missed that there's
another use for the bdev->sb lookup? There are 1600 more emails for me
to go through... :P
--D
>
> The cursor also keeps scanning past dying superblocks where the old
> walk gave up after the first s_dev match, so a mount racing with the
> unmount of the same device (or with the reuse of a recycled anonymous
> dev_t) finds the live superblock where the old walk could spuriously
> return NULL.
>
> This removes the last s_dev-keyed walk of the super_blocks list and
> takes ustat() and quotactl()'s block device lookup off sb_lock
> entirely.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
> fs/super.c | 28 ++++++++--------------------
> 1 file changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 2d0a07861bfc..93f24aea75c4 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -501,7 +501,6 @@ static int super_dev_register(struct super_block *sb)
> return err;
> }
>
> -#ifdef CONFIG_BLOCK
> static struct super_dev *super_dev_get(struct rhlist_head *pos)
> {
> struct super_dev *sb_dev;
> @@ -535,7 +534,6 @@ static struct super_dev *super_dev_next(struct super_dev *prev)
> super_dev_put(prev);
> return sb_dev;
> }
> -#endif
>
> static void kill_super_notify(struct super_block *sb)
> {
> @@ -1044,29 +1042,19 @@ EXPORT_SYMBOL(iterate_supers_type);
>
> struct super_block *user_get_super(dev_t dev, bool excl)
> {
> - struct super_block *sb;
> -
> - spin_lock(&sb_lock);
> - list_for_each_entry(sb, &super_blocks, s_list) {
> - bool locked;
> + struct super_dev *sb_dev;
>
> - if (sb->s_dev != dev)
> - continue;
> + for (sb_dev = super_dev_first(dev); sb_dev; sb_dev = super_dev_next(sb_dev)) {
> + struct super_block *sb = sb_dev->sd_sb;
>
> - if (!refcount_inc_not_zero(&sb->s_passive))
> + if (!super_lock(sb, excl))
> continue;
>
> - spin_unlock(&sb_lock);
> -
> - locked = super_lock(sb, excl);
> - if (locked)
> - return sb;
> -
> - put_super(sb);
> - spin_lock(&sb_lock);
> - break;
> + /* The pinned entry holds a passive reference, take our own. */
> + refcount_inc(&sb->s_passive);
> + super_dev_put(sb_dev);
> + return sb;
> }
> - spin_unlock(&sb_lock);
> return NULL;
> }
>
>
> --
> 2.47.3
>
>
^ permalink raw reply
* Re: [PATCH v4 09/23] ext4: implement writeback path using iomap
From: Jan Kara @ 2026-06-24 17:16 UTC (permalink / raw)
To: Zhang Yi
Cc: Jan Kara, Zhang Yi, linux-ext4, linux-fsdevel, linux-kernel,
tytso, adilger.kernel, libaokun, ojaswin, ritesh.list, djwong,
hch, yi.zhang, yangerkun, yukuai
In-Reply-To: <b0781809-4759-4e12-be17-71555b764f48@gmail.com>
On Mon 22-06-26 20:36:02, Zhang Yi wrote:
> On 6/16/2026 7:47 PM, Jan Kara wrote:
> > On Mon 11-05-26 15:23:29, Zhang Yi wrote:
> > > From: Zhang Yi <yi.zhang@huawei.com>
> > >
> > > Add the iomap writeback path for ext4 buffered I/O. This introduces:
> > >
> > > - ext4_iomap_writepages(): the main writeback entry point.
> > > - ext4_writeback_ops: a new iomap_writeback_ops instance to handle
> > > block mapping and I/O submission.
> > > - A new end I/O worker for converting unwritten extents, updating file
> > > size, and handling DATA_ERR_ABORT after I/O completion.
> > >
> > > Core implementation details:
> > >
> > > - ->writeback_range() callback
> > > Calls ext4_iomap_map_writeback_range() to query the longest range of
> > > existing mapped extents. For performance, when a block range is not
> > > yet allocated, it allocates based on the writeback length and delalloc
> > > extent length, rather than allocating for a single folio at a time.
> > > The folio is then added to an iomap_ioend instance.
> > >
> > > - ->writeback_submit() callback
> > > Registers ext4_iomap_end_bio() as the end bio callback. This callback
> > > schedules a worker to handle:
> > > - Unwritten extent conversion.
> > > - i_disksize update after data is written back.
> > > - Journal abort on writeback I/O failure.
> > >
> > > Key changes and considerations:
> > >
> > > - Append write and unwritten extents
> > > Since data=ordered mode is not used to prevent stale data exposure
> > > during append writebacks, new blocks are always allocated as unwritten
> > > extents (i.e. always enable dioread_nolock), and i_disksize update is
> > > postponed until I/O completion. Additionally, the deadlock that the
> > > reserve handle was expected to resolve does not occur anymore.
> > > Therefore, the end I/O worker can start a normal journal handle
> > > instead of a reserve handle when converting unwritten extents.
> > >
> > > - Lock ordering
> > > The ->writeback_range() callback runs under the folio lock, requiring
> > > the journal handle to be started under that same lock. This reverses
> > > the order compared to the buffer_head writeback path. The lock ordering
> > > documentation in super.c has been updated accordingly.
> > >
> > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> > > ---
> > > fs/ext4/ext4.h | 4 +
> > > fs/ext4/inode.c | 208 +++++++++++++++++++++++++++++++++++++++++-
> > > fs/ext4/page-io.c | 126 +++++++++++++++++++++++++
> > > fs/ext4/super.c | 7 +-
> > > fs/iomap/ioend.c | 3 +-
> > > include/linux/iomap.h | 1 +
> > > 6 files changed, 346 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > > index 4832e7f7db82..078feda47e36 100644
> > > --- a/fs/ext4/ext4.h
> > > +++ b/fs/ext4/ext4.h
> > > @@ -1173,6 +1173,8 @@ struct ext4_inode_info {
> > > */
> > > struct list_head i_rsv_conversion_list;
> > > struct work_struct i_rsv_conversion_work;
> > > + struct list_head i_iomap_ioend_list;
> > > + struct work_struct i_iomap_ioend_work;
> >
> > Ugh, this adds 48 bytes to ext4 inode. That's pretty heavy. Cannot we reuse
> > i_rsv_conversion_list / work for this? For each inode only one of them
> > should be used AFAICS.
>
> Thanks for your suggestion. I think we should be able to reuse
> i_rsv_conversion_list / work. We can choose the corresponding
> initialization function for i_rsv_conversion_work based on the buffered
> write path at initialization time, and then reinitialize the work
> handler when changing the path via the ioctl that sets the journal
> flag. That should be sufficient.
Great, thanks.
> > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > > index 1ae7d3f4a1c8..a80195bd6f20 100644
> > > --- a/fs/ext4/inode.c
> > > +++ b/fs/ext4/inode.c
> > > @@ -44,6 +44,7 @@
> > > #include <linux/iversion.h>
> > > #include "ext4_jbd2.h"
> > > +#include "ext4_extents.h"
> > > #include "xattr.h"
> > > #include "acl.h"
> > > #include "truncate.h"
> > > @@ -4120,10 +4121,215 @@ static void ext4_iomap_readahead(struct readahead_control *rac)
> > > iomap_bio_readahead(rac, &ext4_iomap_buffered_read_ops);
> > > }
> > > +static int ext4_iomap_map_one_extent(struct inode *inode,
> > > + struct ext4_map_blocks *map)
> > > +{
> > > + struct extent_status es;
> > > + handle_t *handle = NULL;
> > > + int credits, map_flags;
> > > + int retval;
> > > +
> > > + credits = ext4_chunk_trans_blocks(inode, map->m_len);
> > > + handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, credits);
> > > + if (IS_ERR(handle))
> > > + return PTR_ERR(handle);
> > > +
> > > + map->m_flags = 0;
> > > + /*
> > > + * It is necessary to look up extent and map blocks under i_data_sem
> > > + * in write mode, otherwise, the delalloc extent may become stale
> > > + * during concurrent truncate operations.
> > > + */
> > > + ext4_fc_track_inode(handle, inode);
> > > + down_write(&EXT4_I(inode)->i_data_sem);
> > > + if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
> > > + retval = es.es_len - (map->m_lblk - es.es_lblk);
> > > + map->m_len = min_t(unsigned int, retval, map->m_len);
> > > +
> > > + if (ext4_es_is_delayed(&es)) {
> > > + map->m_flags |= EXT4_MAP_DELAYED;
> > > + trace_ext4_da_write_pages_extent(inode, map);
> > > + /*
> > > + * Call ext4_map_create_blocks() to allocate any
> > > + * delayed allocation blocks. It is possible that
> > > + * we're going to need more metadata blocks, however
> > > + * we must not fail because we're in writeback and
> > > + * there is nothing we can do so it might result in
> > > + * data loss. So use reserved blocks to allocate
> > > + * metadata if possible.
> > > + */
> > > + map_flags = EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT |
> > > + EXT4_GET_BLOCKS_METADATA_NOFAIL |
> > > + EXT4_EX_NOCACHE;
> > > +
> > > + retval = ext4_map_create_blocks(handle, inode, map,
> > > + map_flags);
> > > + if (retval > 0)
> > > + ext4_fc_track_range(handle, inode, map->m_lblk,
> > > + map->m_lblk + map->m_len - 1);
> > > + goto out;
> > > + } else if (unlikely(ext4_es_is_hole(&es)))
> > > + goto out;
> > > +
> > > + /* Found written or unwritten extent. */
> > > + map->m_pblk = ext4_es_pblock(&es) + map->m_lblk - es.es_lblk;
> > > + map->m_flags = ext4_es_is_written(&es) ?
> > > + EXT4_MAP_MAPPED : EXT4_MAP_UNWRITTEN;
> > > + goto out;
> > > + }
> > > +
> > > + retval = ext4_map_query_blocks(handle, inode, map, EXT4_EX_NOCACHE);
> > > +out:
> > > + up_write(&EXT4_I(inode)->i_data_sem);
> > > + ext4_journal_stop(handle);
> > > + return retval < 0 ? retval : 0;
> > > +}
> > > +
> > > +static int ext4_iomap_map_writeback_range(struct iomap_writepage_ctx *wpc,
> > > + loff_t offset, unsigned int dirty_len)
> > > +{
> > > + struct inode *inode = wpc->inode;
> > > + struct super_block *sb = inode->i_sb;
> > > + struct journal_s *journal = EXT4_SB(sb)->s_journal;
> > > + struct ext4_map_blocks map;
> > > + unsigned int blkbits = inode->i_blkbits;
> > > + unsigned int index = offset >> blkbits;
> > > + unsigned int blk_end, blk_len;
> > > + int ret;
> > > +
> > > + ret = ext4_emergency_state(sb);
> > > + if (unlikely(ret))
> > > + return ret;
> > > +
> > > + /* Check validity of the cached writeback mapping. */
> > > + if (offset >= wpc->iomap.offset &&
> > > + offset < wpc->iomap.offset + wpc->iomap.length &&
> > > + ext4_iomap_valid(inode, &wpc->iomap))
> > > + return 0;
> > > +
> > > + blk_len = dirty_len >> blkbits;
> > > + blk_end = min_t(unsigned int, (wpc->wbc->range_end >> blkbits),
> > > + (UINT_MAX - 1));
> > > + if (blk_end > index + blk_len)
> > > + blk_len = blk_end - index + 1;
> > > +
> > > +retry:
> > > + map.m_lblk = index;
> > > + map.m_len = min_t(unsigned int, MAX_WRITEPAGES_EXTENT_LEN, blk_len);
> > > + ret = ext4_map_blocks(NULL, inode, &map,
> > > + EXT4_GET_BLOCKS_IO_SUBMIT | EXT4_EX_NOCACHE);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /*
> > > + * The map is not a delalloc extent, it must either be a hole
> > > + * or an extent which have already been allocated.
> > > + */
> > > + if (!(map.m_flags & EXT4_MAP_DELAYED))
> > > + goto out;
> > > +
> > > + /* Map one delalloc extent. */
> > > + ret = ext4_iomap_map_one_extent(inode, &map);
> >
> > So it looks somewhat strange that here we call ext4_map_blocks() (which
> > consults extent status tree and then possibly on-disk extent tree) and then
> > we call ext4_iomap_map_one_extent() which manipulates with the extent
> > status tree and possibly extent tree as well. Is all this complexity to
> > avoid starting a jbd2 handle unless really needed? If yes, is that really
> > worth it? Given iomap code caches the extent we'd start the transaction
> > only once per mapped extent which shouldn't be that bad?
> >
> > If you have some benchmark showing this is really worth it,
>
> There are actually two reasons for this. First, we want to avoid
> starting a journal handle in overwrite scenarios. Second, we want to
> be able to query the extent locklessly without holding i_data_sem in
> overwrite cases as well (note that ext4_es_lookup_extent() in
> ext4_iomap_map_one_extent() is called with i_data_sem held).
>
> I ran a set of benchmark tests in my VM, performing the following FIO
> overwrite test on a 500GB ramdisk:
>
> $fio -filename=/test_dir/foo -direct=0 -iodepth=8 -fsync=0 -rw=write \
> -numjobs=1 -bs=4k -ioengine=io_uring -size=20G -uncached=1 \
> -runtime=30 --ramp_time=5s -time_based -norandommap=0 \
> -fallocate=none -overwrite=1 \
> -group_reportin -name=test --output=/tmp/log
>
> The results are as follows:
>
> a: on a non-fragmented file
> A: on a fragmented file [1]
> b: no background metadata pressure
> B: with background metadata pressure [2]
>
> buffer_head | iomap pre-map w/o journal | iomap directly map
> a+b: 680 691 690
> a+B: 560 568 567
> A+b: 637 633 579
> A+B: 540 571 495
>
> [1] The file is pre-fragmented such that each block occupies a separate
> extent.
> [2] A background fsstress process is running (only contains metadata
> ops):
> taskset -c 2 fsstress -c -d /test_dir -l 0 -n 1000 -f clonerange=0 \
> -f copyrange=0 -f awrite=0 -f aread=0 -f dread=0 \
> -f dwrite=0 -f mread=0 -f mwrite=0 -f readv=0 -f write=0 \
> -f writev=0 -f read=0 -f sync=0 -f afsync=0 -f fsync=0
>
> As can be seen, for large contiguous files, the performance impact is
> minimal. However, in heavily fragmented scenarios or under other
> metadata pressure, pre-querying the mapping brings noticeable gains.
> However, this is testing the most extreme case — I'm not sure about
> the real-world impact, so I don't have a strong preference either way.
> But I suppose faster is better, at least not slower than the old
> buffer_head path. :)
OK, thanks for the test! So for fragmented files the optimization of not
starting a transaction seems indeed worth it. I still dislike the
opencoding :) So given we have the reversed lock ordering now, why don't we
teach ext4_map_blocks() to start a transaction (if not provided) just
before it acquires i_data_sem for writing? This should be quite elegant. I
know you have some concerns about possible races below so let's discuss
that separately but at least in terms of performance and code complexity
this would look ideal :).
> > then I'd
> > probably prefer coming up with an ext4_get_blocks flag which tells it to
> > start a transaction on its own if we need to allocate blocks... That would
> > be much simpler than opencoding all this.
>
> Additionally, there is a key point here. The reason I open-coded
> ext4_iomap_map_writeback_range() is that we must ensure extent query
> and allocation are performed atomically under i_data_sem. Otherwise,
> concurrent truncate could lead to quota leaks.
>
> Specifically, consider the following scenario: we call
> ext4_map_blocks() to allocate blocks. Suppose there is a delalloc
> extent covering blocks [0,3). While writeback is submitting block 0, a
> concurrent truncate(block 1) occurs:
>
> wb truncate
> ext4_es_lookup_extent() ext4_truncate_down()
> //get [0,3)
> truncate_inode_pages_range()
> //clear page 1&2
> ext4_truncate()
> down_write(i_data_sem)
> ext4_es_remove_extent()
> //drop extent [1,3)
> //i_reserved_data_blocks: 3->1
> up_write(i_data_sem)
> down_write(i_data_sem)
> ext4_map_create_blocks()
> //alloc 3 blocks
> ext4_es_insert_extent()
> //only reclaim 1 block，stale 2 blocks
> up_write(i_data_sem)
>
> Therefore, If we don't open-coding this part, we would need to
> significantly rework ext4_map_blocks(), which might have a larger
> impact at this point. What do you think?
Hum, is something like this really possible? I mean iomap_writepages() will
lookup and lock folio. Only then it calls ->iomap_begin to map it to
underlying blocks. And folio lock synchronizes against
truncate_inode_pages_range() so how would writeback end up trying to
allocate something underlying pages 1 or 2?
> > > + if (ret < 0) {
> > > + if (ext4_emergency_state(sb))
> > > + return ret;
> > > +
> > > + /*
> > > + * Retry transient ENOSPC errors, if
> > > + * ext4_count_free_blocks() is non-zero, a commit
> > > + * should free up blocks.
> > > + */
> > > + if (ret == -ENOSPC && journal && ext4_count_free_clusters(sb)) {
> > > + jbd2_journal_force_commit_nested(journal);
> > > + goto retry;
> > > + }
> > > +
> > > + ext4_msg(sb, KERN_CRIT,
> > > + "Delayed block allocation failed for inode %llu at logical offset %llu with max blocks %u with error %d",
> > > + inode->i_ino, (unsigned long long)map.m_lblk,
> > > + (unsigned int)map.m_len, -ret);
> > > + ext4_msg(sb, KERN_CRIT,
> > > + "This should not happen!! Data will be lost\n");
> > > + if (ret == -ENOSPC)
> > > + ext4_print_free_blocks(inode);
> > > + return ret;
> > > + }
> > > +out:
> > > + ext4_set_iomap(inode, &wpc->iomap, &map, offset, dirty_len, 0);
> > > + return 0;
> > > +}
> > > +
> >
> > ...
> >
> > > +void ext4_iomap_end_bio(struct bio *bio)
> > > +{
> > > + struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
> > > + struct inode *inode = ioend->io_inode;
> > > + struct ext4_inode_info *ei = EXT4_I(inode);
> > > + struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> > > + unsigned long flags;
> > > +
> > > + /* Needs to convert unwritten extents or update the i_disksize. */
> > > + if ((ioend->io_flags & IOMAP_IOEND_UNWRITTEN) ||
> > > + ioend->io_offset + ioend->io_size > READ_ONCE(ei->i_disksize))
> > > + goto defer;
> > > +
> > > + /* Needs to abort the journal on data_err=abort. */
> > > + if (unlikely(ioend->io_bio.bi_status))
> > > + goto defer;
> > > +
> > > + iomap_finish_ioend(ioend, 0);
> > > + return;
> > > +defer:
> > > + spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> > > + if (list_empty(&ei->i_iomap_ioend_list))
> > > + queue_work(sbi->rsv_conversion_wq, &ei->i_iomap_ioend_work);
> > > + list_add_tail(&ioend->io_list, &ei->i_iomap_ioend_list);
> > > + spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
> > > +}
> >
> > For now, I'd prefer to do what XFS does and offload everything. Then you
> > don't have to export iomap_finish_ioend() (which would need to be in a
> > separate patch and acked by iomap maintainers) and the code is more
> > standard. There's a patchset in the works which adds general ioend offloading
> > infrastructure into iomap [1] and when that lands we should get all these
> ^^^^^ block layer?
>
> > bells and whistles (even better ones with percpu work queues, batching,
> > etc.) for free.
> >
> > [1] https://lore.kernel.org/all/20260514-blk-dontcache-v6-0-782e2fa7477b@columbia.edu/
> >
> > Honza
>
> Ha, I've noticed this patchset, so I haven't implemented
> uncached I/O handling for now. As a side note, I have a question:
> if we convert all endio processing to worker threads, IIRC, my
> recollection from previous performance tests is that pure overwrite
> scenarios would see at least a 20% degradation. Is that acceptable?
No, but the latest version of the patches exactly does IO completion in the
interrupt unless the bio is flagged as needing IO completion from process
context or unless end_io handler returns a particular error - which means
IO completion wasn't actually done and needs offloading into process
context instead.
> I understand why uncached I/O might need the entire completion path
> in a worker, but can we complete the I/O in interrupt context for
> pure overwrite and then release the page cache in a worker? Must
> page cache invalidation and I/O completion be synchronous?
Strictly speaking no, we can first complete the IO and evict page cache
independently later. But it would be quite tricky locking wise (folios and
the mapping containing them can get evicted once folio writeback bit gets
cleared) so the whole uncached writes handling would have to be reworked. I
don't think it's worth it at this point.
> The reason I kept ext4_iomap_end_bio() handling I/O completion in
> interrupt context is for overwrite performance. XFS also handles
> overwrites in interrupt context (via ioend_writeback_end_bio()).
> However, ext4 has the data_error=abort mount option — when this mode
> is set and an I/O error occurs, we must abort the journal in a
> worker. Since we cannot predict I/O errors at submission time, we
> can't directly use ioend_writeback_end_bio() and must instead bind
> our own ext4_iomap_end_bio(). At the same time, I want to avoid
> spawning a worker for pure overwrites when no I/O error occurs, so I
> exported iomap_finish_ioend(). What do you think?
So data_error=abort handling can exactly use the new generic framework - if
we detect during processing IO completion we cannot actually do it in the
interrupt (like in case of error), we just return appropriately from the
handler and the generic code will handle offloading and call the ->end_io
callback again.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox