| Message ID | 160140140527.830233.8494766872686671838.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: fix how we deal with new intents during recovery | expand |
On Tue, Sep 29, 2020 at 10:43:25AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The ->iop_recover method of a log intent item removes the recovered > intent item from the AIL by logging an intent done item and committing > the transaction, so it's superfluous to have this flag check. Nothing > else uses it, so get rid of the flag entirely. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_log_recover.c | 8 +++----- > fs/xfs/xfs_trans.h | 4 +--- > 2 files changed, 4 insertions(+), 8 deletions(-) > > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index e0675071b39e..84f876c6d498 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2539,11 +2539,9 @@ xlog_recover_process_intents( > * this routine or else those subsequent intents will get > * replayed in the wrong order! > */ > - if (!test_and_set_bit(XFS_LI_RECOVERED, &lip->li_flags)) { > - spin_unlock(&ailp->ail_lock); > - error = lip->li_ops->iop_recover(lip, parent_tp); > - spin_lock(&ailp->ail_lock); > - } > + spin_unlock(&ailp->ail_lock); > + error = lip->li_ops->iop_recover(lip, parent_tp); > + spin_lock(&ailp->ail_lock); > if (error) > goto out; > lip = xfs_trans_ail_cursor_next(ailp, &cur); > diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h > index a71b4f443e39..ced62a35a62b 100644 > --- a/fs/xfs/xfs_trans.h > +++ b/fs/xfs/xfs_trans.h > @@ -55,14 +55,12 @@ struct xfs_log_item { > #define XFS_LI_ABORTED 1 > #define XFS_LI_FAILED 2 > #define XFS_LI_DIRTY 3 /* log item dirty in transaction */ > -#define XFS_LI_RECOVERED 4 /* log intent item has been recovered */ > > #define XFS_LI_FLAGS \ > { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \ > { (1 << XFS_LI_ABORTED), "ABORTED" }, \ > { (1 << XFS_LI_FAILED), "FAILED" }, \ > - { (1 << XFS_LI_DIRTY), "DIRTY" }, \ > - { (1 << XFS_LI_RECOVERED), "RECOVERED" } > + { (1 << XFS_LI_DIRTY), "DIRTY" } > > struct xfs_item_ops { > unsigned flags; >
On Tue, Sep 29, 2020 at 10:43:25AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The ->iop_recover method of a log intent item removes the recovered > intent item from the AIL by logging an intent done item and committing > the transaction, so it's superfluous to have this flag check. Nothing > else uses it, so get rid of the flag entirely. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index e0675071b39e..84f876c6d498 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2539,11 +2539,9 @@ xlog_recover_process_intents( * this routine or else those subsequent intents will get * replayed in the wrong order! */ - if (!test_and_set_bit(XFS_LI_RECOVERED, &lip->li_flags)) { - spin_unlock(&ailp->ail_lock); - error = lip->li_ops->iop_recover(lip, parent_tp); - spin_lock(&ailp->ail_lock); - } + spin_unlock(&ailp->ail_lock); + error = lip->li_ops->iop_recover(lip, parent_tp); + spin_lock(&ailp->ail_lock); if (error) goto out; lip = xfs_trans_ail_cursor_next(ailp, &cur); diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index a71b4f443e39..ced62a35a62b 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -55,14 +55,12 @@ struct xfs_log_item { #define XFS_LI_ABORTED 1 #define XFS_LI_FAILED 2 #define XFS_LI_DIRTY 3 /* log item dirty in transaction */ -#define XFS_LI_RECOVERED 4 /* log intent item has been recovered */ #define XFS_LI_FLAGS \ { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \ { (1 << XFS_LI_ABORTED), "ABORTED" }, \ { (1 << XFS_LI_FAILED), "FAILED" }, \ - { (1 << XFS_LI_DIRTY), "DIRTY" }, \ - { (1 << XFS_LI_RECOVERED), "RECOVERED" } + { (1 << XFS_LI_DIRTY), "DIRTY" } struct xfs_item_ops { unsigned flags;