| Message ID | 1525627494-12873-15-git-send-email-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, May 06, 2018 at 10:24:47AM -0700, Allison Henderson wrote: > Add lock_flags to xfs_ialloc and xfs_dir_ialloc to control > whick locks are released by xfs_trans_ijoin. We will need this > later in defered parent pointers > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/xfs_inode.c | 17 +++++++++-------- > fs/xfs/xfs_inode.h | 2 +- > fs/xfs/xfs_qm.c | 2 +- > fs/xfs/xfs_symlink.c | 2 +- > 4 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 5c291d2..2859a697 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -766,7 +766,8 @@ xfs_ialloc( > dev_t rdev, > prid_t prid, > xfs_buf_t **ialloc_context, > - xfs_inode_t **ipp) > + xfs_inode_t **ipp, > + int lock_flags) Wait, what? Oh, these are the locks we want *dropped* at the first _trans_commit after this call returns, and for xfs_create we need to retain the ilock while we roll the transaction(s) during _defer_finish; and for everything else (create temp file, create quota inode, and symlink??) we want the ilock dropped as soon as the transaction commits. I dislike having this oddly named parameter, can we amend the comment to say that the caller is responsible for unlocking the inode manually (i.e. we're going to xfs_trans_ijoin(tp, ip, 0)) , and then change all the callers to do the iunlock explicitly if they need to? --D > { > struct xfs_mount *mp = tp->t_mountp; > xfs_ino_t ino; > @@ -942,7 +943,7 @@ xfs_ialloc( > /* > * Log the new values stuffed into the inode. > */ > - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); > + xfs_trans_ijoin(tp, ip, lock_flags); > xfs_trans_log_inode(tp, ip, flags); > > /* now that we have an i_mode we can setup the inode structure */ > @@ -972,8 +973,8 @@ xfs_dir_ialloc( > xfs_nlink_t nlink, > dev_t rdev, > prid_t prid, /* project id */ > - xfs_inode_t **ipp) /* pointer to inode; it will be > - locked. */ > + xfs_inode_t **ipp, /* pointer to inode; it will be locked. */ > + int lock_flags) > { > xfs_trans_t *tp; > xfs_inode_t *ip; > @@ -1001,7 +1002,7 @@ xfs_dir_ialloc( > * the inode(s) that we've just allocated. > */ > code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, &ialloc_context, > - &ip); > + &ip, lock_flags); > > /* > * Return an error if we were unable to allocate a new inode. > @@ -1071,7 +1072,7 @@ xfs_dir_ialloc( > * this call should always succeed. > */ > code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, > - &ialloc_context, &ip); > + &ialloc_context, &ip, lock_flags); > > /* > * If we get an error at this point, return to the caller > @@ -1210,7 +1211,7 @@ xfs_create( > * entry pointing to them, but a directory also the "." entry > * pointing to itself. > */ > - error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip); > + error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip, XFS_ILOCK_EXCL); > if (error) > goto out_trans_cancel; > > @@ -1343,7 +1344,7 @@ xfs_create_tmpfile( > if (error) > goto out_trans_cancel; > > - error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip); > + error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip, XFS_ILOCK_EXCL); > if (error) > goto out_trans_cancel; > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index 1eebc53..466f252 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h > @@ -431,7 +431,7 @@ xfs_extlen_t xfs_get_cowextsz_hint(struct xfs_inode *ip); > > int xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t, > xfs_nlink_t, dev_t, prid_t, > - struct xfs_inode **); > + struct xfs_inode **, int lock_flags); > > /* from xfs_file.c */ > enum xfs_prealloc_flags { > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > index ec39ae2..3e68a52 100644 > --- a/fs/xfs/xfs_qm.c > +++ b/fs/xfs/xfs_qm.c > @@ -787,7 +787,7 @@ xfs_qm_qino_alloc( > return error; > > if (need_alloc) { > - error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip); > + error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip, XFS_ILOCK_EXCL); > if (error) { > xfs_trans_cancel(tp); > return error; > diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c > index b1d3301..ce8dbea 100644 > --- a/fs/xfs/xfs_symlink.c > +++ b/fs/xfs/xfs_symlink.c > @@ -264,7 +264,7 @@ xfs_symlink( > * Allocate an inode for the symlink. > */ > error = xfs_dir_ialloc(&tp, dp, S_IFLNK | (mode & ~S_IFMT), 1, 0, > - prid, &ip); > + prid, &ip, XFS_ILOCK_EXCL); > if (error) > goto out_trans_cancel; > > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2018 03:30 PM, Darrick J. Wong wrote: > On Sun, May 06, 2018 at 10:24:47AM -0700, Allison Henderson wrote: >> Add lock_flags to xfs_ialloc and xfs_dir_ialloc to control >> whick locks are released by xfs_trans_ijoin. We will need this >> later in defered parent pointers >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> --- >> fs/xfs/xfs_inode.c | 17 +++++++++-------- >> fs/xfs/xfs_inode.h | 2 +- >> fs/xfs/xfs_qm.c | 2 +- >> fs/xfs/xfs_symlink.c | 2 +- >> 4 files changed, 12 insertions(+), 11 deletions(-) >> >> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c >> index 5c291d2..2859a697 100644 >> --- a/fs/xfs/xfs_inode.c >> +++ b/fs/xfs/xfs_inode.c >> @@ -766,7 +766,8 @@ xfs_ialloc( >> dev_t rdev, >> prid_t prid, >> xfs_buf_t **ialloc_context, >> - xfs_inode_t **ipp) >> + xfs_inode_t **ipp, >> + int lock_flags) > > Wait, what? > > Oh, these are the locks we want *dropped* at the first _trans_commit > after this call returns, and for xfs_create we need to retain the ilock > while we roll the transaction(s) during _defer_finish; and for > everything else (create temp file, create quota inode, and symlink??) we > want the ilock dropped as soon as the transaction commits. > > I dislike having this oddly named parameter, can we amend the comment to > say that the caller is responsible for unlocking the inode manually > (i.e. we're going to xfs_trans_ijoin(tp, ip, 0)) , and then change all > the callers to do the iunlock explicitly if they need to? > > --D Sure, I can update the comment. Hmm, would an op_flag be a more appropriate parameter in this case? Thx! Allison > >> { >> struct xfs_mount *mp = tp->t_mountp; >> xfs_ino_t ino; >> @@ -942,7 +943,7 @@ xfs_ialloc( >> /* >> * Log the new values stuffed into the inode. >> */ >> - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); >> + xfs_trans_ijoin(tp, ip, lock_flags); >> xfs_trans_log_inode(tp, ip, flags); >> >> /* now that we have an i_mode we can setup the inode structure */ >> @@ -972,8 +973,8 @@ xfs_dir_ialloc( >> xfs_nlink_t nlink, >> dev_t rdev, >> prid_t prid, /* project id */ >> - xfs_inode_t **ipp) /* pointer to inode; it will be >> - locked. */ >> + xfs_inode_t **ipp, /* pointer to inode; it will be locked. */ >> + int lock_flags) >> { >> xfs_trans_t *tp; >> xfs_inode_t *ip; >> @@ -1001,7 +1002,7 @@ xfs_dir_ialloc( >> * the inode(s) that we've just allocated. >> */ >> code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, &ialloc_context, >> - &ip); >> + &ip, lock_flags); >> >> /* >> * Return an error if we were unable to allocate a new inode. >> @@ -1071,7 +1072,7 @@ xfs_dir_ialloc( >> * this call should always succeed. >> */ >> code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, >> - &ialloc_context, &ip); >> + &ialloc_context, &ip, lock_flags); >> >> /* >> * If we get an error at this point, return to the caller >> @@ -1210,7 +1211,7 @@ xfs_create( >> * entry pointing to them, but a directory also the "." entry >> * pointing to itself. >> */ >> - error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip); >> + error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip, XFS_ILOCK_EXCL); >> if (error) >> goto out_trans_cancel; >> >> @@ -1343,7 +1344,7 @@ xfs_create_tmpfile( >> if (error) >> goto out_trans_cancel; >> >> - error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip); >> + error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip, XFS_ILOCK_EXCL); >> if (error) >> goto out_trans_cancel; >> >> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h >> index 1eebc53..466f252 100644 >> --- a/fs/xfs/xfs_inode.h >> +++ b/fs/xfs/xfs_inode.h >> @@ -431,7 +431,7 @@ xfs_extlen_t xfs_get_cowextsz_hint(struct xfs_inode *ip); >> >> int xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t, >> xfs_nlink_t, dev_t, prid_t, >> - struct xfs_inode **); >> + struct xfs_inode **, int lock_flags); >> >> /* from xfs_file.c */ >> enum xfs_prealloc_flags { >> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c >> index ec39ae2..3e68a52 100644 >> --- a/fs/xfs/xfs_qm.c >> +++ b/fs/xfs/xfs_qm.c >> @@ -787,7 +787,7 @@ xfs_qm_qino_alloc( >> return error; >> >> if (need_alloc) { >> - error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip); >> + error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip, XFS_ILOCK_EXCL); >> if (error) { >> xfs_trans_cancel(tp); >> return error; >> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c >> index b1d3301..ce8dbea 100644 >> --- a/fs/xfs/xfs_symlink.c >> +++ b/fs/xfs/xfs_symlink.c >> @@ -264,7 +264,7 @@ xfs_symlink( >> * Allocate an inode for the symlink. >> */ >> error = xfs_dir_ialloc(&tp, dp, S_IFLNK | (mode & ~S_IFMT), 1, 0, >> - prid, &ip); >> + prid, &ip, XFS_ILOCK_EXCL); >> if (error) >> goto out_trans_cancel; >> >> -- >> 2.7.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 5c291d2..2859a697 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -766,7 +766,8 @@ xfs_ialloc( dev_t rdev, prid_t prid, xfs_buf_t **ialloc_context, - xfs_inode_t **ipp) + xfs_inode_t **ipp, + int lock_flags) { struct xfs_mount *mp = tp->t_mountp; xfs_ino_t ino; @@ -942,7 +943,7 @@ xfs_ialloc( /* * Log the new values stuffed into the inode. */ - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); + xfs_trans_ijoin(tp, ip, lock_flags); xfs_trans_log_inode(tp, ip, flags); /* now that we have an i_mode we can setup the inode structure */ @@ -972,8 +973,8 @@ xfs_dir_ialloc( xfs_nlink_t nlink, dev_t rdev, prid_t prid, /* project id */ - xfs_inode_t **ipp) /* pointer to inode; it will be - locked. */ + xfs_inode_t **ipp, /* pointer to inode; it will be locked. */ + int lock_flags) { xfs_trans_t *tp; xfs_inode_t *ip; @@ -1001,7 +1002,7 @@ xfs_dir_ialloc( * the inode(s) that we've just allocated. */ code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, &ialloc_context, - &ip); + &ip, lock_flags); /* * Return an error if we were unable to allocate a new inode. @@ -1071,7 +1072,7 @@ xfs_dir_ialloc( * this call should always succeed. */ code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, - &ialloc_context, &ip); + &ialloc_context, &ip, lock_flags); /* * If we get an error at this point, return to the caller @@ -1210,7 +1211,7 @@ xfs_create( * entry pointing to them, but a directory also the "." entry * pointing to itself. */ - error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip); + error = xfs_dir_ialloc(&tp, dp, mode, is_dir ? 2 : 1, rdev, prid, &ip, XFS_ILOCK_EXCL); if (error) goto out_trans_cancel; @@ -1343,7 +1344,7 @@ xfs_create_tmpfile( if (error) goto out_trans_cancel; - error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip); + error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip, XFS_ILOCK_EXCL); if (error) goto out_trans_cancel; diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index 1eebc53..466f252 100644 --- a/fs/xfs/xfs_inode.h +++ b/fs/xfs/xfs_inode.h @@ -431,7 +431,7 @@ xfs_extlen_t xfs_get_cowextsz_hint(struct xfs_inode *ip); int xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t, xfs_nlink_t, dev_t, prid_t, - struct xfs_inode **); + struct xfs_inode **, int lock_flags); /* from xfs_file.c */ enum xfs_prealloc_flags { diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index ec39ae2..3e68a52 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -787,7 +787,7 @@ xfs_qm_qino_alloc( return error; if (need_alloc) { - error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip); + error = xfs_dir_ialloc(&tp, NULL, S_IFREG, 1, 0, 0, ip, XFS_ILOCK_EXCL); if (error) { xfs_trans_cancel(tp); return error; diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c index b1d3301..ce8dbea 100644 --- a/fs/xfs/xfs_symlink.c +++ b/fs/xfs/xfs_symlink.c @@ -264,7 +264,7 @@ xfs_symlink( * Allocate an inode for the symlink. */ error = xfs_dir_ialloc(&tp, dp, S_IFLNK | (mode & ~S_IFMT), 1, 0, - prid, &ip); + prid, &ip, XFS_ILOCK_EXCL); if (error) goto out_trans_cancel;
Add lock_flags to xfs_ialloc and xfs_dir_ialloc to control whick locks are released by xfs_trans_ijoin. We will need this later in defered parent pointers Signed-off-by: Allison Henderson <allison.henderson@oracle.com> --- fs/xfs/xfs_inode.c | 17 +++++++++-------- fs/xfs/xfs_inode.h | 2 +- fs/xfs/xfs_qm.c | 2 +- fs/xfs/xfs_symlink.c | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-)