> On Oct 21, 2022, at 3:29 PM, allison.henderson@xxxxxxxxxx wrote:
>
> From: Allison Henderson <allison.henderson@xxxxxxxxxx>
>
> Modify xfs_trans_alloc_dir to hold locks after return. Caller will be
> responsible for manual unlock. We will need this later to hold locks
> across parent pointer operations
>
> 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 | 14 ++++++++++++--
> fs/xfs/xfs_trans.c | 6 ++++--
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index f21f625b428e..9a3174a8f895 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1277,10 +1277,15 @@ xfs_link(
> if (xfs_has_wsync(mp) || xfs_has_dirsync(mp))
> xfs_trans_set_sync(tp);
>
> - return xfs_trans_commit(tp);
> + error = xfs_trans_commit(tp);
> + xfs_iunlock(tdp, XFS_ILOCK_EXCL);
> + xfs_iunlock(sip, XFS_ILOCK_EXCL);
> + return error;
>
> error_return:
> xfs_trans_cancel(tp);
> + xfs_iunlock(tdp, XFS_ILOCK_EXCL);
> + xfs_iunlock(sip, XFS_ILOCK_EXCL);
> std_return:
> if (error == -ENOSPC && nospace_error)
> error = nospace_error;
> @@ -2516,15 +2521,20 @@ xfs_remove(
>
> error = xfs_trans_commit(tp);
> if (error)
> - goto std_return;
> + goto out_unlock;
>
> if (is_dir && xfs_inode_is_filestream(ip))
> xfs_filestream_deassociate(ip);
>
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + xfs_iunlock(dp, XFS_ILOCK_EXCL);
> return 0;
>
> out_trans_cancel:
> xfs_trans_cancel(tp);
> + out_unlock:
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + xfs_iunlock(dp, XFS_ILOCK_EXCL);
> std_return:
> return error;
> }
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 7bd16fbff534..ac98ff416e54 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1356,6 +1356,8 @@ xfs_trans_alloc_ichange(
> * The caller must ensure that the on-disk dquots attached to this inode have
> * already been allocated and initialized. The ILOCKs will be dropped when the
> * transaction is committed or cancelled.
> + *
> + * Caller is responsible for unlocking the inodes manually upon return
> */
> int
> xfs_trans_alloc_dir(
> @@ -1386,8 +1388,8 @@ xfs_trans_alloc_dir(
>
> xfs_lock_two_inodes(dp, XFS_ILOCK_EXCL, ip, XFS_ILOCK_EXCL);
>
> - xfs_trans_ijoin(tp, dp, XFS_ILOCK_EXCL);
> - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, dp, 0);
> + xfs_trans_ijoin(tp, ip, 0);
>
> error = xfs_qm_dqattach_locked(dp, false);
> if (error) {
> --
> 2.25.1
>
