[PATCH v7 0/4] ext4: fix xattr iput deadlock with s_writepages_rwsem

[PATCH v7 0/4] ext4: fix xattr iput deadlock with s_writepages_rwsem

[Yun Zhou]

This series fixes a circular lock dependency reported by syzbot:

  s_writepages_rwsem --> jbd2_handle --> xattr_sem --> s_writepages_rwsem

The deadlock occurs when iput() on an EA inode triggers write_inode_now()
while xattr_sem and a jbd2 handle are held.  The triggering path is
during mount-time orphan cleanup (!SB_ACTIVE) where iput_final() calls
write_inode_now() synchronously.

Patch 1 blocks the deadlock by skipping extra isize expansion when
!SB_ACTIVE -- this prevents the xattr manipulation path from being
entered during mount.

Patch 2 is a belt-and-suspenders semantic improvement: an inode under
eviction never needs extra isize expansion.

Patches 3-4 are a structural improvement using a per-sb workqueue:

  Patch 3 introduces ext4_put_ea_inode(), which does direct iput() when
  SB_ACTIVE (zero overhead) and defers to a workqueue when !SB_ACTIVE.
  It also converts the first call site (ext4_xattr_block_set release
  path) which previously called iput under xattr_sem + jbd2 handle.

  Patch 4 converts the remaining EA inode iput() calls that execute
  under locks.  Sites where direct iput() is provably safe (i_nlink=0
  after dec_ref, or lookup-only paths) are left unchanged with comments.

Link: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5

v7:
 - Replaced the deferred-iput array threading approach (v4-v6) with a
   simpler per-sb workqueue + lock-free llist design.  No function
   signature changes needed.  ext4_put_ea_inode() does direct iput when
   SB_ACTIVE (zero overhead in normal operation) and defers to the
   workqueue only during mount (!SB_ACTIVE).
 - Converted the iput in ext4_xattr_delete_inode()'s quota accounting
   loop to ext4_put_ea_inode() to eliminate a lockdep-reportable lock
   ordering violation (jbd2_handle -> iput -> s_writepages_rwsem).
 - Moved flush_work() before the if (sbi->s_journal) check in
   ext4_put_super() to cover nojournal mode.

v6:
 - ext4_inline_data_truncate(): use local ea_inode_array instead of
   passing NULL, freed after ext4_journal_stop().  Fixes a deadlock
   reachable via crafted filesystem where inline data xattr entry has
   e_value_inum set: orphan cleanup -> ext4_truncate ->
   ext4_inline_data_truncate -> iput under !SB_ACTIVE.

v5:
 - Split into 3 patches for easier review.
 - Add explicit !SB_ACTIVE early-return in ext4_try_to_expand_extra_isize()
   to block ALL mount-time paths (ext4_process_orphan -> ext4_truncate ->
   ext4_mark_inode_dirty), not just the eviction path. v4 only relied on
   EXT4_STATE_NO_EXPAND which doesn't cover orphan truncation.

v4:
 - Comprehensive rewrite of the deferred iput mechanism.
 - Thread ea_inode_array through ext4_expand_extra_isize_ea() and
   ext4_xattr_move_to_block() so ALL ea_inode iputs in the expand
   path are deferred, not just those in ext4_xattr_block_set().
 - Add NULL safety to ext4_expand_inode_array(): when ea_inode_array
   pointer is NULL, fall back to synchronous iput (for callers like
   ext4_initxattrs that only run with SB_ACTIVE).
 - Use __GFP_NOFAIL to guarantee deferred array growth, eliminating
   fallback to synchronous iput under locks.
 - Update ext4_xattr_ibody_set() and ext4_xattr_set_entry() signatures
   to accept ea_inode_array, converting ALL iput(ea_inode) calls.
 - Set EXT4_STATE_NO_EXPAND in ext4_evict_inode() before
   ext4_mark_inode_dirty().

v3:
 - Check ext4_expand_inode_array() return value; fallback to
   direct iput() on ENOMEM to prevent inode leak.
 - Make ext4_xattr_set_handle() take an optional ea_inode_array
   output parameter so callers can free after ext4_journal_stop(),
   avoiding the jbd2_handle vs s_writepages_rwsem AB-BA.
 - Pass ea_inode_array directly to ext4_xattr_release_block()
   instead of using a local array freed under xattr_sem.
 - Move ext4_xattr_inode_array_free() after ext4_journal_stop()

v2:
 - Defer iput() in ext4_xattr_block_set() via ea_inode_array,
   freed after xattr_sem is released. Fixes the root cause.

v1:
 - Set EXT4_STATE_NO_EXPAND in ext4_evict_inode() to skip expand
   on inodes being deleted. Only fixes the syzbot reproducer, not
   the underlying lock ordering violation.

Yun Zhou (4):
  ext4: skip extra isize expansion during mount to prevent deadlock
  ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode
  ext4: introduce ext4_put_ea_inode() for safe deferred iput
  ext4: convert remaining EA inode iput() calls to ext4_put_ea_inode()

 fs/ext4/ext4.h  |   5 +++
 fs/ext4/inode.c |  11 +++++
 fs/ext4/super.c |   6 +++
 fs/ext4/xattr.c | 105 +++++++++++++++++++++++++++++++++++++++++++-----
 fs/ext4/xattr.h |   2 +
 5 files changed, 120 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH v7 1/4] ext4: skip extra isize expansion during mount to prevent deadlock

[Yun Zhou]

ext4_try_to_expand_extra_isize() is called from __ext4_mark_inode_dirty()
while holding an active jbd2 handle.  During mount (!SB_ACTIVE), the
expand path may move xattrs to external blocks and release ea_inodes via
iput().  When !SB_ACTIVE, iput() calls write_inode_now() which acquires
s_writepages_rwsem, creating a circular lock dependency:

  s_writepages_rwsem --> jbd2_handle --> xattr_sem --> s_writepages_rwsem

This can be triggered via:

  ext4_process_orphan() -> ext4_truncate() -> ext4_mark_inode_dirty()
    -> ext4_try_to_expand_extra_isize()

or:

  ext4_evict_inode() -> ext4_mark_inode_dirty()
    -> ext4_try_to_expand_extra_isize()

Skip expansion when !SB_ACTIVE.  This is a minor loss of functionality
(extra isize won't grow for these inodes during mount), which e2fsck
can resolve later if needed.

Reported-by: syzbot+5d19358d7eb30ffb0cc5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5
Fixes: c8585c6fcaf2 ("ext4: fix races between changing inode journal mode and ext4_writepages")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/ext4/inode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c2c2d6ac7f3d..09dcfb6bf48c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -6458,6 +6458,16 @@ static int ext4_try_to_expand_extra_isize(struct inode *inode,
 	if (ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND))
 		return -EOVERFLOW;
 
+	/*
+	 * Skip expansion during mount (!SB_ACTIVE).  Expanding extra isize
+	 * may move xattrs to external blocks and release ea_inodes via iput.
+	 * When !SB_ACTIVE, iput triggers write_inode_now() which acquires
+	 * s_writepages_rwsem, causing a deadlock with the caller's active
+	 * jbd2 handle (lock order: s_writepages_rwsem -> jbd2_handle).
+	 */
+	if (unlikely(!(inode->i_sb->s_flags & SB_ACTIVE)))
+		return -EBUSY;
+
 	/*
 	 * In nojournal mode, we can immediately attempt to expand
 	 * the inode.  When journaled, we first need to obtain extra
-- 
2.43.0

[PATCH v7 2/4] ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode

[Yun Zhou]

An inode being evicted will never need its extra isize expanded.  Set
EXT4_STATE_NO_EXPAND before ext4_mark_inode_dirty() in ext4_evict_inode()
to make this explicit and prevent any unnecessary work in
ext4_try_to_expand_extra_isize().

This also provides defense-in-depth for the s_writepages_rwsem deadlock
during mount-time orphan cleanup, ensuring the expand path is blocked
for inodes under eviction regardless of how they are reached.

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/ext4/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 09dcfb6bf48c..1de0aaa28e63 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -264,6 +264,7 @@ void ext4_evict_inode(struct inode *inode)
 	if (ext4_inode_is_fast_symlink(inode))
 		memset(EXT4_I(inode)->i_data, 0, sizeof(EXT4_I(inode)->i_data));
 	inode->i_size = 0;
+	ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
 	err = ext4_mark_inode_dirty(handle, inode);
 	if (err) {
 		ext4_warning(inode->i_sb,
-- 
2.43.0

[PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Yun Zhou]

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 safely releases EA inode references:
when SB_ACTIVE, it calls iput() directly (write_inode_now cannot be
triggered); during mount (!SB_ACTIVE), it queues the inode on a per-sb
lock-free llist and schedules a worker to call iput() in a clean
context without holding any ext4 locks.

Convert the iput in ext4_xattr_block_set()'s "Drop the previous xattr
block" path to use ext4_xattr_inode_array_free_deferred(), which
releases EA inodes via ext4_put_ea_inode().  This path previously called
ext4_xattr_inode_array_free() (synchronous iput) while holding xattr_sem
and a jbd2 handle.

The worker is flushed in ext4_put_super() before journal destruction to
ensure all pending EA inode cleanup completes while the journal is still
available.

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/ext4/ext4.h  |  5 ++++
 fs/ext4/super.c |  6 ++++
 fs/ext4/xattr.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/ext4/xattr.h |  2 ++
 4 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 94283a991e5c..690202303269 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1706,6 +1706,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 work_struct 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 6a77db4d3124..b777bb0a81ea 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1308,6 +1308,9 @@ static void ext4_put_super(struct super_block *sb)
 	destroy_workqueue(sbi->rsv_conversion_wq);
 	ext4_release_orphan_info(sb);
 
+	/* Flush deferred EA inode iputs before destroying journal */
+	flush_work(&sbi->s_ea_inode_work);
+
 	if (sbi->s_journal) {
 		aborted = is_journal_aborted(sbi->s_journal);
 		err = ext4_journal_destroy(sbi, sbi->s_journal);
@@ -5535,6 +5538,9 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		needs_recovery = 0;
 	}
 
+	init_llist_head(&sbi->s_ea_inode_to_free);
+	INIT_WORK(&sbi->s_ea_inode_work, ext4_ea_inode_work);
+
 	if (!test_opt(sb, NO_MBCACHE)) {
 		sbi->s_ea_block_cache = ext4_xattr_create_cache();
 		if (!sbi->s_ea_block_cache) {
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 982a1f831e22..04e7f674340d 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);
+}
+
+struct ext4_ea_iput_entry {
+	struct llist_node node;
+	struct inode *inode;
+};
+
+/*
+ * 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.
+ */
+void ext4_ea_inode_work(struct work_struct *work)
+{
+	struct ext4_sb_info *sbi = container_of(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_ea_iput_entry *entry = container_of(node,
+				struct ext4_ea_iput_entry, node);
+		next = node->next;
+		iput(entry->inode);
+		kfree(entry);
+		node = next;
+	}
+}
+
+/*
+ * Release a VFS reference on an EA inode after ext4_xattr_inode_dec_ref()
+ * may have set i_nlink=0.  Must be used instead of iput() in any context
+ * where xattr_sem or a jbd2 handle is held, because eviction of a nlink=0
+ * inode can acquire those same locks.
+ *
+ * When SB_ACTIVE, eviction does not call write_inode_now() so direct
+ * iput() is safe.  During mount (!SB_ACTIVE), defer to a workqueue.
+ *
+ * For EA inode references dropped without a preceding dec_ref (e.g.,
+ * lookup-only paths where nlink remains >= 1), plain iput() is safe
+ * and preferred.
+ */
+void ext4_put_ea_inode(struct super_block *sb, struct inode *inode)
+{
+	struct ext4_ea_iput_entry *entry;
+
+	if (!inode)
+		return;
+	if (sb->s_flags & SB_ACTIVE) {
+		iput(inode);
+		return;
+	}
+	entry = kmalloc(sizeof(*entry), GFP_NOFS | __GFP_NOFAIL);
+	entry->inode = inode;
+	llist_add(&entry->node, &EXT4_SB(sb)->s_ea_inode_to_free);
+	schedule_work(&EXT4_SB(sb)->s_ea_inode_work);
+}
+
 /*
  * ext4_xattr_block_cache_insert()
  *
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index 1fedf44d4fb6..52074537dce5 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -190,6 +190,8 @@ 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_ea_inode_work(struct work_struct *work);
+extern void ext4_put_ea_inode(struct super_block *sb, struct inode *inode);
 
 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

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Zhou, Yun]

Hi Honza,
> Add ext4_put_ea_inode() which safely releases EA inode references:
> when SB_ACTIVE, it calls iput() directly (write_inode_now cannot be
> triggered); during mount (!SB_ACTIVE), it queues the inode on a per-sb
> lock-free llist and schedules a worker to call iput() in a clean
> context without holding any ext4 locks.
>
> Convert the iput in ext4_xattr_block_set()'s "Drop the previous xattr
> block" path to use ext4_xattr_inode_array_free_deferred(), which
> releases EA inodes via ext4_put_ea_inode().  This path previously called
> ext4_xattr_inode_array_free() (synchronous iput) while holding xattr_sem
> and a jbd2 handle.
>
> The worker is flushed in ext4_put_super() before journal destruction to
> ensure all pending EA inode cleanup completes while the journal is still
> available.
>
>   
> +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);
> +}
> +
> +struct ext4_ea_iput_entry {
> +	struct llist_node node;
> +	struct inode *inode;
> +};
> +
> +/*
> + * 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.
> + */
> +void ext4_ea_inode_work(struct work_struct *work)
> +{
> +	struct ext4_sb_info *sbi = container_of(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_ea_iput_entry *entry = container_of(node,
> +				struct ext4_ea_iput_entry, node);
> +		next = node->next;
> +		iput(entry->inode);
> +		kfree(entry);
> +		node = next;
> +	}
> +}
> +
> +/*
> + * Release a VFS reference on an EA inode after ext4_xattr_inode_dec_ref()
> + * may have set i_nlink=0.  Must be used instead of iput() in any context
> + * where xattr_sem or a jbd2 handle is held, because eviction of a nlink=0
> + * inode can acquire those same locks.
> + *
> + * When SB_ACTIVE, eviction does not call write_inode_now() so direct
> + * iput() is safe.  During mount (!SB_ACTIVE), defer to a workqueue.
> + *
> + * For EA inode references dropped without a preceding dec_ref (e.g.,
> + * lookup-only paths where nlink remains >= 1), plain iput() is safe
> + * and preferred.
> + */
> +void ext4_put_ea_inode(struct super_block *sb, struct inode *inode)
> +{
> +	struct ext4_ea_iput_entry *entry;
> +
> +	if (!inode)
> +		return;
> +	if (sb->s_flags & SB_ACTIVE) {
> +		iput(inode);
> +		return;
> +	}
> +	entry = kmalloc(sizeof(*entry), GFP_NOFS | __GFP_NOFAIL);
> +	entry->inode = inode;
> +	llist_add(&entry->node, &EXT4_SB(sb)->s_ea_inode_to_free);
> +	schedule_work(&EXT4_SB(sb)->s_ea_inode_work);
> +}
> +
>
Could you help me check if this is the way you expected?

Thanks,
Yun

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Jan Kara]

On Tue 16-06-26 23:15:57, Yun Zhou wrote:
> 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 safely releases EA inode references:
> when SB_ACTIVE, it calls iput() directly (write_inode_now cannot be
> triggered); during mount (!SB_ACTIVE), it queues the inode on a per-sb
> lock-free llist and schedules a worker to call iput() in a clean
> context without holding any ext4 locks.
> 
> Convert the iput in ext4_xattr_block_set()'s "Drop the previous xattr
> block" path to use ext4_xattr_inode_array_free_deferred(), which
> releases EA inodes via ext4_put_ea_inode().  This path previously called
> ext4_xattr_inode_array_free() (synchronous iput) while holding xattr_sem
> and a jbd2 handle.
> 
> The worker is flushed in ext4_put_super() before journal destruction to
> ensure all pending EA inode cleanup completes while the journal is still
> available.
> 
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>

Yes, this goes in the direction I intended. But I have couple of
suggestions:

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 94283a991e5c..690202303269 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1706,6 +1706,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 work_struct s_ea_inode_work;
> +

I'd probably use delayed work and schedule it with a delay of one jiffie so
that some inodes can accumulate before we process them which should reduce
the amount of task switching to workqueues.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6a77db4d3124..b777bb0a81ea 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1308,6 +1308,9 @@ static void ext4_put_super(struct super_block *sb)
>  	destroy_workqueue(sbi->rsv_conversion_wq);
>  	ext4_release_orphan_info(sb);
>  
> +	/* Flush deferred EA inode iputs before destroying journal */
> +	flush_work(&sbi->s_ea_inode_work);
> +

This should happen earlier in ext4_put_super(). At this place quotas were
already turned off and so quota accounting would go wrong.

> @@ -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);
> +}

The array of EA inodes used in xattr handling is just another mechanism
used for delaying iput() of EA inodes. It doesn't make sense to stack these
to one on top of another. Just completely replace the array mechanism with
always deferring iput of EA inode into the workqueue.

> +struct ext4_ea_iput_entry {
> +	struct llist_node node;
> +	struct inode *inode;
> +};
> +
> +/*
> + * 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.
> + */
> +void ext4_ea_inode_work(struct work_struct *work)
> +{
> +	struct ext4_sb_info *sbi = container_of(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_ea_iput_entry *entry = container_of(node,
> +				struct ext4_ea_iput_entry, node);
> +		next = node->next;
> +		iput(entry->inode);
> +		kfree(entry);
> +		node = next;
> +	}
> +}

Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
I want to suggest another scheme (somewhat more involved but more efficient
scheme):

1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
drops inode reference if it is not the last one (and returns true in that
case). Basically:

bool iput_if_not_last(struct inode *inode)
{
	return atomic_add_unless(&inode->i_count, -1, 1);
}

This needs to be a separate patch as it should get vetting from VFS
maintainers.

2) Use iput_if_not_last() in ext4_put_ea_inode(). If it returns true, we
are done. Otherwise we know we were at least for a moment holders of the
last inode reference, so we link the inode to the list of inodes to drop
through llist_node embedded in ext4_inode_info. We cannot race with anybody
else trying to link the same inode into the list because we hold one inode
ref and so nobody else can hit this "I was holding the last ref" path.
I'd union this llist_node say with xattr_sem which is unused for EA inodes
to avoid growing ext4_inode_info.

This way we avoid offloading unless really necessary and we don't have to
do allocations just to drop EA inode ref.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Zhou, Yun]

On 6/18/2026 2:42 AM, Jan Kara wrote:
> On Tue 16-06-26 23:15:57, Yun Zhou wrote:
>> +
>> +     /* Deferred iput for EA inodes to avoid lock ordering issues */
>> +     struct llist_head s_ea_inode_to_free;
>> +     struct work_struct s_ea_inode_work;
>> +
> 
> I'd probably use delayed work and schedule it with a delay of one jiffie so
> that some inodes can accumulate before we process them which should reduce
> the amount of task switching to workqueues.
> 
Good idea, I will use delayed_work in next version.

>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 6a77db4d3124..b777bb0a81ea 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1308,6 +1308,9 @@ static void ext4_put_super(struct super_block *sb)
>>        destroy_workqueue(sbi->rsv_conversion_wq);
>>        ext4_release_orphan_info(sb);
>>
>> +     /* Flush deferred EA inode iputs before destroying journal */
>> +     flush_work(&sbi->s_ea_inode_work);
>> +
> 
> This should happen earlier in ext4_put_super(). At this place quotas were
> already turned off and so quota accounting would go wrong.
That makes sense. I'll move it up to right before ext4_quotas_off().

>> +static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
>> +                             struct ext4_xattr_inode_array *array)
> 
> The array of EA inodes used in xattr handling is just another mechanism
> used for delaying iput() of EA inodes. It doesn't make sense to stack these
> to one on top of another. Just completely replace the array mechanism with
> always deferring iput of EA inode into the workqueue.
> 
I'm thinking that a complete replacement might be too large a change. 
Should we consider postponing this work, or perhaps appending a new 
patch to this series to handle it?

> 
> Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> I want to suggest another scheme (somewhat more involved but more efficient
> scheme):
> 
> 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> drops inode reference if it is not the last one (and returns true in that
> case). Basically:
> 
> bool iput_if_not_last(struct inode *inode)
> {
>          return atomic_add_unless(&inode->i_count, -1, 1);
> }
> 
> This needs to be a separate patch as it should get vetting from VFS
> maintainers.
> 
> 2) Use iput_if_not_last() in ext4_put_ea_inode(). If it returns true, we
> are done. Otherwise we know we were at least for a moment holders of the
> last inode reference, so we link the inode to the list of inodes to drop
> through llist_node embedded in ext4_inode_info. We cannot race with anybody
> else trying to link the same inode into the list because we hold one inode
> ref and so nobody else can hit this "I was holding the last ref" path.
> I'd union this llist_node say with xattr_sem which is unused for EA inodes
> to avoid growing ext4_inode_info.
> 
> This way we avoid offloading unless really necessary and we don't have to
> do allocations just to drop EA inode ref.
> 
Your idea makes a lot of sense. It greatly simplifies the current deferred
iput logic and eliminates the risk of failing to allocate an entry during
an OOM. However, as you mentioned, getting the VFS maintainers to agree 
might be quite challenging.

BR,
Yun

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Jan Kara]

On Fri 19-06-26 14:24:51, Zhou, Yun wrote:
> On 6/18/2026 2:42 AM, Jan Kara wrote:
> > > +static void ext4_xattr_inode_array_free_deferred(struct super_block *sb,
> > > +                             struct ext4_xattr_inode_array *array)
> > 
> > The array of EA inodes used in xattr handling is just another mechanism
> > used for delaying iput() of EA inodes. It doesn't make sense to stack these
> > to one on top of another. Just completely replace the array mechanism with
> > always deferring iput of EA inode into the workqueue.
> > 
> I'm thinking that a complete replacement might be too large a change. Should
> we consider postponing this work, or perhaps appending a new patch to this
> series to handle it?

Do one patch to implement the new framework for delayed EA iputs. Then
maybe one patch to convert existing iputs() of EA inodes to the new
delaying framework and then one patch to convert users of the current
'array to release' mechanism to the new delaying framework.

> > Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> > I want to suggest another scheme (somewhat more involved but more efficient
> > scheme):
> > 
> > 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> > drops inode reference if it is not the last one (and returns true in that
> > case). Basically:
> > 
> > bool iput_if_not_last(struct inode *inode)
> > {
> >          return atomic_add_unless(&inode->i_count, -1, 1);
> > }
> > 
> > This needs to be a separate patch as it should get vetting from VFS
> > maintainers.
> > 
> > 2) Use iput_if_not_last() in ext4_put_ea_inode(). If it returns true, we
> > are done. Otherwise we know we were at least for a moment holders of the
> > last inode reference, so we link the inode to the list of inodes to drop
> > through llist_node embedded in ext4_inode_info. We cannot race with anybody
> > else trying to link the same inode into the list because we hold one inode
> > ref and so nobody else can hit this "I was holding the last ref" path.
> > I'd union this llist_node say with xattr_sem which is unused for EA inodes
> > to avoid growing ext4_inode_info.
> > 
> > This way we avoid offloading unless really necessary and we don't have to
> > do allocations just to drop EA inode ref.
> 
> Your idea makes a lot of sense. It greatly simplifies the current deferred
> iput logic and eliminates the risk of failing to allocate an entry during
> an OOM. However, as you mentioned, getting the VFS maintainers to agree
> might be quite challenging.

VFS maintainers don't bite, in the worst case they'd disagree :) For a fact
I'm one of VFS reviewers and I think iput_if_not_last() idea is an
acceptable one. So I think it's worth trying.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Zhou, Yun]

On 6/22/26 16:32, Jan Kara wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Fri 19-06-26 14:24:51, Zhou, Yun wrote:
>> Your idea makes a lot of sense. It greatly simplifies the current deferred
>> iput logic and eliminates the risk of failing to allocate an entry during
>> an OOM. However, as you mentioned, getting the VFS maintainers to agree
>> might be quite challenging.
> VFS maintainers don't bite, in the worst case they'd disagree :) For a fact
> I'm one of VFS reviewers and I think iput_if_not_last() idea is an
> acceptable one. So I think it's worth trying.
Thanks a lot for the strong support, I'd be happy to give it a try. For 
now, please
just ignore the v8 patch series, as the AI reviewer actually found some 
issues with
it as well.

BR,
Yun

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Zhou, Yun]

Hi Honza,

On 6/18/26 02:42, Jan Kara wrote:
> 
> Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> I want to suggest another scheme (somewhat more involved but more efficient
> scheme):
> 
> 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> drops inode reference if it is not the last one (and returns true in that
> case). Basically:
> 
> bool iput_if_not_last(struct inode *inode)
> {
>          return atomic_add_unless(&inode->i_count, -1, 1);
> }
> 
> This needs to be a separate patch as it should get vetting from VFS
> maintainers.
After taking a closer look, it seems that this function doesn't need to 
be added to the VFS layer — at least not for now. For example, we could 
directly inline atomic_add_unless() into ext4_put_ea_inode():

void ext4_put_ea_inode(struct super_block *sb, struct inode *inode)
{
     if (!inode)
         return;
     if (atomic_add_unless(&inode->i_count, -1, 1))
         return;
     llist_add(&EXT4_I(inode)->i_ea_iput_node,
         &EXT4_SB(sb)->s_ea_inode_to_free);
     schedule_delayed_work(&EXT4_SB(sb)->s_ea_inode_work, 1);
}

This way, we avoid submitting an isolated patch to the VFS layer that 
currently has only one user (ext4). If there is indeed a real need for 
it in the future, we can always submit a follow-up patch to refactor it.

What do you think?

BR,
Yun

Re: [PATCH v7 3/4] ext4: introduce ext4_put_ea_inode() for safe deferred iput

[Jan Kara]

On Mon 22-06-26 18:06:23, Zhou, Yun wrote:
> Hi Honza,
> 
> On 6/18/26 02:42, Jan Kara wrote:
> > 
> > Allocating ext4_ea_iput_entry for dropping each inode is somewhat wasteful.
> > I want to suggest another scheme (somewhat more involved but more efficient
> > scheme):
> > 
> > 1) Create a VFS helper bool iput_if_not_last(struct inode *inode) which
> > drops inode reference if it is not the last one (and returns true in that
> > case). Basically:
> > 
> > bool iput_if_not_last(struct inode *inode)
> > {
> >          return atomic_add_unless(&inode->i_count, -1, 1);
> > }
> > 
> > This needs to be a separate patch as it should get vetting from VFS
> > maintainers.
> After taking a closer look, it seems that this function doesn't need to be
> added to the VFS layer — at least not for now. For example, we could
> directly inline atomic_add_unless() into ext4_put_ea_inode():
> 
> void ext4_put_ea_inode(struct super_block *sb, struct inode *inode)
> {
>     if (!inode)
>         return;
>     if (atomic_add_unless(&inode->i_count, -1, 1))
>         return;
>     llist_add(&EXT4_I(inode)->i_ea_iput_node,
>         &EXT4_SB(sb)->s_ea_inode_to_free);
>     schedule_delayed_work(&EXT4_SB(sb)->s_ea_inode_work, 1);
> }
> 
> This way, we avoid submitting an isolated patch to the VFS layer that
> currently has only one user (ext4). If there is indeed a real need for it in
> the future, we can always submit a follow-up patch to refactor it.
> 
> What do you think?

No, please provide a proper helper in VFS for this. We don't really want
filesystems to play games with inode refcount without proper abstraction in
VFS. That causes longterm maintenance issues.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v7 4/4] ext4: convert remaining EA inode iput() calls to ext4_put_ea_inode()

[Yun Zhou]

Convert all remaining iput() calls on EA inodes that execute under
xattr_sem or a jbd2 handle to use ext4_put_ea_inode().  With i_nlink>=1
and !SB_ACTIVE, a direct iput() would trigger write_inode_now() ->
s_writepages_rwsem, creating a lock ordering violation with the caller's
active jbd2 handle.

Converted sites and why defer is necessary:

- ext4_xattr_inode_inc_ref_all() cleanup: dec_ref undoes the failed
  inc_ref, but the EA inode may be shared so i_nlink remains 1.

- ext4_xattr_inode_lookup_create() out_err: may be a cache-found inode
  where inc_ref failed; i_nlink remains 1.

- ext4_xattr_set_entry() old_ea_inode: dec_ref was called but the EA
  inode may be shared by other xattr blocks, so i_nlink remains 1.

- ext4_xattr_block_set() new block path: dec_ref drops the "extra" ref
  but inc_ref_all added another, so i_nlink stays 1.

- ext4_xattr_block_set() cleanup: on success no dec_ref was called
  (i_nlink=1); on error dec_ref may leave i_nlink=1 if shared.

- ext4_xattr_ibody_set() error path: dec_ref on a cache-found EA inode
  may leave i_nlink=1 if shared.

- ext4_xattr_ibody_set() success path: newly stored EA inode with
  i_nlink=1, just releasing the lookup reference.

- ext4_xattr_delete_inode() quota loop: iget for quota accounting only,
  no dec_ref called, i_nlink=1, jbd2 handle is active.

Sites where direct iput() is provably safe are left unchanged with a
comment: ext4_xattr_inode_create() error path (dec_ref guarantees
i_nlink=0, so eviction skips write_inode_now).

Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/ext4/xattr.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 04e7f674340d..b8a2ccd0a958 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1079,6 +1079,13 @@ static int ext4_xattr_inode_inc_ref(handle_t *handle, struct inode *ea_inode)
 	return ext4_xattr_inode_update_ref(handle, ea_inode, 1);
 }
 
+/*
+ * Decrement on-disk reference count of an EA inode.  If refcount reaches 0,
+ * i_nlink is cleared and the inode is added to the orphan list.  Callers
+ * must use ext4_put_ea_inode() (not iput) to release the VFS reference
+ * afterwards, since iput on a nlink=0 inode triggers eviction which may
+ * deadlock if called under xattr_sem or an active jbd2 handle.
+ */
 static int ext4_xattr_inode_dec_ref(handle_t *handle, struct inode *ea_inode)
 {
 	return ext4_xattr_inode_update_ref(handle, ea_inode, -1);
@@ -1135,7 +1142,8 @@ 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);
+		/* i_nlink may remain 1 if shared; defer for !SB_ACTIVE safety */
+		ext4_put_ea_inode(parent->i_sb, ea_inode);
 	}
 	return saved_err;
 }
@@ -1507,6 +1515,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);
+			/* dec_ref set i_nlink=0; iput won't trigger write_inode_now */
 			iput(ea_inode);
 			return ERR_PTR(err);
 		}
@@ -1617,7 +1626,8 @@ 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);
+	/* May be cache-found inode with i_nlink=1 (inc_ref failed) */
+	ext4_put_ea_inode(inode->i_sb, ea_inode);
 	ext4_xattr_inode_free_quota(inode, NULL, value_len);
 	return ERR_PTR(err);
 }
@@ -1850,7 +1860,8 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 
 	ret = 0;
 out:
-	iput(old_ea_inode);
+	/* old_ea_inode had dec_ref; may still have i_nlink=1 if shared */
+	ext4_put_ea_inode(inode->i_sb, old_ea_inode);
 	return ret;
 }
 
@@ -2152,7 +2163,8 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode,
 					ext4_warning_inode(ea_inode,
 							   "dec ref error=%d",
 							   error);
-				iput(ea_inode);
+				/* i_nlink stays 1 (inc_ref_all added a ref) */
+				ext4_put_ea_inode(inode->i_sb, ea_inode);
 				ea_inode = NULL;
 			}
 
@@ -2206,7 +2218,8 @@ 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);
+		/* success: i_nlink=1; error+dec_ref: may still be 1 if shared */
+		ext4_put_ea_inode(inode->i_sb, ea_inode);
 	}
 	if (ce)
 		mb_cache_entry_put(ea_block_cache, ce);
@@ -2288,7 +2301,8 @@ 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);
+			/* cache-found ea_inode may retain i_nlink=1 */
+			ext4_put_ea_inode(inode->i_sb, ea_inode);
 		}
 		return error;
 	}
@@ -2300,7 +2314,8 @@ 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);
+	/* ea_inode has i_nlink=1 (new ref just stored in xattr entry) */
+	ext4_put_ea_inode(inode->i_sb, ea_inode);
 	return 0;
 }
 
@@ -2989,7 +3004,8 @@ 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);
+				/* no dec_ref yet but i_nlink=1; handle is active */
+				ext4_put_ea_inode(inode->i_sb, ea_inode);
 			}
 
 		}
-- 
2.43.0

Re: [PATCH v7 0/4] ext4: fix xattr iput deadlock with s_writepages_rwsem

[Jan Kara]

On Tue 16-06-26 23:15:54, Yun Zhou wrote:
> This series fixes a circular lock dependency reported by syzbot:
> 
>   s_writepages_rwsem --> jbd2_handle --> xattr_sem --> s_writepages_rwsem
> 
> The deadlock occurs when iput() on an EA inode triggers write_inode_now()
> while xattr_sem and a jbd2 handle are held.  The triggering path is
> during mount-time orphan cleanup (!SB_ACTIVE) where iput_final() calls
> write_inode_now() synchronously.
> 
> Patch 1 blocks the deadlock by skipping extra isize expansion when
> !SB_ACTIVE -- this prevents the xattr manipulation path from being
> entered during mount.
> 
> Patch 2 is a belt-and-suspenders semantic improvement: an inode under
> eviction never needs extra isize expansion.
> 
> Patches 3-4 are a structural improvement using a per-sb workqueue:
> 
>   Patch 3 introduces ext4_put_ea_inode(), which does direct iput() when
>   SB_ACTIVE (zero overhead) and defers to a workqueue when !SB_ACTIVE.
>   It also converts the first call site (ext4_xattr_block_set release
>   path) which previously called iput under xattr_sem + jbd2 handle.
> 
>   Patch 4 converts the remaining EA inode iput() calls that execute
>   under locks.  Sites where direct iput() is provably safe (i_nlink=0
>   after dec_ref, or lookup-only paths) are left unchanged with comments.
> 
> Link: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5

Please don't send the series so quickly. I'd say twice per week is about
maximum sensible cadence. It takes time (easily several days) for people to
get to look at your patches and sending your patches sometimes even several
times per day just creates a mess in the mailbox.

Also in some previous version, I gave my Reviewed-by tag for patch 1 and
some comment and Reviewed-by tag for patch 2. So please reflect that in the
next posting.

								Honza

> v7:
>  - Replaced the deferred-iput array threading approach (v4-v6) with a
>    simpler per-sb workqueue + lock-free llist design.  No function
>    signature changes needed.  ext4_put_ea_inode() does direct iput when
>    SB_ACTIVE (zero overhead in normal operation) and defers to the
>    workqueue only during mount (!SB_ACTIVE).
>  - Converted the iput in ext4_xattr_delete_inode()'s quota accounting
>    loop to ext4_put_ea_inode() to eliminate a lockdep-reportable lock
>    ordering violation (jbd2_handle -> iput -> s_writepages_rwsem).
>  - Moved flush_work() before the if (sbi->s_journal) check in
>    ext4_put_super() to cover nojournal mode.
> 
> v6:
>  - ext4_inline_data_truncate(): use local ea_inode_array instead of
>    passing NULL, freed after ext4_journal_stop().  Fixes a deadlock
>    reachable via crafted filesystem where inline data xattr entry has
>    e_value_inum set: orphan cleanup -> ext4_truncate ->
>    ext4_inline_data_truncate -> iput under !SB_ACTIVE.
> 
> v5:
>  - Split into 3 patches for easier review.
>  - Add explicit !SB_ACTIVE early-return in ext4_try_to_expand_extra_isize()
>    to block ALL mount-time paths (ext4_process_orphan -> ext4_truncate ->
>    ext4_mark_inode_dirty), not just the eviction path. v4 only relied on
>    EXT4_STATE_NO_EXPAND which doesn't cover orphan truncation.
> 
> v4:
>  - Comprehensive rewrite of the deferred iput mechanism.
>  - Thread ea_inode_array through ext4_expand_extra_isize_ea() and
>    ext4_xattr_move_to_block() so ALL ea_inode iputs in the expand
>    path are deferred, not just those in ext4_xattr_block_set().
>  - Add NULL safety to ext4_expand_inode_array(): when ea_inode_array
>    pointer is NULL, fall back to synchronous iput (for callers like
>    ext4_initxattrs that only run with SB_ACTIVE).
>  - Use __GFP_NOFAIL to guarantee deferred array growth, eliminating
>    fallback to synchronous iput under locks.
>  - Update ext4_xattr_ibody_set() and ext4_xattr_set_entry() signatures
>    to accept ea_inode_array, converting ALL iput(ea_inode) calls.
>  - Set EXT4_STATE_NO_EXPAND in ext4_evict_inode() before
>    ext4_mark_inode_dirty().
> 
> v3:
>  - Check ext4_expand_inode_array() return value; fallback to
>    direct iput() on ENOMEM to prevent inode leak.
>  - Make ext4_xattr_set_handle() take an optional ea_inode_array
>    output parameter so callers can free after ext4_journal_stop(),
>    avoiding the jbd2_handle vs s_writepages_rwsem AB-BA.
>  - Pass ea_inode_array directly to ext4_xattr_release_block()
>    instead of using a local array freed under xattr_sem.
>  - Move ext4_xattr_inode_array_free() after ext4_journal_stop()
> 
> v2:
>  - Defer iput() in ext4_xattr_block_set() via ea_inode_array,
>    freed after xattr_sem is released. Fixes the root cause.
> 
> v1:
>  - Set EXT4_STATE_NO_EXPAND in ext4_evict_inode() to skip expand
>    on inodes being deleted. Only fixes the syzbot reproducer, not
>    the underlying lock ordering violation.
> 
> Yun Zhou (4):
>   ext4: skip extra isize expansion during mount to prevent deadlock
>   ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode
>   ext4: introduce ext4_put_ea_inode() for safe deferred iput
>   ext4: convert remaining EA inode iput() calls to ext4_put_ea_inode()
> 
>  fs/ext4/ext4.h  |   5 +++
>  fs/ext4/inode.c |  11 +++++
>  fs/ext4/super.c |   6 +++
>  fs/ext4/xattr.c | 105 +++++++++++++++++++++++++++++++++++++++++++-----
>  fs/ext4/xattr.h |   2 +
>  5 files changed, 120 insertions(+), 9 deletions(-)
> 
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v7 0/4] ext4: fix xattr iput deadlock with s_writepages_rwsem

[Zhou, Yun]

On 6/18/26 02:13, Jan Kara wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Tue 16-06-26 23:15:54, Yun Zhou wrote:
>> This series fixes a circular lock dependency reported by syzbot:
>>
>>    s_writepages_rwsem --> jbd2_handle --> xattr_sem --> s_writepages_rwsem
>>
>> The deadlock occurs when iput() on an EA inode triggers write_inode_now()
>> while xattr_sem and a jbd2 handle are held.  The triggering path is
>> during mount-time orphan cleanup (!SB_ACTIVE) where iput_final() calls
>> write_inode_now() synchronously.
>>
>> Patch 1 blocks the deadlock by skipping extra isize expansion when
>> !SB_ACTIVE -- this prevents the xattr manipulation path from being
>> entered during mount.
>>
>> Patch 2 is a belt-and-suspenders semantic improvement: an inode under
>> eviction never needs extra isize expansion.
>>
>> Patches 3-4 are a structural improvement using a per-sb workqueue:
>>
>>    Patch 3 introduces ext4_put_ea_inode(), which does direct iput() when
>>    SB_ACTIVE (zero overhead) and defers to a workqueue when !SB_ACTIVE.
>>    It also converts the first call site (ext4_xattr_block_set release
>>    path) which previously called iput under xattr_sem + jbd2 handle.
>>
>>    Patch 4 converts the remaining EA inode iput() calls that execute
>>    under locks.  Sites where direct iput() is provably safe (i_nlink=0
>>    after dec_ref, or lookup-only paths) are left unchanged with comments.
>>
>> Link: https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5
> Please don't send the series so quickly. I'd say twice per week is about
> maximum sensible cadence. It takes time (easily several days) for people to
> get to look at your patches and sending your patches sometimes even several
> times per day just creates a mess in the mailbox.

I admit I've been pushing patches too frequently, but I'll definitely tone
it down going forward. The reason is that I get feedback from the 'AI 
Review'
almost immediately after submitting. It flags a lot of edge cases—some I
introduced, others that were pre-existing, or even deadlock paths I missed
despite claiming they were fixed. Most of these are genuine issues. Even if
some are extremely strict, I just want to resolve them as quickly as 
possible.

I mistakenly thought that once the AI Reviewer raised concerns, human 
reviewers
wouldn't check it again. So I kept pushing new patches to fix those 
potential
issues, only for the bot to immediately flag new ones... It would be 
great if there
were a way to send patches directly to the AI bot for a separate review.
> Also in some previous version, I gave my Reviewed-by tag for patch 1 and
> some comment and Reviewed-by tag for patch 2. So please reflect that in the
> next posting.
Got it, I will add it in the next version.

Thanks,
Yun