* [merged mm-nonmm-stable] ocfs2-fix-deadlock-in-inline-data-truncate-transactions.patch removed from -mm tree
@ 2026-10-07 2:04 Andrew Morton
0 siblings, 0 replies; only message in thread
From: Andrew Morton @ 2026-10-07 2:04 UTC (permalink / raw)
To: mm-commits, piaojun, mark, junxiao.bi, jlbec, heming.zhao,
gechangwei, gality369, joseph.qi, akpm
The quilt patch titled
Subject: ocfs2: fix deadlock in inline-data truncate transactions
has been removed from the -mm tree. Its filename was
ocfs2-fix-deadlock-in-inline-data-truncate-transactions.patch
This patch was dropped because it was merged into the mm-nonmm-stable branch
of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
------------------------------------------------------
From: Joseph Qi <joseph.qi@linux.alibaba.com>
Subject: ocfs2: fix deadlock in inline-data truncate transactions
Date: Wed, 26 Aug 2026 19:26:59 +0800
Updating an inode xattr can cause an ABBA deadlock with inline file
truncation:
ocfs2_truncate_file()
down_write(&oi->ip_alloc_sem)
ocfs2_truncate_inline()
ocfs2_start_trans()
ocfs2_xattr_set()
ocfs2_start_trans()
ocfs2_xattr_ibody_set()
down_write(&oi->ip_alloc_sem)
The xattr set path starts the merged transaction before the inode-body
xattr helper acquires ip_alloc_sem, reversing the ip_alloc_sem ->
transaction order used by the allocation and truncate paths. The
transaction merge in commit 85db90e77806 ("ocfs2/xattr: Merge xattr set
transaction.") introduced this ordering.
Fix it by acquiring ip_alloc_sem once in ocfs2_xattr_set(), before xattr
preparation, allocation reservations and ocfs2_start_trans(), and removing
the per-helper acquisition from ocfs2_xattr_ibody_find(),
ocfs2_xattr_ibody_set() and ocfs2_xattr_create_index_block(). These
helpers now assert via lockdep that the caller holds ip_alloc_sem.
ocfs2_xattr_set_handle(), which only sets initial ACL or security xattrs
on unpublished inodes inside the create transaction, takes ip_alloc_sem
under a dedicated lockdep subclass so that the assertions hold without
creating a transaction -> ip_alloc_sem cycle against the ip_alloc_sem ->
transaction order. The inode is unpublished, so the acquisition can never
contend.
This keeps the established ip_alloc_sem -> transaction order and makes the
locking unconditional, so lockdep can verify a single plain ordering
instead of conditional acquisitions.
Link: https://lore.kernel.org/20260826112659.246574-1-joseph.qi@linux.alibaba.com
Fixes: 85db90e77806 ("ocfs2/xattr: Merge xattr set transaction.")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: ZhengYuan Huang <gality369@gmail.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
---
fs/ocfs2/xattr.c | 79 ++++++++++++++++++++++++++++++---------------
1 file changed, 53 insertions(+), 26 deletions(-)
--- a/fs/ocfs2/xattr.c~ocfs2-fix-deadlock-in-inline-data-truncate-transactions
+++ a/fs/ocfs2/xattr.c
@@ -2909,6 +2909,9 @@ static int ocfs2_xattr_has_space_inline(
*
* Find extended attribute in inode block and
* fill search info into struct ocfs2_xattr_search.
+ *
+ * The inline free-space check races with truncate and allocation, so
+ * callers must hold ip_alloc_sem for writing.
*/
static int ocfs2_xattr_ibody_find(struct inode *inode,
int name_index,
@@ -2920,13 +2923,13 @@ static int ocfs2_xattr_ibody_find(struct
int ret;
int has_space = 0;
+ lockdep_assert_held_write(&oi->ip_alloc_sem);
+
if (inode->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE)
return 0;
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
- down_read(&oi->ip_alloc_sem);
has_space = ocfs2_xattr_has_space_inline(inode, di);
- up_read(&oi->ip_alloc_sem);
if (!has_space)
return 0;
}
@@ -3007,6 +3010,7 @@ out:
*
* Set, replace or remove an extended attribute into inode block.
*
+ * Callers must hold ip_alloc_sem for writing.
*/
static int ocfs2_xattr_ibody_set(struct inode *inode,
struct ocfs2_xattr_info *xi,
@@ -3017,16 +3021,17 @@ static int ocfs2_xattr_ibody_set(struct
struct ocfs2_inode_info *oi = OCFS2_I(inode);
struct ocfs2_xa_loc loc;
+ lockdep_assert_held_write(&oi->ip_alloc_sem);
+
if (inode->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE)
return -ENOSPC;
- down_write(&oi->ip_alloc_sem);
if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
ret = ocfs2_xattr_ibody_init(inode, xs->inode_bh, ctxt);
if (ret) {
if (ret != -ENOSPC)
mlog_errno(ret);
- goto out;
+ return ret;
}
}
@@ -3036,13 +3041,10 @@ static int ocfs2_xattr_ibody_set(struct
if (ret) {
if (ret != -ENOSPC)
mlog_errno(ret);
- goto out;
+ return ret;
}
xs->here = loc.xl_entry;
-out:
- up_write(&oi->ip_alloc_sem);
-
return ret;
}
@@ -3687,6 +3689,18 @@ out:
}
/*
+ * ip_alloc_sem subclass for inodes being initialized before publication.
+ * ocfs2_xattr_set_handle() runs inside the create transaction, so taking
+ * ip_alloc_sem there adds a transaction -> ip_alloc_sem order that would
+ * form a lockdep cycle with the ip_alloc_sem -> transaction order used
+ * elsewhere, if not for this separate subclass. The inode is unpublished
+ * so the acquisition can never contend.
+ */
+enum {
+ OCFS2_IP_ALLOC_SEM_UNPUBLISHED = 1,
+};
+
+/*
* This helper is only for setting initial ACL or security xattrs on an inode
* that is still unpublished, unhashed, and unattached to a dentry.
* Ordinary xattr updates must use ocfs2_xattr_set().
@@ -3747,6 +3761,13 @@ int ocfs2_xattr_set_handle(handle_t *han
xis.inode_bh = xbs.inode_bh = di_bh;
di = (struct ocfs2_dinode *)di_bh->b_data;
+ /*
+ * The inode is unpublished and cannot contend, but take the
+ * semaphore anyway so the helpers' lockdep assertions hold.
+ */
+ down_write_nested(&OCFS2_I(inode)->ip_alloc_sem,
+ OCFS2_IP_ALLOC_SEM_UNPUBLISHED);
+
ret = ocfs2_xattr_ibody_find(inode, name_index, name, &xis);
if (ret)
goto cleanup;
@@ -3759,6 +3780,7 @@ int ocfs2_xattr_set_handle(handle_t *han
ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt);
cleanup:
+ up_write(&OCFS2_I(inode)->ip_alloc_sem);
brelse(xbs.xattr_bh);
ocfs2_xattr_bucket_free(xbs.bucket);
@@ -3828,29 +3850,37 @@ int ocfs2_xattr_set(struct inode *inode,
down_write(&OCFS2_I(inode)->ip_xattr_sem);
/*
+ * The allocation and truncate paths take ip_alloc_sem before
+ * starting a transaction, so take it here before xattr
+ * preparation, allocation reservations and ocfs2_start_trans()
+ * to keep that order. The xattr helpers below no longer take
+ * it themselves.
+ */
+ down_write(&OCFS2_I(inode)->ip_alloc_sem);
+ /*
* Scan inode and external block to find the same name
* extended attribute and collect search information.
*/
ret = ocfs2_xattr_ibody_find(inode, name_index, name, &xis);
if (ret)
- goto cleanup;
+ goto out_free_ac;
if (xis.not_found) {
ret = ocfs2_xattr_block_find(inode, name_index, name, &xbs);
if (ret)
- goto cleanup;
+ goto out_free_ac;
}
if (xis.not_found && xbs.not_found) {
ret = -ENODATA;
if (flags & XATTR_REPLACE)
- goto cleanup;
+ goto out_free_ac;
ret = 0;
if (!value)
- goto cleanup;
+ goto out_free_ac;
} else {
ret = -EEXIST;
if (flags & XATTR_CREATE)
- goto cleanup;
+ goto out_free_ac;
}
/* Check whether the value is refcounted and do some preparation. */
@@ -3861,7 +3891,7 @@ int ocfs2_xattr_set(struct inode *inode,
&ref_meta, &ref_credits);
if (ret) {
mlog_errno(ret);
- goto cleanup;
+ goto out_free_ac;
}
}
@@ -3872,7 +3902,7 @@ int ocfs2_xattr_set(struct inode *inode,
if (ret < 0) {
inode_unlock(tl_inode);
mlog_errno(ret);
- goto cleanup;
+ goto out_free_ac;
}
}
inode_unlock(tl_inode);
@@ -3881,7 +3911,7 @@ int ocfs2_xattr_set(struct inode *inode,
&xbs, &ctxt, ref_meta, &credits);
if (ret) {
mlog_errno(ret);
- goto cleanup;
+ goto out_free_ac;
}
/* we need to update inode's ctime field, so add credit for it. */
@@ -3899,6 +3929,7 @@ int ocfs2_xattr_set(struct inode *inode,
ocfs2_commit_trans(osb, ctxt.handle);
out_free_ac:
+ up_write(&OCFS2_I(inode)->ip_alloc_sem);
if (ctxt.data_ac)
ocfs2_free_alloc_context(ctxt.data_ac);
if (ctxt.meta_ac)
@@ -3907,7 +3938,6 @@ out_free_ac:
ocfs2_schedule_truncate_log_flush(osb, 1);
ocfs2_run_deallocs(osb, &ctxt.dealloc);
-cleanup:
if (ref_tree)
ocfs2_unlock_refcount_tree(osb, ref_tree, 1);
up_write(&OCFS2_I(inode)->ip_xattr_sem);
@@ -4508,6 +4538,10 @@ static void ocfs2_xattr_update_xattr_sea
xs->here = &xs->header->xh_entries[i];
}
+/*
+ * Caller must hold ip_alloc_sem for writing, since a new xattr block
+ * is allocated and the xattr block header is rewritten.
+ */
static int ocfs2_xattr_create_index_block(struct inode *inode,
struct ocfs2_xattr_search *xs,
struct ocfs2_xattr_set_ctxt *ctxt)
@@ -4523,19 +4557,14 @@ static int ocfs2_xattr_create_index_bloc
struct ocfs2_xattr_tree_root *xr;
u16 xb_flags = le16_to_cpu(xb->xb_flags);
+ lockdep_assert_held_write(&oi->ip_alloc_sem);
+
trace_ocfs2_xattr_create_index_block_begin(
(unsigned long long)xb_bh->b_blocknr);
BUG_ON(xb_flags & OCFS2_XATTR_INDEXED);
BUG_ON(!xs->bucket);
- /*
- * XXX:
- * We can use this lock for now, and maybe move to a dedicated mutex
- * if performance becomes a problem later.
- */
- down_write(&oi->ip_alloc_sem);
-
ret = ocfs2_journal_access_xb(handle, INODE_CACHE(inode), xb_bh,
OCFS2_JOURNAL_ACCESS_WRITE);
if (ret) {
@@ -4597,8 +4626,6 @@ static int ocfs2_xattr_create_index_bloc
ocfs2_journal_dirty(handle, xb_bh);
out:
- up_write(&oi->ip_alloc_sem);
-
return ret;
}
_
Patches currently in -mm which might be from joseph.qi@linux.alibaba.com are
ocfs2-retain-all-security-xattrs-during-inode-creation.patch
ocfs2-restore-erange-check-in-ocfs2_xattr_tree_list_index_block.patch
ocfs2-fix-possible-deadlock-between-nfs_sync_rwlock-and-fs_reclaim.patch
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2026-10-07 2:04 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07 2:04 [merged mm-nonmm-stable] ocfs2-fix-deadlock-in-inline-data-truncate-transactions.patch removed from -mm tree Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.