Skip to content

Commit

Permalink
xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT
Browse files Browse the repository at this point in the history
Backport from the mainline kernel:
	commit bc56ad8c74b8588685c2875de0df8ab6974828ef
	commit 3fb21fc8cc04e9a75a426510dfe597f0d0b19134

Fix deadlock between AGI and AGF with RENAME_WHITEOUT.

Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
  • Loading branch information
kaixuxiakx authored and gxm-newton committed Jan 2, 2020
1 parent 9d289d8 commit f8c4b7a
Showing 1 changed file with 41 additions and 43 deletions.
84 changes: 41 additions & 43 deletions fs/xfs/xfs_inode.c
Expand Up @@ -3035,7 +3035,8 @@ xfs_rename(
&dfops, &first_block, spaceres);

/*
* Set up the target.
* Check for expected errors before we dirty the transaction
* so we can return an error without a transaction abort.
*/
if (target_ip == NULL) {
/*
Expand All @@ -3047,6 +3048,45 @@ xfs_rename(
if (error)
goto out_trans_cancel;
}
} else {
/*
* If target exists and it's a directory, check that whether
* it can be destroyed.
*/
if (S_ISDIR(VFS_I(target_ip)->i_mode) &&
(!xfs_dir_isempty(target_ip) ||
(VFS_I(target_ip)->i_nlink > 2))) {
error = -EEXIST;
goto out_trans_cancel;
}
}

/*
* Directory entry creation below may acquire the AGF. Remove
* the whiteout from the unlinked list first to preserve correct
* AGI/AGF locking order. This dirties the transaction so failures
* after this point will abort and log recovery will clean up the
* mess.
*
* For whiteouts, we need to bump the link count on the whiteout
* inode. After this point, we have a real link, clear the tmpfile
* state flag from the inode so it doesn't accidentally get misused
* in future.
*/
if (wip) {
ASSERT(VFS_I(wip)->i_nlink == 0);
error = xfs_iunlink_remove(tp, wip);
if (error)
goto out_trans_cancel;

xfs_bumplink(tp, wip);
VFS_I(wip)->i_state &= ~I_LINKABLE;
}

/*
* Set up the target.
*/
if (target_ip == NULL) {
/*
* If target does not exist and the rename crosses
* directories, adjust the target directory link count
Expand All @@ -3067,22 +3107,6 @@ xfs_rename(
goto out_bmap_cancel;
}
} else { /* target_ip != NULL */
/*
* If target exists and it's a directory, check that both
* target and source are directories and that target can be
* destroyed, or that neither is a directory.
*/
if (S_ISDIR(VFS_I(target_ip)->i_mode)) {
/*
* Make sure target dir is empty.
*/
if (!(xfs_dir_isempty(target_ip)) ||
(VFS_I(target_ip)->i_nlink > 2)) {
error = -EEXIST;
goto out_trans_cancel;
}
}

/*
* Link the source inode under the target name.
* If the source inode is a directory and we are moving
Expand Down Expand Up @@ -3175,32 +3199,6 @@ xfs_rename(
if (error)
goto out_bmap_cancel;

/*
* For whiteouts, we need to bump the link count on the whiteout inode.
* This means that failures all the way up to this point leave the inode
* on the unlinked list and so cleanup is a simple matter of dropping
* the remaining reference to it. If we fail here after bumping the link
* count, we're shutting down the filesystem so we'll never see the
* intermediate state on disk.
*/
if (wip) {
ASSERT(VFS_I(wip)->i_nlink == 0);
error = xfs_bumplink(tp, wip);
if (error)
goto out_bmap_cancel;
error = xfs_iunlink_remove(tp, wip);
if (error)
goto out_bmap_cancel;
xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE);

/*
* Now we have a real link, clear the "I'm a tmpfile" state
* flag from the inode so it doesn't accidentally get misused in
* future.
*/
VFS_I(wip)->i_state &= ~I_LINKABLE;
}

xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE);
if (new_parent)
Expand Down

0 comments on commit f8c4b7a

Please sign in to comment.