Re: [PATCH 14/28] xfs: refactor recovered EFI log item playback

Chandan Babu R
2020-05-05T09:03:18Z

On Tuesday 5 May 2020 6:42:01 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.
>

EFI 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_extfree_item.c | 47 +++++++++++++++++++++++++++++++++++----------
> fs/xfs/xfs_extfree_item.h | 5 -----
> fs/xfs/xfs_log_recover.c | 46 ++++----------------------------------------
> fs/xfs/xfs_trans.h | 1 +
> 4 files changed, 42 insertions(+), 57 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index dca098660753..3fc8a9864217 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -28,6 +28,8 @@
> kmem_zone_t *xfs_efi_zone;
> kmem_zone_t *xfs_efd_zone;
>
> +static const struct xfs_item_ops xfs_efi_item_ops;
> +
> static inline struct xfs_efi_log_item *EFI_ITEM(struct xfs_log_item *lip)
> {
> return container_of(lip, struct xfs_efi_log_item, efi_item);
> @@ -51,7 +53,7 @@ xfs_efi_item_free(
> * committed vs unpin operations in bulk insert operations. Hence the reference
> * count to ensure only the last caller frees the EFI.
> */
> -void
> +STATIC void
> xfs_efi_release(
> struct xfs_efi_log_item *efip)
> {
> @@ -141,14 +143,6 @@ xfs_efi_item_release(
> xfs_efi_release(EFI_ITEM(lip));
> }
>
> -static const struct xfs_item_ops xfs_efi_item_ops = {
> - .iop_size = xfs_efi_item_size,
> - .iop_format = xfs_efi_item_format,
> - .iop_unpin = xfs_efi_item_unpin,
> - .iop_release = xfs_efi_item_release,
> -};
> -
> -
> /*
> * Allocate and initialize an efi item with the given number of extents.
> */
> @@ -586,7 +580,7 @@ const struct xfs_defer_op_type xfs_agfl_free_defer_type = {
> * Process an extent free intent item that was recovered from
> * the log. We need to free the extents that it describes.
> */
> -int
> +STATIC int
> xfs_efi_recover(
> struct xfs_mount *mp,
> struct xfs_efi_log_item *efip)
> @@ -647,6 +641,39 @@ xfs_efi_recover(
> return error;
> }
>
> +/* Recover the EFI if necessary. */
> +STATIC int
> +xfs_efi_item_recover(
> + struct xfs_log_item *lip,
> + struct xfs_trans *tp)
> +{
> + struct xfs_ail *ailp = lip->li_ailp;
> + struct xfs_efi_log_item *efip;
> + int error;
> +
> + /*
> + * Skip EFIs that we've already processed.
> + */
> + efip = container_of(lip, struct xfs_efi_log_item, efi_item);
> + if (test_bit(XFS_EFI_RECOVERED, &efip->efi_flags))
> + return 0;
> +
> + spin_unlock(&ailp->ail_lock);
> + error = xfs_efi_recover(tp->t_mountp, efip);
> + spin_lock(&ailp->ail_lock);
> +
> + return error;
> +}
> +
> +static const struct xfs_item_ops xfs_efi_item_ops = {
> + .iop_size = xfs_efi_item_size,
> + .iop_format = xfs_efi_item_format,
> + .iop_unpin = xfs_efi_item_unpin,
> + .iop_release = xfs_efi_item_release,
> + .iop_recover = xfs_efi_item_recover,
> +};
> +
> +
> /*
> * This routine is called to create an in-core extent free intent
> * item from the efi format structure which was logged on disk.
> diff --git a/fs/xfs/xfs_extfree_item.h b/fs/xfs/xfs_extfree_item.h
> index 876e3d237f48..4b2c2c5c5985 100644
> --- a/fs/xfs/xfs_extfree_item.h
> +++ b/fs/xfs/xfs_extfree_item.h
> @@ -78,9 +78,4 @@ struct xfs_efd_log_item {
> extern struct kmem_zone *xfs_efi_zone;
> extern struct kmem_zone *xfs_efd_zone;
>
> -void xfs_efi_release(struct xfs_efi_log_item *);
> -
> -int xfs_efi_recover(struct xfs_mount *mp,
> - struct xfs_efi_log_item *efip);
> -
> #endif /* __XFS_EXTFREE_ITEM_H__ */
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 929e2caeeb42..f12e14719202 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 EFI if necessary. */
> -STATIC int
> -xlog_recover_process_efi(
> - struct xfs_mount *mp,
> - struct xfs_ail *ailp,
> - struct xfs_log_item *lip)
> -{
> - struct xfs_efi_log_item *efip;
> - int error;
> -
> - /*
> - * Skip EFIs that we've already processed.
> - */
> - efip = container_of(lip, struct xfs_efi_log_item, efi_item);
> - if (test_bit(XFS_EFI_RECOVERED, &efip->efi_flags))
> - return 0;
> -
> - spin_unlock(&ailp->ail_lock);
> - error = xfs_efi_recover(mp, efip);
> - spin_lock(&ailp->ail_lock);
> -
> - return error;
> -}
> -
> -/* Release the EFI since we're cancelling everything. */
> -STATIC void
> -xlog_recover_cancel_efi(
> - struct xfs_mount *mp,
> - struct xfs_ail *ailp,
> - struct xfs_log_item *lip)
> -{
> - struct xfs_efi_log_item *efip;
> -
> - efip = container_of(lip, struct xfs_efi_log_item, efi_item);
> -
> - spin_unlock(&ailp->ail_lock);
> - xfs_efi_release(efip);
> - spin_lock(&ailp->ail_lock);
> -}
> -
> /* Recover the RUI if necessary. */
> STATIC int
> xlog_recover_process_rui(
> @@ -2837,7 +2797,7 @@ xlog_recover_process_intents(
> */
> switch (lip->li_type) {
> case XFS_LI_EFI:
> - error = xlog_recover_process_efi(log->l_mp, ailp, lip);
> + error = lip->li_ops->iop_recover(lip, parent_tp);
> break;
> case XFS_LI_RUI:
> error = xlog_recover_process_rui(log->l_mp, ailp, lip);
> @@ -2893,7 +2853,9 @@ xlog_recover_cancel_intents(
>
> switch (lip->li_type) {
> case XFS_LI_EFI:
> - xlog_recover_cancel_efi(log->l_mp, ailp, lip);
> + spin_unlock(&ailp->ail_lock);
> + lip->li_ops->iop_release(lip);
> + spin_lock(&ailp->ail_lock);
> break;
> case XFS_LI_RUI:
> xlog_recover_cancel_rui(log->l_mp, ailp, lip);
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 752c7fef9de7..3f6a79108991 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -77,6 +77,7 @@ struct xfs_item_ops {
> void (*iop_release)(struct xfs_log_item *);
> xfs_lsn_t (*iop_committed)(struct xfs_log_item *, xfs_lsn_t);
> void (*iop_error)(struct xfs_log_item *, xfs_buf_t *);
> + int (*iop_recover)(struct xfs_log_item *lip, struct xfs_trans *tp);
> };
>
> /*
>
>

--
chandan