Re: [PATCH 10/10] xfs: factor out a xlog_write_space_advance helper

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

On Thu, Oct 30, 2025 at 03:49:20PM +0100, Christoph Hellwig wrote:
> Add a new xlog_write_space_advance that returns the current place in the
> iclog that data is written to, and advances the various counters by the
> amount taken from xlog_write_iovec, and also use it xlog_write_partial,
> which open codes the counter adjustments, but misses the asserts.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks decent,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 8b8fdef6414d..511756429336 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1883,21 +1883,31 @@ static inline uint32_t xlog_write_space_left(struct xlog_write_data *data)
>  	return data->iclog->ic_size - data->log_offset;
>  }
>  
> +static void *
> +xlog_write_space_advance(
> +	struct xlog_write_data	*data,
> +	unsigned int		len)
> +{
> +	void			*p = data->iclog->ic_datap + data->log_offset;
> +
> +	ASSERT(xlog_write_space_left(data) >= len);
> +	ASSERT(data->log_offset % sizeof(int32_t) == 0);
> +	ASSERT(len % sizeof(int32_t) == 0);
> +
> +	data->data_cnt += len;
> +	data->log_offset += len;
> +	data->bytes_left -= len;
> +	return p;
> +}
> +
>  static inline void
>  xlog_write_iovec(
>  	struct xlog_write_data	*data,
>  	void			*buf,
>  	uint32_t		buf_len)
>  {
> -	ASSERT(xlog_write_space_left(data) >= buf_len);
> -	ASSERT(data->log_offset % sizeof(int32_t) == 0);
> -	ASSERT(buf_len % sizeof(int32_t) == 0);
> -
> -	memcpy(data->iclog->ic_datap + data->log_offset, buf, buf_len);
> -	data->log_offset += buf_len;
> -	data->bytes_left -= buf_len;
> +	memcpy(xlog_write_space_advance(data, buf_len), buf, buf_len);
>  	data->record_cnt++;
> -	data->data_cnt += buf_len;
>  }
>  
>  /*
> @@ -2038,7 +2048,8 @@ xlog_write_partial(
>  			if (error)
>  				return error;
>  
> -			ophdr = data->iclog->ic_datap + data->log_offset;
> +			ophdr = xlog_write_space_advance(data,
> +					sizeof(struct xlog_op_header));
>  			ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
>  			ophdr->oh_clientid = XFS_TRANSACTION;
>  			ophdr->oh_res2 = 0;
> @@ -2046,9 +2057,6 @@ xlog_write_partial(
>  
>  			data->ticket->t_curr_res -=
>  				sizeof(struct xlog_op_header);
> -			data->log_offset += sizeof(struct xlog_op_header);
> -			data->data_cnt += sizeof(struct xlog_op_header);
> -			data->bytes_left -= sizeof(struct xlog_op_header);
>  
>  			/*
>  			 * If rlen fits in the iclog, then end the region
> -- 
> 2.47.3
> 
>