Re: [PATCH 7/8] xfs: factor out a xlog_write_full_log_vec helper

Brian Foster
2021-06-17T16:24:38Z

On Wed, Jun 16, 2021 at 06:32:11PM +0200, Christoph Hellwig wrote:
> Add a helper to write out an entire log_vec to prepare for additional
> cleanups.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_log.c | 61 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 4afa8ff1a82076..f8df09f37c3b84 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2137,6 +2137,44 @@ xlog_write_iovec(
> *data_cnt += write_len;
> }
>
> +/*
> + * Write a whole log vector into a single iclog which is guaranteed to have
> + * either sufficient space for the entire log vector chain to be written or
> + * exclusive access to the remaining space in the iclog.
> + *
> + * Return the number of iovecs and data written into the iclog.
> + */
> +static void
> +xlog_write_full(
> + struct xfs_log_vec *lv,
> + struct xlog_ticket *ticket,
> + struct xlog_in_core *iclog,
> + uint32_t *log_offset,
> + uint32_t *len,
> + uint32_t *record_cnt,
> + uint32_t *data_cnt)
> +{
> + int i;
> +
> + ASSERT(*log_offset + *len <= iclog->ic_size ||
> + iclog->ic_state == XLOG_STATE_WANT_SYNC);

I suspect this could be lifted into the original caller (xlog_write())
since it doesn't have much to do with the current lv (and this is
eventually no longer called for each lv in the chain)..? Otherwise the
patch LGTM.

Brian

> +
> + /*
> + * Ordered log vectors have no regions to write so this loop will
> + * naturally skip them.
> + */
> + for (i = 0; i < lv->lv_niovecs; i++) {
> + struct xfs_log_iovec *reg = &lv->lv_iovecp[i];
> + struct xlog_op_header *ophdr = reg->i_addr;
> +
> + ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> + ophdr->oh_len =
> + cpu_to_be32(reg->i_len - sizeof(struct xlog_op_header));
> + xlog_write_iovec(iclog, log_offset, reg->i_addr, reg->i_len,
> + len, record_cnt, data_cnt);
> + }
> +}
> +
> /*
> * Write whole log vectors into a single iclog which is guaranteed to have
> * either sufficient space for the entire log vector chain to be written or
> @@ -2158,10 +2196,6 @@ xlog_write_single(
> uint32_t *data_cnt)
> {
> struct xfs_log_vec *lv;
> - int index;
> -
> - ASSERT(*log_offset + *len <= iclog->ic_size ||
> - iclog->ic_state == XLOG_STATE_WANT_SYNC);
>
> for (lv = log_vector;
> !list_entry_is_head(lv, lv_chain, lv_list);
> @@ -2173,23 +2207,8 @@ xlog_write_single(
> if (lv->lv_niovecs &&
> lv->lv_bytes > iclog->ic_size - *log_offset)
> break;
> -
> - /*
> - * Ordered log vectors have no regions to write so this
> - * loop will naturally skip them.
> - */
> - for (index = 0; index < lv->lv_niovecs; index++) {
> - struct xfs_log_iovec *reg = &lv->lv_iovecp[index];
> - struct xlog_op_header *ophdr = reg->i_addr;
> -
> - ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> - ophdr->oh_len = cpu_to_be32(reg->i_len -
> - sizeof(struct xlog_op_header));
> -
> - xlog_write_iovec(iclog, log_offset, reg->i_addr,
> - reg->i_len, len, record_cnt,
> - data_cnt);
> - }
> + xlog_write_full(lv, ticket, iclog, log_offset, len, record_cnt,
> + data_cnt);
> }
> if (list_entry_is_head(lv, lv_chain, lv_list))
> lv = NULL;
> --
> 2.30.2
>