| Message ID | 158864111362.182683.13426589599394215389.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: refactor log recovery | expand |
On Tuesday 5 May 2020 6:41:53 AM IST Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Quotaoff doesn't actually do anything, so take advantage of the > commit_pass2 pointer being optional and get rid of the switch > statement clause. > If we did have an invalid item the check in xlog_recover_commit_trans() would have caught it. Hence we don't require yet another invalid item type check. Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_dquot_item_recover.c | 1 + > fs/xfs/xfs_log_recover.c | 33 ++++++--------------------------- > 2 files changed, 7 insertions(+), 27 deletions(-) > > > diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c > index 07ff943972a3..a07c1c8344d8 100644 > --- a/fs/xfs/xfs_dquot_item_recover.c > +++ b/fs/xfs/xfs_dquot_item_recover.c > @@ -197,4 +197,5 @@ xlog_recover_quotaoff_commit_pass1( > const struct xlog_recover_item_ops xlog_quotaoff_item_ops = { > .item_type = XFS_LI_QUOTAOFF, > .commit_pass1 = xlog_recover_quotaoff_commit_pass1, > + .commit_pass2 = NULL, /* nothing to do in pass2 */ > }; > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index a5158e9e0662..929e2caeeb42 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2034,31 +2034,6 @@ xlog_buf_readahead( > xfs_buf_readahead(log->l_mp->m_ddev_targp, blkno, len, ops); > } > > -STATIC int > -xlog_recover_commit_pass2( > - struct xlog *log, > - struct xlog_recover *trans, > - struct list_head *buffer_list, > - struct xlog_recover_item *item) > -{ > - trace_xfs_log_recover_item_recover(log, trans, item, XLOG_RECOVER_PASS2); > - > - if (item->ri_ops && item->ri_ops->commit_pass2) > - return item->ri_ops->commit_pass2(log, buffer_list, item, > - trans->r_lsn); > - > - switch (ITEM_TYPE(item)) { > - case XFS_LI_QUOTAOFF: > - /* nothing to do in pass2 */ > - return 0; > - default: > - xfs_warn(log->l_mp, "%s: invalid item type (%d)", > - __func__, ITEM_TYPE(item)); > - ASSERT(0); > - return -EFSCORRUPTED; > - } > -} > - > STATIC int > xlog_recover_items_pass2( > struct xlog *log, > @@ -2070,8 +2045,12 @@ xlog_recover_items_pass2( > int error = 0; > > list_for_each_entry(item, item_list, ri_list) { > - error = xlog_recover_commit_pass2(log, trans, > - buffer_list, item); > + trace_xfs_log_recover_item_recover(log, trans, item, > + XLOG_RECOVER_PASS2); > + > + if (item->ri_ops->commit_pass2) > + error = item->ri_ops->commit_pass2(log, buffer_list, > + item, trans->r_lsn); > if (error) > return error; > } > >
> diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c > index 07ff943972a3..a07c1c8344d8 100644 > --- a/fs/xfs/xfs_dquot_item_recover.c > +++ b/fs/xfs/xfs_dquot_item_recover.c > @@ -197,4 +197,5 @@ xlog_recover_quotaoff_commit_pass1( > const struct xlog_recover_item_ops xlog_quotaoff_item_ops = { > .item_type = XFS_LI_QUOTAOFF, > .commit_pass1 = xlog_recover_quotaoff_commit_pass1, > + .commit_pass2 = NULL, /* nothing to do in pass2 */ No need to initialize 0 or NULL fields in static structures. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, May 06, 2020 at 08:16:11AM -0700, Christoph Hellwig wrote: > > diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c > > index 07ff943972a3..a07c1c8344d8 100644 > > --- a/fs/xfs/xfs_dquot_item_recover.c > > +++ b/fs/xfs/xfs_dquot_item_recover.c > > @@ -197,4 +197,5 @@ xlog_recover_quotaoff_commit_pass1( > > const struct xlog_recover_item_ops xlog_quotaoff_item_ops = { > > .item_type = XFS_LI_QUOTAOFF, > > .commit_pass1 = xlog_recover_quotaoff_commit_pass1, > > + .commit_pass2 = NULL, /* nothing to do in pass2 */ > > No need to initialize 0 or NULL fields in static structures. Ok, I'll trim this line to only have the comment, to make it explicit that quotaoff does no work for commit_pass2. --D > Otherwise looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/xfs_dquot_item_recover.c b/fs/xfs/xfs_dquot_item_recover.c index 07ff943972a3..a07c1c8344d8 100644 --- a/fs/xfs/xfs_dquot_item_recover.c +++ b/fs/xfs/xfs_dquot_item_recover.c @@ -197,4 +197,5 @@ xlog_recover_quotaoff_commit_pass1( const struct xlog_recover_item_ops xlog_quotaoff_item_ops = { .item_type = XFS_LI_QUOTAOFF, .commit_pass1 = xlog_recover_quotaoff_commit_pass1, + .commit_pass2 = NULL, /* nothing to do in pass2 */ }; diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index a5158e9e0662..929e2caeeb42 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2034,31 +2034,6 @@ xlog_buf_readahead( xfs_buf_readahead(log->l_mp->m_ddev_targp, blkno, len, ops); } -STATIC int -xlog_recover_commit_pass2( - struct xlog *log, - struct xlog_recover *trans, - struct list_head *buffer_list, - struct xlog_recover_item *item) -{ - trace_xfs_log_recover_item_recover(log, trans, item, XLOG_RECOVER_PASS2); - - if (item->ri_ops && item->ri_ops->commit_pass2) - return item->ri_ops->commit_pass2(log, buffer_list, item, - trans->r_lsn); - - switch (ITEM_TYPE(item)) { - case XFS_LI_QUOTAOFF: - /* nothing to do in pass2 */ - return 0; - default: - xfs_warn(log->l_mp, "%s: invalid item type (%d)", - __func__, ITEM_TYPE(item)); - ASSERT(0); - return -EFSCORRUPTED; - } -} - STATIC int xlog_recover_items_pass2( struct xlog *log, @@ -2070,8 +2045,12 @@ xlog_recover_items_pass2( int error = 0; list_for_each_entry(item, item_list, ri_list) { - error = xlog_recover_commit_pass2(log, trans, - buffer_list, item); + trace_xfs_log_recover_item_recover(log, trans, item, + XLOG_RECOVER_PASS2); + + if (item->ri_ops->commit_pass2) + error = item->ri_ops->commit_pass2(log, buffer_list, + item, trans->r_lsn); if (error) return error; }