Re: [PATCH 4/8] xfs: pass a CIL context to xlog_write()

Christoph Hellwig
2021-06-18T14:24:28Z

> + /*
> + * If we have a CIL context, record the LSN of the iclog we were just
> + * granted space to start writing into. If the context doesn't have
> + * a start_lsn recorded, then this iclog will contain the start record
> + * for the checkpoint. Otherwise this write contains the commit record
> + * for the checkpoint.
> + */
> + if (ctx) {
> + spin_lock(&ctx->cil->xc_push_lock);
> + if (!ctx->start_lsn)
> + ctx->start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> + else
> + ctx->commit_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> + spin_unlock(&ctx->cil->xc_push_lock);
> + }

I have to say that having this cil_ctx specific logic that somehow
reverse eingeer what the callers is doing here seems pretty awkware.
To me the logical interface would be to pass a function pointer and
private data except for the performance penalty of indirect calls.

But to make this somewhat bearable I think you should start with the
above block as a helper implemented in xfs_log_cil.c.