| Message ID | 158864119233.182683.339682087935092440.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: refactor log recovery | expand |
On Tuesday 5 May 2020 6:43:12 AM IST Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The only purpose of XFS_LI_RECOVERED is to prevent log recovery from > trying to replay recovered intents more than once. Therefore, we can > move the bit setting up to the ->iop_recover caller. > The functionality is the same as was before applying this patch. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_bmap_item.c | 5 ----- > fs/xfs/xfs_extfree_item.c | 4 ---- > fs/xfs/xfs_log_recover.c | 2 +- > fs/xfs/xfs_refcount_item.c | 4 ---- > fs/xfs/xfs_rmap_item.c | 4 ---- > 5 files changed, 1 insertion(+), 18 deletions(-) > > > diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c > index 8f0dc6d550d1..0793c317defb 100644 > --- a/fs/xfs/xfs_bmap_item.c > +++ b/fs/xfs/xfs_bmap_item.c > @@ -442,11 +442,8 @@ xfs_bui_item_recover( > int whichfork; > int error = 0; > > - ASSERT(!test_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags)); > - > /* Only one mapping operation per BUI... */ > if (buip->bui_format.bui_nextents != XFS_BUI_MAX_FAST_EXTENTS) { > - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); > xfs_bui_release(buip); > return -EFSCORRUPTED; > } > @@ -480,7 +477,6 @@ xfs_bui_item_recover( > * This will pull the BUI from the AIL and > * free the memory associated with it. > */ > - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); > xfs_bui_release(buip); > return -EFSCORRUPTED; > } > @@ -538,7 +534,6 @@ xfs_bui_item_recover( > xfs_bmap_unmap_extent(tp, ip, &irec); > } > > - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); > xfs_defer_move(parent_tp, tp); > error = xfs_trans_commit(tp); > xfs_iunlock(ip, XFS_ILOCK_EXCL); > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c > index ec8a79fe6cab..b92678bede24 100644 > --- a/fs/xfs/xfs_extfree_item.c > +++ b/fs/xfs/xfs_extfree_item.c > @@ -594,8 +594,6 @@ xfs_efi_item_recover( > int i; > int error = 0; > > - ASSERT(!test_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags)); > - > /* > * First check the validity of the extents described by the > * EFI. If any are bad, then assume that all are bad and > @@ -613,7 +611,6 @@ xfs_efi_item_recover( > * This will pull the EFI from the AIL and > * free the memory associated with it. > */ > - set_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags); > xfs_efi_release(efip); > return -EFSCORRUPTED; > } > @@ -634,7 +631,6 @@ xfs_efi_item_recover( > > } > > - set_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags); > error = xfs_trans_commit(tp); > return error; > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 8ff957da2845..a49435db3be0 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2717,7 +2717,7 @@ xlog_recover_process_intents( > * this routine or else those subsequent intents will get > * replayed in the wrong order! > */ > - if (!test_bit(XFS_LI_RECOVERED, &lip->li_flags)) { > + 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); > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c > index fab821fce76b..e6d355a09bb3 100644 > --- a/fs/xfs/xfs_refcount_item.c > +++ b/fs/xfs/xfs_refcount_item.c > @@ -442,8 +442,6 @@ xfs_cui_item_recover( > int i; > int error = 0; > > - ASSERT(!test_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags)); > - > /* > * First check the validity of the extents described by the > * CUI. If any are bad, then assume that all are bad and > @@ -473,7 +471,6 @@ xfs_cui_item_recover( > * This will pull the CUI from the AIL and > * free the memory associated with it. > */ > - set_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags); > xfs_cui_release(cuip); > return -EFSCORRUPTED; > } > @@ -557,7 +554,6 @@ xfs_cui_item_recover( > } > > xfs_refcount_finish_one_cleanup(tp, rcur, error); > - set_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags); > xfs_defer_move(parent_tp, tp); > error = xfs_trans_commit(tp); > return error; > diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c > index c9233a220551..4a5e2b1cf75a 100644 > --- a/fs/xfs/xfs_rmap_item.c > +++ b/fs/xfs/xfs_rmap_item.c > @@ -482,8 +482,6 @@ xfs_rui_item_recover( > int whichfork; > int error = 0; > > - ASSERT(!test_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags)); > - > /* > * First check the validity of the extents described by the > * RUI. If any are bad, then assume that all are bad and > @@ -517,7 +515,6 @@ xfs_rui_item_recover( > * This will pull the RUI from the AIL and > * free the memory associated with it. > */ > - set_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags); > xfs_rui_release(ruip); > return -EFSCORRUPTED; > } > @@ -575,7 +572,6 @@ xfs_rui_item_recover( > } > > xfs_rmap_finish_one_cleanup(tp, rcur, error); > - set_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags); > error = xfs_trans_commit(tp); > return error; > > >
On Mon, May 04, 2020 at 06:13:12PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > The only purpose of XFS_LI_RECOVERED is to prevent log recovery from > trying to replay recovered intents more than once. Therefore, we can > move the bit setting up to the ->iop_recover caller. > > 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_bmap_item.c b/fs/xfs/xfs_bmap_item.c index 8f0dc6d550d1..0793c317defb 100644 --- a/fs/xfs/xfs_bmap_item.c +++ b/fs/xfs/xfs_bmap_item.c @@ -442,11 +442,8 @@ xfs_bui_item_recover( int whichfork; int error = 0; - ASSERT(!test_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags)); - /* Only one mapping operation per BUI... */ if (buip->bui_format.bui_nextents != XFS_BUI_MAX_FAST_EXTENTS) { - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); xfs_bui_release(buip); return -EFSCORRUPTED; } @@ -480,7 +477,6 @@ xfs_bui_item_recover( * This will pull the BUI from the AIL and * free the memory associated with it. */ - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); xfs_bui_release(buip); return -EFSCORRUPTED; } @@ -538,7 +534,6 @@ xfs_bui_item_recover( xfs_bmap_unmap_extent(tp, ip, &irec); } - set_bit(XFS_LI_RECOVERED, &buip->bui_item.li_flags); xfs_defer_move(parent_tp, tp); error = xfs_trans_commit(tp); xfs_iunlock(ip, XFS_ILOCK_EXCL); diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c index ec8a79fe6cab..b92678bede24 100644 --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -594,8 +594,6 @@ xfs_efi_item_recover( int i; int error = 0; - ASSERT(!test_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags)); - /* * First check the validity of the extents described by the * EFI. If any are bad, then assume that all are bad and @@ -613,7 +611,6 @@ xfs_efi_item_recover( * This will pull the EFI from the AIL and * free the memory associated with it. */ - set_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags); xfs_efi_release(efip); return -EFSCORRUPTED; } @@ -634,7 +631,6 @@ xfs_efi_item_recover( } - set_bit(XFS_LI_RECOVERED, &efip->efi_item.li_flags); error = xfs_trans_commit(tp); return error; diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 8ff957da2845..a49435db3be0 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2717,7 +2717,7 @@ xlog_recover_process_intents( * this routine or else those subsequent intents will get * replayed in the wrong order! */ - if (!test_bit(XFS_LI_RECOVERED, &lip->li_flags)) { + 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); diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index fab821fce76b..e6d355a09bb3 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -442,8 +442,6 @@ xfs_cui_item_recover( int i; int error = 0; - ASSERT(!test_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags)); - /* * First check the validity of the extents described by the * CUI. If any are bad, then assume that all are bad and @@ -473,7 +471,6 @@ xfs_cui_item_recover( * This will pull the CUI from the AIL and * free the memory associated with it. */ - set_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags); xfs_cui_release(cuip); return -EFSCORRUPTED; } @@ -557,7 +554,6 @@ xfs_cui_item_recover( } xfs_refcount_finish_one_cleanup(tp, rcur, error); - set_bit(XFS_LI_RECOVERED, &cuip->cui_item.li_flags); xfs_defer_move(parent_tp, tp); error = xfs_trans_commit(tp); return error; diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c index c9233a220551..4a5e2b1cf75a 100644 --- a/fs/xfs/xfs_rmap_item.c +++ b/fs/xfs/xfs_rmap_item.c @@ -482,8 +482,6 @@ xfs_rui_item_recover( int whichfork; int error = 0; - ASSERT(!test_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags)); - /* * First check the validity of the extents described by the * RUI. If any are bad, then assume that all are bad and @@ -517,7 +515,6 @@ xfs_rui_item_recover( * This will pull the RUI from the AIL and * free the memory associated with it. */ - set_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags); xfs_rui_release(ruip); return -EFSCORRUPTED; } @@ -575,7 +572,6 @@ xfs_rui_item_recover( } xfs_rmap_finish_one_cleanup(tp, rcur, error); - set_bit(XFS_LI_RECOVERED, &ruip->rui_item.li_flags); error = xfs_trans_commit(tp); return error;