| Message ID | 160125007449.174438.15988765709988942671.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: fix how we deal with new intents during recovery | expand |
On Sun, Sep 27, 2020 at 04:41:14PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Remove this one-line helper. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/libxfs/xfs_defer.c | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > > diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c > index 29e9762f3b77..36c103c14bc9 100644 > --- a/fs/xfs/libxfs/xfs_defer.c > +++ b/fs/xfs/libxfs/xfs_defer.c > @@ -312,22 +312,6 @@ xfs_defer_trans_roll( > return error; > } > > -/* > - * Reset an already used dfops after finish. > - */ > -static void > -xfs_defer_reset( > - struct xfs_trans *tp) > -{ > - ASSERT(list_empty(&tp->t_dfops)); > - > - /* > - * Low mode state transfers across transaction rolls to mirror dfops > - * lifetime. Clear it now that dfops is reset. > - */ > - tp->t_flags &= ~XFS_TRANS_LOWMODE; > -} > - > /* > * Free up any items left in the list. > */ > @@ -477,7 +461,10 @@ xfs_defer_finish( > return error; > } > } > - xfs_defer_reset(*tp); > + > + /* Reset LOWMODE now that we've finished all the dfops. */ > + ASSERT(list_empty(&(*tp)->t_dfops)); > + (*tp)->t_flags &= ~XFS_TRANS_LOWMODE; > return 0; > } > > @@ -551,8 +538,7 @@ xfs_defer_move( > * that behavior. > */ > dtp->t_flags |= (stp->t_flags & XFS_TRANS_LOWMODE); > - > - xfs_defer_reset(stp); > + stp->t_flags &= ~XFS_TRANS_LOWMODE; > } > > /* looks fine. Reviewed-by: Dave Chinner <dchinner@redhat.com>
On Sun, Sep 27, 2020 at 04:41:14PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Remove this one-line helper. Maybe expand the rationale here a little more? Otherwise looks fine: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Mon, Sep 28, 2020 at 08:20:34AM +0200, Christoph Hellwig wrote: > On Sun, Sep 27, 2020 at 04:41:14PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Remove this one-line helper. > > Maybe expand the rationale here a little more? I'll change it to: "Remove this one-line helper since the assert is trivially true in one call site and the rest just obscures a bitmask operation." --D > Otherwise looks fine: > > Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c index 29e9762f3b77..36c103c14bc9 100644 --- a/fs/xfs/libxfs/xfs_defer.c +++ b/fs/xfs/libxfs/xfs_defer.c @@ -312,22 +312,6 @@ xfs_defer_trans_roll( return error; } -/* - * Reset an already used dfops after finish. - */ -static void -xfs_defer_reset( - struct xfs_trans *tp) -{ - ASSERT(list_empty(&tp->t_dfops)); - - /* - * Low mode state transfers across transaction rolls to mirror dfops - * lifetime. Clear it now that dfops is reset. - */ - tp->t_flags &= ~XFS_TRANS_LOWMODE; -} - /* * Free up any items left in the list. */ @@ -477,7 +461,10 @@ xfs_defer_finish( return error; } } - xfs_defer_reset(*tp); + + /* Reset LOWMODE now that we've finished all the dfops. */ + ASSERT(list_empty(&(*tp)->t_dfops)); + (*tp)->t_flags &= ~XFS_TRANS_LOWMODE; return 0; } @@ -551,8 +538,7 @@ xfs_defer_move( * that behavior. */ dtp->t_flags |= (stp->t_flags & XFS_TRANS_LOWMODE); - - xfs_defer_reset(stp); + stp->t_flags &= ~XFS_TRANS_LOWMODE; } /*