diff mbox series

[v3,04/26] xfs: Hold inode locks in xfs_trans_alloc_dir

Message ID 20220922054458.40826-5-allison.henderson@oracle.com (mailing list archive)
State Superseded
Headers show
Series Parent Pointers | expand

Commit Message

Allison Henderson Sept. 22, 2022, 5:44 a.m. UTC 
From: Allison Henderson <allison.henderson@oracle.com>

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@oracle.com>
---
 fs/xfs/xfs_inode.c | 14 ++++++++++++--
 fs/xfs/xfs_trans.c |  6 ++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

Comments

Darrick J. Wong Sept. 23, 2022, 7:04 p.m. UTC | #1 
On Wed, Sep 21, 2022 at 10:44:36PM -0700, allison.henderson@oracle.com wrote:
> From: Allison Henderson <allison.henderson@oracle.com>
> 
> 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@oracle.com>

Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  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
>
Allison Henderson Sept. 23, 2022, 8:44 p.m. UTC | #2 
On Fri, 2022-09-23 at 12:04 -0700, Darrick J. Wong wrote:
> On Wed, Sep 21, 2022 at 10:44:36PM -0700,
> allison.henderson@oracle.com wrote:
> > From: Allison Henderson <allison.henderson@oracle.com>
> > 
> > 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@oracle.com>
> 
> Looks ok,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Great, thanks!
Allison
> 
> --D
> 
> > ---
> >  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
> >
diff mbox series

Patch

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) {