From: Andrew Morton <akpm@linux-foundation.org>
To: mm-commits@vger.kernel.org,piaojun@huawei.com,mark@fasheh.com,junxiao.bi@oracle.com,jlbec@evilplan.org,heming.zhao@suse.com,gechangwei@live.cn,gality369@gmail.com,joseph.qi@linux.alibaba.com,akpm@linux-foundation.org
Subject: [merged mm-nonmm-stable] ocfs2-fix-deadlock-in-inline-data-truncate-transactions.patch removed from -mm tree
Date: Tue, 06 Oct 2026 19:04:00 -0700 [thread overview]
Message-ID: <20261007020400.DF3A51F0089D@smtp.kernel.org> (raw)
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
reply other threads:[~2026-10-07 2:04 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261007020400.DF3A51F0089D@smtp.kernel.org \
--to=akpm@linux-foundation.org \
--cc=gality369@gmail.com \
--cc=gechangwei@live.cn \
--cc=heming.zhao@suse.com \
--cc=jlbec@evilplan.org \
--cc=joseph.qi@linux.alibaba.com \
--cc=junxiao.bi@oracle.com \
--cc=mark@fasheh.com \
--cc=mm-commits@vger.kernel.org \
--cc=piaojun@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.