On Wed, Feb 24, 2021 at 05:34:57PM +1100, Dave Chinner wrote:

From: Dave Chinner <redacted>

Handle writing of a logvec chain into an iclog that doesn't have
enough space to fit it all. The iclog has already been changed to
WANT_SYNC by xlog_get_iclog_space(), so the entire remaining space
in the iclog is exclusively owned by this logvec chain.

The difference between the single and partial cases is that
we end up with partial iovec writes in the iclog and have to split
a log vec regions across two iclogs. The state handling for this is
currently awful and so we're building up the pieces needed to
handle this more cleanly one at a time.

I did not fully grasp the refactoring yet, so just some superficial
ramblings for now:

How is this related to the rest of the patch?  Maybe just add it
in a prep patch?

+	error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
+					   &contwr, &log_offset);
+	if (error)
+		return error;
 
+	/* start_lsn is the LSN of the first iclog written to. */
+	if (start_lsn)
+		*start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
 
+	/*
+	 * iclogs containing commit records or unmount records need
+	 * to issue ordering cache flushes and commit immediately
+	 * to stable storage to guarantee journal vs metadata ordering
+	 * is correctly maintained in the storage media. This will always
+	 * fit in the iclog we have been already been passed.
+	 */
+	if (optype & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)) {
+		iclog->ic_flags |= (XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
+		ASSERT(!contwr);
+	}
 
+	while (lv) {
+		lv = xlog_write_single(lv, ticket, iclog, &log_offset,
+					&len, &record_cnt, &data_cnt);
+		if (!lv)
 			break;
 
+		ASSERT(!(optype & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)));
+		lv = xlog_write_partial(log, lv, ticket, &iclog, &log_offset,
+					&len, &record_cnt, &data_cnt, &contwr);
+		if (IS_ERR(lv)) {
+			error = PTR_ERR(lv);
+			break;
 		}
 	}

Maybe user IS_ERR_OR_NULL and PTR_ERR_OR_ZERO here to catch the NULL
case as well?  e.g.

	for (;;) {
		....
		lv = xlog_write_partial();
		if (IS_ERR_OR_NULL(lv)) {
			error = PTR_ERR_OR_ZERO(lv);
			break;
		}
	}

This seems misplaced.