Re: [PATCH 3/8] xfs: factor out a helper to write a log_iovec into the iclog

Brian Foster
2021-06-17T16:23:18Z

On Wed, Jun 16, 2021 at 06:32:07PM +0200, Christoph Hellwig wrote:
> Add a new helper to copy the log iovec into the in-core log buffer,
> and open code the handling continuation opheader as a special case.
>

What do you mean by "open code the handling continuation opheader?"

Otherwise looks reasonable to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_log.c | 55 +++++++++++++++++++++++++++---------------------
> 1 file changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 32cb0fc459a364..5b431d53287d2c 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2124,6 +2124,26 @@ xlog_print_trans(
> }
> }
>
> +static inline void
> +xlog_write_iovec(
> + struct xlog_in_core *iclog,
> + uint32_t *log_offset,
> + void *data,
> + uint32_t write_len,
> + int *bytes_left,
> + uint32_t *record_cnt,
> + uint32_t *data_cnt)
> +{
> + ASSERT(*log_offset % sizeof(int32_t) == 0);
> + ASSERT(write_len % sizeof(int32_t) == 0);
> +
> + memcpy(iclog->ic_datap + *log_offset, data, write_len);
> + *log_offset += write_len;
> + *bytes_left -= write_len;
> + (*record_cnt)++;
> + *data_cnt += write_len;
> +}
> +
> /*
> * 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
> @@ -2145,13 +2165,11 @@ xlog_write_single(
> uint32_t *data_cnt)
> {
> struct xfs_log_vec *lv;
> - void *ptr;
> int index;
>
> ASSERT(*log_offset + *len <= iclog->ic_size ||
> iclog->ic_state == XLOG_STATE_WANT_SYNC);
>
> - ptr = iclog->ic_datap + *log_offset;
> for (lv = log_vector;
> !list_entry_is_head(lv, lv_chain, lv_list);
> lv = list_next_entry(lv, lv_list)) {
> @@ -2171,16 +2189,13 @@ xlog_write_single(
> struct xfs_log_iovec *reg = &lv->lv_iovecp[index];
> struct xlog_op_header *ophdr = reg->i_addr;
>
> - ASSERT(reg->i_len % sizeof(int32_t) == 0);
> - ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
> -
> ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> ophdr->oh_len = cpu_to_be32(reg->i_len -
> sizeof(struct xlog_op_header));
> - memcpy(ptr, reg->i_addr, reg->i_len);
> - xlog_write_adv_cnt(&ptr, len, log_offset, reg->i_len);
> - (*record_cnt)++;
> - *data_cnt += reg->i_len;
> +
> + xlog_write_iovec(iclog, log_offset, reg->i_addr,
> + reg->i_len, len, record_cnt,
> + data_cnt);
> }
> }
> if (list_entry_is_head(lv, lv_chain, lv_list))
> @@ -2249,12 +2264,10 @@ xlog_write_partial(
> int error;
>
> /* walk the logvec, copying until we run out of space in the iclog */
> - ptr = iclog->ic_datap + *log_offset;
> for (index = 0; index < lv->lv_niovecs; index++) {
> uint32_t reg_offset = 0;
>
> reg = &lv->lv_iovecp[index];
> - ASSERT(reg->i_len % sizeof(int32_t) == 0);
>
> /*
> * The first region of a continuation must have a non-zero
> @@ -2274,7 +2287,6 @@ xlog_write_partial(
> data_cnt);
> if (error)
> return ERR_PTR(error);
> - ptr = iclog->ic_datap + *log_offset;
> }
>
> ophdr = reg->i_addr;
> @@ -2285,12 +2297,9 @@ xlog_write_partial(
> if (rlen != reg->i_len)
> ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
>
> - ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
> - xlog_verify_dest_ptr(log, ptr);
> - memcpy(ptr, reg->i_addr, rlen);
> - xlog_write_adv_cnt(&ptr, len, log_offset, rlen);
> - (*record_cnt)++;
> - *data_cnt += rlen;
> + xlog_verify_dest_ptr(log, iclog->ic_datap + *log_offset);
> + xlog_write_iovec(iclog, log_offset, reg->i_addr, rlen, len,
> + record_cnt, data_cnt);
>
> /* If we wrote the whole region, move to the next. */
> if (rlen == reg->i_len)
> @@ -2356,12 +2365,10 @@ xlog_write_partial(
> rlen = min_t(uint32_t, rlen, iclog->ic_size - *log_offset);
> ophdr->oh_len = cpu_to_be32(rlen);
>
> - xlog_verify_dest_ptr(log, ptr);
> - memcpy(ptr, reg->i_addr + reg_offset, rlen);
> - xlog_write_adv_cnt(&ptr, len, log_offset, rlen);
> - (*record_cnt)++;
> - *data_cnt += rlen;
> -
> + xlog_verify_dest_ptr(log, iclog->ic_datap + *log_offset);
> + xlog_write_iovec(iclog, log_offset,
> + reg->i_addr + reg_offset, rlen, len,
> + record_cnt, data_cnt);
> } while (ophdr->oh_flags & XLOG_CONTINUE_TRANS);
> }
>
> --
> 2.30.2
>