Re: [PATCH 16/28] xfs: refactor recovered CUI log item playback

Chandan Babu R
2020-05-05T09:29:57Z

On Tuesday 5 May 2020 2:59:02 PM IST Chandan Babu R wrote:
> On Tuesday 5 May 2020 6:42:14 AM IST Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Move the code that processes the log items created from the recovered
> > log items into the per-item source code files and use dispatch functions
> > to call them. No functional changes.
> >
>
> RUI log item playback is consistent with what was done before the patch is
> applied.

I meant "CUI log item playback ...".

>
> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/xfs_log_recover.c | 48 ++------------------------------------------
> > fs/xfs/xfs_refcount_item.c | 44 ++++++++++++++++++++++++++++++++--------
> > fs/xfs/xfs_refcount_item.h | 3 ---
> > 3 files changed, 37 insertions(+), 58 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index da66484acaa7..ad5ac97ed0c7 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2553,46 +2553,6 @@ xlog_recover_process_data(
> > return 0;
> > }
> >
> > -/* Recover the CUI if necessary. */
> > -STATIC int
> > -xlog_recover_process_cui(
> > - struct xfs_trans *parent_tp,
> > - struct xfs_ail *ailp,
> > - struct xfs_log_item *lip)
> > -{
> > - struct xfs_cui_log_item *cuip;
> > - int error;
> > -
> > - /*
> > - * Skip CUIs that we've already processed.
> > - */
> > - cuip = container_of(lip, struct xfs_cui_log_item, cui_item);
> > - if (test_bit(XFS_CUI_RECOVERED, &cuip->cui_flags))
> > - return 0;
> > -
> > - spin_unlock(&ailp->ail_lock);
> > - error = xfs_cui_recover(parent_tp, cuip);
> > - spin_lock(&ailp->ail_lock);
> > -
> > - return error;
> > -}
> > -
> > -/* Release the CUI since we're cancelling everything. */
> > -STATIC void
> > -xlog_recover_cancel_cui(
> > - struct xfs_mount *mp,
> > - struct xfs_ail *ailp,
> > - struct xfs_log_item *lip)
> > -{
> > - struct xfs_cui_log_item *cuip;
> > -
> > - cuip = container_of(lip, struct xfs_cui_log_item, cui_item);
> > -
> > - spin_unlock(&ailp->ail_lock);
> > - xfs_cui_release(cuip);
> > - spin_lock(&ailp->ail_lock);
> > -}
> > -
> > /* Recover the BUI if necessary. */
> > STATIC int
> > xlog_recover_process_bui(
> > @@ -2758,10 +2718,8 @@ xlog_recover_process_intents(
> > switch (lip->li_type) {
> > case XFS_LI_EFI:
> > case XFS_LI_RUI:
> > - error = lip->li_ops->iop_recover(lip, parent_tp);
> > - break;
> > case XFS_LI_CUI:
> > - error = xlog_recover_process_cui(parent_tp, ailp, lip);
> > + error = lip->li_ops->iop_recover(lip, parent_tp);
> > break;
> > case XFS_LI_BUI:
> > error = xlog_recover_process_bui(parent_tp, ailp, lip);
> > @@ -2812,13 +2770,11 @@ xlog_recover_cancel_intents(
> > switch (lip->li_type) {
> > case XFS_LI_EFI:
> > case XFS_LI_RUI:
> > + case XFS_LI_CUI:
> > spin_unlock(&ailp->ail_lock);
> > lip->li_ops->iop_release(lip);
> > spin_lock(&ailp->ail_lock);
> > break;
> > - case XFS_LI_CUI:
> > - xlog_recover_cancel_cui(log->l_mp, ailp, lip);
> > - break;
> > case XFS_LI_BUI:
> > xlog_recover_cancel_bui(log->l_mp, ailp, lip);
> > break;
> > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> > index 28b41f5dd6bc..5b72eebd8764 100644
> > --- a/fs/xfs/xfs_refcount_item.c
> > +++ b/fs/xfs/xfs_refcount_item.c
> > @@ -24,6 +24,8 @@
> > kmem_zone_t *xfs_cui_zone;
> > kmem_zone_t *xfs_cud_zone;
> >
> > +static const struct xfs_item_ops xfs_cui_item_ops;
> > +
> > static inline struct xfs_cui_log_item *CUI_ITEM(struct xfs_log_item *lip)
> > {
> > return container_of(lip, struct xfs_cui_log_item, cui_item);
> > @@ -46,7 +48,7 @@ xfs_cui_item_free(
> > * committed vs unpin operations in bulk insert operations. Hence the reference
> > * count to ensure only the last caller frees the CUI.
> > */
> > -void
> > +STATIC void
> > xfs_cui_release(
> > struct xfs_cui_log_item *cuip)
> > {
> > @@ -125,13 +127,6 @@ xfs_cui_item_release(
> > xfs_cui_release(CUI_ITEM(lip));
> > }
> >
> > -static const struct xfs_item_ops xfs_cui_item_ops = {
> > - .iop_size = xfs_cui_item_size,
> > - .iop_format = xfs_cui_item_format,
> > - .iop_unpin = xfs_cui_item_unpin,
> > - .iop_release = xfs_cui_item_release,
> > -};
> > -
> > /*
> > * Allocate and initialize an cui item with the given number of extents.
> > */
> > @@ -425,7 +420,7 @@ const struct xfs_defer_op_type xfs_refcount_update_defer_type = {
> > * Process a refcount update intent item that was recovered from the log.
> > * We need to update the refcountbt.
> > */
> > -int
> > +STATIC int
> > xfs_cui_recover(
> > struct xfs_trans *parent_tp,
> > struct xfs_cui_log_item *cuip)
> > @@ -573,6 +568,37 @@ xfs_cui_recover(
> > return error;
> > }
> >
> > +/* Recover the CUI if necessary. */
> > +STATIC int
> > +xfs_cui_item_recover(
> > + struct xfs_log_item *lip,
> > + struct xfs_trans *tp)
> > +{
> > + struct xfs_ail *ailp = lip->li_ailp;
> > + struct xfs_cui_log_item *cuip = CUI_ITEM(lip);
> > + int error;
> > +
> > + /*
> > + * Skip CUIs that we've already processed.
> > + */
> > + if (test_bit(XFS_CUI_RECOVERED, &cuip->cui_flags))
> > + return 0;
> > +
> > + spin_unlock(&ailp->ail_lock);
> > + error = xfs_cui_recover(tp, cuip);
> > + spin_lock(&ailp->ail_lock);
> > +
> > + return error;
> > +}
> > +
> > +static const struct xfs_item_ops xfs_cui_item_ops = {
> > + .iop_size = xfs_cui_item_size,
> > + .iop_format = xfs_cui_item_format,
> > + .iop_unpin = xfs_cui_item_unpin,
> > + .iop_release = xfs_cui_item_release,
> > + .iop_recover = xfs_cui_item_recover,
> > +};
> > +
> > /*
> > * Copy an CUI format buffer from the given buf, and into the destination
> > * CUI format structure. The CUI/CUD items were designed not to need any
> > diff --git a/fs/xfs/xfs_refcount_item.h b/fs/xfs/xfs_refcount_item.h
> > index ebe12779eaac..cfaa857673a6 100644
> > --- a/fs/xfs/xfs_refcount_item.h
> > +++ b/fs/xfs/xfs_refcount_item.h
> > @@ -77,7 +77,4 @@ struct xfs_cud_log_item {
> > extern struct kmem_zone *xfs_cui_zone;
> > extern struct kmem_zone *xfs_cud_zone;
> >
> > -void xfs_cui_release(struct xfs_cui_log_item *);
> > -int xfs_cui_recover(struct xfs_trans *parent_tp, struct xfs_cui_log_item *cuip);
> > -
> > #endif /* __XFS_REFCOUNT_ITEM_H__ */
> >
> >
>
>
>

--
chandan