Re: [PATCH 3/7] xfs: factor a move some code in xfs_log_cil.c

[Allison Henderson <allison.henderson@oracle.com>]

On 9/2/21 2:59 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> In preparation for adding support for intent item whiteouts.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
Looks like a straight forward hoist
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   fs/xfs/xfs_log_cil.c | 109 +++++++++++++++++++++++++------------------
>   1 file changed, 64 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 9488db6c6b21..bd2c8178255e 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -58,6 +58,38 @@ xlog_cil_set_iclog_hdr_count(struct xfs_cil *cil)
>   			(log->l_iclog_size - log->l_iclog_hsize)));
>   }
>   
> +/*
> + * Check if the current log item was first committed in this sequence.
> + * We can't rely on just the log item being in the CIL, we have to check
> + * the recorded commit sequence number.
> + *
> + * Note: for this to be used in a non-racy manner, it has to be called with
> + * CIL flushing locked out. As a result, it should only be used during the
> + * transaction commit process when deciding what to format into the item.
> + */
> +static bool
> +xlog_item_in_current_chkpt(
> +	struct xfs_cil		*cil,
> +	struct xfs_log_item	*lip)
> +{
> +	if (test_bit(XLOG_CIL_EMPTY, &cil->xc_flags))
> +		return false;
> +
> +	/*
> +	 * li_seq is written on the first commit of a log item to record the
> +	 * first checkpoint it is written to. Hence if it is different to the
> +	 * current sequence, we're in a new checkpoint.
> +	 */
> +	return lip->li_seq == cil->xc_ctx->sequence;
> +}
> +
> +bool
> +xfs_log_item_in_current_chkpt(
> +	struct xfs_log_item *lip)
> +{
> +	return xlog_item_in_current_chkpt(lip->li_mountp->m_log->l_cilp, lip);
> +}
> +
>   /*
>    * Unavoidable forward declaration - xlog_cil_push_work() calls
>    * xlog_cil_ctx_alloc() itself.
> @@ -995,6 +1027,37 @@ xlog_cil_order_cmp(
>   	return l1->lv_order_id > l2->lv_order_id;
>   }
>   
> +/*
> + * Build a log vector chain from the current CIL.
> + */
> +static void
> +xlog_cil_build_lv_chain(
> +	struct xfs_cil_ctx	*ctx,
> +	uint32_t		*num_iovecs,
> +	uint32_t		*num_bytes)
> +{
> +
> +	while (!list_empty(&ctx->log_items)) {
> +		struct xfs_log_item	*item;
> +		struct xfs_log_vec	*lv;
> +
> +		item = list_first_entry(&ctx->log_items,
> +					struct xfs_log_item, li_cil);
> +
> +		lv = item->li_lv;
> +		lv->lv_order_id = item->li_order_id;
> +		*num_iovecs += lv->lv_niovecs;
> +		/* we don't write ordered log vectors */
> +		if (lv->lv_buf_len != XFS_LOG_VEC_ORDERED)
> +			*num_bytes += lv->lv_bytes;
> +
> +		list_add_tail(&lv->lv_list, &ctx->lv_chain);
> +		list_del_init(&item->li_cil);
> +		item->li_order_id = 0;
> +		item->li_lv = NULL;
> +	}
> +}
> +
>   /*
>    * Push the Committed Item List to the log.
>    *
> @@ -1017,7 +1080,6 @@ xlog_cil_push_work(
>   		container_of(work, struct xfs_cil_ctx, push_work);
>   	struct xfs_cil		*cil = ctx->cil;
>   	struct xlog		*log = cil->xc_log;
> -	struct xfs_log_vec	*lv;
>   	struct xfs_cil_ctx	*new_ctx;
>   	int			num_iovecs = 0;
>   	int			num_bytes = 0;
> @@ -1116,24 +1178,7 @@ xlog_cil_push_work(
>   				&bdev_flush);
>   
>   	xlog_cil_pcp_aggregate(cil, ctx);
> -
> -	while (!list_empty(&ctx->log_items)) {
> -		struct xfs_log_item	*item;
> -
> -		item = list_first_entry(&ctx->log_items,
> -					struct xfs_log_item, li_cil);
> -		lv = item->li_lv;
> -		lv->lv_order_id = item->li_order_id;
> -		num_iovecs += lv->lv_niovecs;
> -		/* we don't write ordered log vectors */
> -		if (lv->lv_buf_len != XFS_LOG_VEC_ORDERED)
> -			num_bytes += lv->lv_bytes;
> -
> -		list_add_tail(&lv->lv_list, &ctx->lv_chain);
> -		list_del_init(&item->li_cil);
> -		item->li_order_id = 0;
> -		item->li_lv = NULL;
> -	}
> +	xlog_cil_build_lv_chain(ctx, &num_iovecs, &num_bytes);
>   
>   	/*
>   	 * Switch the contexts so we can drop the context lock and move out
> @@ -1612,32 +1657,6 @@ xlog_cil_force_seq(
>   	return 0;
>   }
>   
> -/*
> - * Check if the current log item was first committed in this sequence.
> - * We can't rely on just the log item being in the CIL, we have to check
> - * the recorded commit sequence number.
> - *
> - * Note: for this to be used in a non-racy manner, it has to be called with
> - * CIL flushing locked out. As a result, it should only be used during the
> - * transaction commit process when deciding what to format into the item.
> - */
> -bool
> -xfs_log_item_in_current_chkpt(
> -	struct xfs_log_item *lip)
> -{
> -	struct xfs_cil		*cil = lip->li_mountp->m_log->l_cilp;
> -
> -	if (test_bit(XLOG_CIL_EMPTY, &cil->xc_flags))
> -		return false;
> -
> -	/*
> -	 * li_seq is written on the first commit of a log item to record the
> -	 * first checkpoint it is written to. Hence if it is different to the
> -	 * current sequence, we're in a new checkpoint.
> -	 */
> -	return lip->li_seq == cil->xc_ctx->sequence;
> -}
> -
>   /*
>    * Move dead percpu state to the relevant CIL context structures.
>    *
>