Re: [PATCH 01/14] xfs: merge xlog_cil_push into xlog_cil_push_work

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, Mar 16, 2020 at 03:42:20PM +0100, Christoph Hellwig wrote:
> xlog_cil_push is only called by xlog_cil_push_work, so merge the two
> functions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks like a simple enough refactoring,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_log_cil.c | 46 +++++++++++++++++---------------------------
>  1 file changed, 18 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 48435cf2aa16..6a6278b8eb2d 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -626,24 +626,26 @@ xlog_cil_process_committed(
>  }
>  
>  /*
> - * Push the Committed Item List to the log. If @push_seq flag is zero, then it
> - * is a background flush and so we can chose to ignore it. Otherwise, if the
> - * current sequence is the same as @push_seq we need to do a flush. If
> - * @push_seq is less than the current sequence, then it has already been
> + * Push the Committed Item List to the log.
> + *
> + * If the current sequence is the same as xc_push_seq we need to do a flush. If
> + * xc_push_seq is less than the current sequence, then it has already been
>   * flushed and we don't need to do anything - the caller will wait for it to
>   * complete if necessary.
>   *
> - * @push_seq is a value rather than a flag because that allows us to do an
> - * unlocked check of the sequence number for a match. Hence we can allows log
> - * forces to run racily and not issue pushes for the same sequence twice. If we
> - * get a race between multiple pushes for the same sequence they will block on
> - * the first one and then abort, hence avoiding needless pushes.
> + * xc_push_seq is checked unlocked against the sequence number for a match.
> + * Hence we can allows log forces to run racily and not issue pushes for the
> + * same sequence twice.  If we get a race between multiple pushes for the same
> + * sequence they will block on the first one and then abort, hence avoiding
> + * needless pushes.
>   */
> -STATIC int
> -xlog_cil_push(
> -	struct xlog		*log)
> +static void
> +xlog_cil_push_work(
> +	struct work_struct	*work)
>  {
> -	struct xfs_cil		*cil = log->l_cilp;
> +	struct xfs_cil		*cil =
> +		container_of(work, struct xfs_cil, xc_push_work);
> +	struct xlog		*log = cil->xc_log;
>  	struct xfs_log_vec	*lv;
>  	struct xfs_cil_ctx	*ctx;
>  	struct xfs_cil_ctx	*new_ctx;
> @@ -657,9 +659,6 @@ xlog_cil_push(
>  	xfs_lsn_t		commit_lsn;
>  	xfs_lsn_t		push_seq;
>  
> -	if (!cil)
> -		return 0;
> -
>  	new_ctx = kmem_zalloc(sizeof(*new_ctx), KM_NOFS);
>  	new_ctx->ticket = xlog_cil_ticket_alloc(log);
>  
> @@ -867,28 +866,19 @@ xlog_cil_push(
>  	spin_unlock(&cil->xc_push_lock);
>  
>  	/* release the hounds! */
> -	return xfs_log_release_iclog(log->l_mp, commit_iclog);
> +	xfs_log_release_iclog(log->l_mp, commit_iclog);
> +	return;
>  
>  out_skip:
>  	up_write(&cil->xc_ctx_lock);
>  	xfs_log_ticket_put(new_ctx->ticket);
>  	kmem_free(new_ctx);
> -	return 0;
> +	return;
>  
>  out_abort_free_ticket:
>  	xfs_log_ticket_put(tic);
>  out_abort:
>  	xlog_cil_committed(ctx, true);
> -	return -EIO;
> -}
> -
> -static void
> -xlog_cil_push_work(
> -	struct work_struct	*work)
> -{
> -	struct xfs_cil		*cil = container_of(work, struct xfs_cil,
> -							xc_push_work);
> -	xlog_cil_push(cil->xc_log);
>  }
>  
>  /*
> -- 
> 2.24.1
>