> On Oct 21, 2022, at 3:29 PM, allison.henderson@xxxxxxxxxx wrote:
>
> From: Allison Henderson <allison.henderson@xxxxxxxxxx>
>
> Modify xfs_rename to hold all inode locks across a rename operation
> We will need this later when we add parent pointers
>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Looks good
Reviewed-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.c | 42 +++++++++++++++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 9a3174a8f895..44b68fa53a72 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2539,6 +2539,21 @@ xfs_remove(
> return error;
> }
>
> +static inline void
> +xfs_iunlock_after_rename(
> + struct xfs_inode **i_tab,
> + int num_inodes)
> +{
> + int i;
> +
> + for (i = num_inodes - 1; i >= 0; i--) {
> + /* Skip duplicate inodes if src and target dps are the same */
> + if (!i_tab[i] || (i > 0 && i_tab[i] == i_tab[i - 1]))
> + continue;
> + xfs_iunlock(i_tab[i], XFS_ILOCK_EXCL);
> + }
> +}
> +
> /*
> * Enter all inodes for a rename transaction into a sorted array.
> */
> @@ -2837,18 +2852,16 @@ xfs_rename(
> xfs_lock_inodes(inodes, num_inodes, XFS_ILOCK_EXCL);
>
> /*
> - * Join all the inodes to the transaction. From this point on,
> - * we can rely on either trans_commit or trans_cancel to unlock
> - * them.
> + * Join all the inodes to the transaction.
> */
> - xfs_trans_ijoin(tp, src_dp, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, src_dp, 0);
> if (new_parent)
> - xfs_trans_ijoin(tp, target_dp, XFS_ILOCK_EXCL);
> - xfs_trans_ijoin(tp, src_ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, target_dp, 0);
> + xfs_trans_ijoin(tp, src_ip, 0);
> if (target_ip)
> - xfs_trans_ijoin(tp, target_ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, target_ip, 0);
> if (wip)
> - xfs_trans_ijoin(tp, wip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, wip, 0);
>
> /*
> * If we are using project inheritance, we only allow renames
> @@ -2862,10 +2875,12 @@ xfs_rename(
> }
>
> /* RENAME_EXCHANGE is unique from here on. */
> - if (flags & RENAME_EXCHANGE)
> - return xfs_cross_rename(tp, src_dp, src_name, src_ip,
> + if (flags & RENAME_EXCHANGE) {
> + error = xfs_cross_rename(tp, src_dp, src_name, src_ip,
> target_dp, target_name, target_ip,
> spaceres);
> + goto out_unlock;
> + }
>
> /*
> * Try to reserve quota to handle an expansion of the target directory.
> @@ -3090,12 +3105,13 @@ xfs_rename(
> xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
>
> error = xfs_finish_rename(tp);
> - if (wip)
> - xfs_irele(wip);
> - return error;
> +
> + goto out_unlock;
>
> out_trans_cancel:
> xfs_trans_cancel(tp);
> +out_unlock:
> + xfs_iunlock_after_rename(inodes, num_inodes);
> out_release_wip:
> if (wip)
> xfs_irele(wip);
> --
> 2.25.1
>
