diff mbox series

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

Message ID 158864113397.182683.5812513715201193839.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs: refactor log recovery | expand

Commit Message

Darrick J. Wong May 5, 2020, 1:12 a.m. UTC 
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.

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(-)

Comments

Chandan Babu R May 5, 2020, 9:29 a.m. UTC | #1 
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.

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 Babu R May 5, 2020, 9:29 a.m. UTC | #2 
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__ */
> > 
> > 
> 
> 
>
Christoph Hellwig May 6, 2020, 3:19 p.m. UTC | #3 
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

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__ */