This is a note to let you know that I've just added the patch titled
xfs: Fix deadlock between AGI and AGF when target_ip exists in xfs_rename()
to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
xfs-fix-deadlock-between-agi-and-agf-when-target_ip-exists-in-xfs_rename.patch
and it can be found in the queue-5.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From foo@baz Mon Sep 26 08:48:28 AM CEST 2022
From: Chandan Babu R <chandan.babu@xxxxxxxxxx>
Date: Sat, 24 Sep 2022 18:26:45 +0530
Subject: xfs: Fix deadlock between AGI and AGF when target_ip exists in xfs_rename()
To: gregkh@xxxxxxxxxxxxxxxxxxx
Cc: sashal@xxxxxxxxxx, mcgrof@xxxxxxxxxx, linux-xfs@xxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx, djwong@xxxxxxxxxx, chandan.babu@xxxxxxxxxx, amir73il@xxxxxxxxx, leah.rumancik@xxxxxxxxx
Message-ID: <20220924125656.101069-9-chandan.babu@xxxxxxxxxx>
From: kaixuxia <xiakaixu1987@xxxxxxxxx>
commit 93597ae8dac0149b5c00b787cba6bf7ba213e666 upstream.
When target_ip exists in xfs_rename(), the xfs_dir_replace() call may
need to hold the AGF lock to allocate more blocks, and then invoking
the xfs_droplink() call to hold AGI lock to drop target_ip onto the
unlinked list, so we get the lock order AGF->AGI. This would break the
ordering constraint on AGI and AGF locking - inode allocation locks
the AGI, then can allocate a new extent for new inodes, locking the
AGF after the AGI.
In this patch we check whether the replace operation need more
blocks firstly. If so, acquire the agi lock firstly to preserve
locking order(AGI/AGF). Actually, the locking order problem only
occurs when we are locking the AGI/AGF of the same AG. For multiple
AGs the AGI lock will be released after the transaction committed.
Signed-off-by: kaixuxia <kaixuxia@xxxxxxxxxxx>
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
[darrick: reword the comment]
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Acked-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
fs/xfs/libxfs/xfs_dir2.h | 2 ++
fs/xfs/libxfs/xfs_dir2_sf.c | 28 +++++++++++++++++++++++-----
fs/xfs/xfs_inode.c | 17 +++++++++++++++++
3 files changed, 42 insertions(+), 5 deletions(-)
--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -124,6 +124,8 @@ extern int xfs_dir_lookup(struct xfs_tra
extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
struct xfs_name *name, xfs_ino_t ino,
xfs_extlen_t tot);
+extern bool xfs_dir2_sf_replace_needblock(struct xfs_inode *dp,
+ xfs_ino_t inum);
extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
struct xfs_name *name, xfs_ino_t inum,
xfs_extlen_t tot);
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -945,6 +945,27 @@ xfs_dir2_sf_removename(
}
/*
+ * Check whether the sf dir replace operation need more blocks.
+ */
+bool
+xfs_dir2_sf_replace_needblock(
+ struct xfs_inode *dp,
+ xfs_ino_t inum)
+{
+ int newsize;
+ struct xfs_dir2_sf_hdr *sfp;
+
+ if (dp->i_d.di_format != XFS_DINODE_FMT_LOCAL)
+ return false;
+
+ sfp = (struct xfs_dir2_sf_hdr *)dp->i_df.if_u1.if_data;
+ newsize = dp->i_df.if_bytes + (sfp->count + 1) * XFS_INO64_DIFF;
+
+ return inum > XFS_DIR2_MAX_SHORT_INUM &&
+ sfp->i8count == 0 && newsize > XFS_IFORK_DSIZE(dp);
+}
+
+/*
* Replace the inode number of an entry in a shortform directory.
*/
int /* error */
@@ -980,17 +1001,14 @@ xfs_dir2_sf_replace(
*/
if (args->inumber > XFS_DIR2_MAX_SHORT_INUM && sfp->i8count == 0) {
int error; /* error return value */
- int newsize; /* new inode size */
- newsize = dp->i_df.if_bytes + (sfp->count + 1) * XFS_INO64_DIFF;
/*
* Won't fit as shortform, convert to block then do replace.
*/
- if (newsize > XFS_IFORK_DSIZE(dp)) {
+ if (xfs_dir2_sf_replace_needblock(dp, args->inumber)) {
error = xfs_dir2_sf_to_block(args);
- if (error) {
+ if (error)
return error;
- }
return xfs_dir2_block_replace(args);
}
/*
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3215,6 +3215,7 @@ xfs_rename(
struct xfs_trans *tp;
struct xfs_inode *wip = NULL; /* whiteout inode */
struct xfs_inode *inodes[__XFS_SORT_INODES];
+ struct xfs_buf *agibp;
int num_inodes = __XFS_SORT_INODES;
bool new_parent = (src_dp != target_dp);
bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
@@ -3379,6 +3380,22 @@ xfs_rename(
* In case there is already an entry with the same
* name at the destination directory, remove it first.
*/
+
+ /*
+ * Check whether the replace operation will need to allocate
+ * blocks. This happens when the shortform directory lacks
+ * space and we have to convert it to a block format directory.
+ * When more blocks are necessary, we must lock the AGI first
+ * to preserve locking order (AGI -> AGF).
+ */
+ if (xfs_dir2_sf_replace_needblock(target_dp, src_ip->i_ino)) {
+ error = xfs_read_agi(mp, tp,
+ XFS_INO_TO_AGNO(mp, target_ip->i_ino),
+ &agibp);
+ if (error)
+ goto out_trans_cancel;
+ }
+
error = xfs_dir_replace(tp, target_dp, target_name,
src_ip->i_ino, spaceres);
if (error)
Patches currently in stable-queue which might be from chandan.babu@xxxxxxxxxx are
queue-5.4/xfs-refactor-agfl-length-computation-function.patch
queue-5.4/xfs-use-bitops-interface-for-buf-log-item-ail-flag-check.patch
queue-5.4/xfs-fix-an-abba-deadlock-in-xfs_rename.patch
queue-5.4/maintainers-add-chandan-as-xfs-maintainer-for-5.4.y.patch
queue-5.4/xfs-split-the-sunit-parameter-update-into-two-parts.patch
queue-5.4/xfs-slightly-tweak-an-assert-in-xfs_fs_map_blocks.patch
queue-5.4/xfs-fix-use-after-free-when-aborting-corrupt-attr-inactivation.patch
queue-5.4/xfs-stabilize-insert-range-start-boundary-to-avoid-cow-writeback-race.patch
queue-5.4/iomap-iomap-that-extends-beyond-eof-should-be-marked-dirty.patch
queue-5.4/xfs-constify-the-buffer-pointer-arguments-to-error-functions.patch
queue-5.4/xfs-attach-dquots-and-reserve-quota-blocks-during-unwritten-conversion.patch
queue-5.4/xfs-range-check-ri_cnt-when-recovering-log-items.patch
queue-5.4/xfs-fix-deadlock-between-agi-and-agf-when-target_ip-exists-in-xfs_rename.patch
queue-5.4/xfs-replace-eio-with-efscorrupted-for-corrupt-metadata.patch
queue-5.4/xfs-convert-eio-to-efscorrupted-when-log-contents-are-invalid.patch
queue-5.4/xfs-add-missing-assert-in-xfs_fsmap_owner_from_rmap.patch
queue-5.4/xfs-always-log-corruption-errors.patch
queue-5.4/xfs-fix-some-memory-leaks-in-log-recovery.patch
queue-5.4/xfs-don-t-commit-sunit-swidth-updates-to-disk-if-that-would-cause-repair-failures.patch
