Re: [PATCH 6/8] xfs: separate out setting CIL context LSNs from xlog_write

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jun 17, 2021 at 06:26:15PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> In preparation for moving more CIL context specific functionality
> into these operations.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks fine as a hoist, though I wonder why you didn't do this in patch
4?

--D

> ---
>  fs/xfs/xfs_log.c      | 17 ++---------------
>  fs/xfs/xfs_log_cil.c  | 23 +++++++++++++++++++++++
>  fs/xfs/xfs_log_priv.h |  2 ++
>  3 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index fc0e43c57683..1c214b395223 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2408,21 +2408,8 @@ xlog_write(
>  	if (error)
>  		return error;
>  
> -	/*
> -	 * If we have a CIL context, record the LSN of the iclog we were just
> -	 * granted space to start writing into. If the context doesn't have
> -	 * a start_lsn recorded, then this iclog will contain the start record
> -	 * for the checkpoint. Otherwise this write contains the commit record
> -	 * for the checkpoint.
> -	 */
> -	if (ctx) {
> -		spin_lock(&ctx->cil->xc_push_lock);
> -		if (!ctx->start_lsn)
> -			ctx->start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> -		else
> -			ctx->commit_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> -		spin_unlock(&ctx->cil->xc_push_lock);
> -	}
> +	if (ctx)
> +		xlog_cil_set_ctx_write_state(ctx, iclog);
>  
>  	lv = list_first_entry_or_null(lv_chain, struct xfs_log_vec, lv_list);
>  	while (lv) {
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index f993ec69fc97..2d8d904ffb78 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -783,6 +783,29 @@ xlog_cil_build_trans_hdr(
>  	tic->t_curr_res -= lvhdr->lv_bytes;
>  }
>  
> +/*
> + * Record the LSN of the iclog we were just granted space to start writing into.
> + * If the context doesn't have a start_lsn recorded, then this iclog will
> + * contain the start record for the checkpoint. Otherwise this write contains
> + * the commit record for the checkpoint.
> + */
> +void
> +xlog_cil_set_ctx_write_state(
> +	struct xfs_cil_ctx	*ctx,
> +	struct xlog_in_core	*iclog)
> +{
> +	struct xfs_cil		*cil = ctx->cil;
> +	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +
> +	ASSERT(!ctx->commit_lsn);
> +	spin_lock(&cil->xc_push_lock);
> +	if (!ctx->start_lsn)
> +		ctx->start_lsn = lsn;
> +	else
> +		ctx->commit_lsn = lsn;
> +	spin_unlock(&cil->xc_push_lock);
> +}
> +
>  /*
>   * Ensure that the order of log writes follows checkpoint sequence order. This
>   * relies on the context LSN being zero until the log write has guaranteed the
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index af8a9dfa8068..849ba2eb3483 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -563,6 +563,8 @@ void	xlog_cil_destroy(struct xlog *log);
>  bool	xlog_cil_empty(struct xlog *log);
>  void	xlog_cil_commit(struct xlog *log, struct xfs_trans *tp,
>  			xfs_csn_t *commit_seq, bool regrant);
> +void	xlog_cil_set_ctx_write_state(struct xfs_cil_ctx *ctx,
> +			struct xlog_in_core *iclog);
>  
>  /*
>   * CIL force routines
> -- 
> 2.31.1
>