Re: [PATCH 10/13] xfs: introduce xlog_write_single()

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Wed, Feb 24, 2021 at 05:34:56PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Vector out to an optimised version of xlog_write() if the entire

s/Vector/Factor/ ?

> +	ptr = iclog->ic_datap + log_offset;
> +	while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
> +
> +
> +		/* ordered log vectors have no regions to write */

The two empty lines above look a little strange.

> +		if (lv->lv_buf_len == XFS_LOG_VEC_ORDERED) {
> +			ASSERT(lv->lv_niovecs == 0);
> +			goto next_lv;
> +		}
> +
> +		reg = &lv->lv_iovecp[index];
> +		ASSERT(reg->i_len % sizeof(int32_t) == 0);
> +		ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
> +
> +		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));
> +
> +		memcpy(ptr, reg->i_addr, reg->i_len);
> +		xlog_write_adv_cnt(&ptr, &len, &log_offset, reg->i_len);
> +		record_cnt++;
> +
> +		/* move to the next iovec */
> +		if (++index < lv->lv_niovecs)
> +			continue;
> +next_lv:
> +		/* move to the next logvec */
> +		lv = lv->lv_next;
> +		index = 0;
> +	}

I always hated this (pre-existing) loop style.  What do you think of
something like this (just whiteboard coding, might be completely broken),
which also handles the ordered case with lv_niovecs == 0 as part of
the natural loop:

	for (lv = log_vector; lv; lv->lv_next) {
		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;

			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++;
		}
	}