cleanup log item formatting v2

cleanup log item formatting v2

[Christoph Hellwig]

Hi all,

I dug into a rabit hole about the log item formatting recently,
and noticed that the handling of the opheaders is still pretty
ugly because it leaks pre-delayed logging implementation
details into the log item implementations.

The core of this series is to remove the to reserve space in the
CIL buffers/shadow buffers for the opheaders that already were
generated more or less on the fly by the lowlevel log write
code anyway, but there's lots of other cleanups around it.

Note that sits on top of the "kill xlog_in_core_2_t v3" because
a struct removal there sits right next to a struct moved here.

Changes since v1:
 - rebased and dropped the already merged patches

Diffstat:
 libxfs/xfs_log_format.h |    7 -
 xfs_attr_item.c         |   27 +---
 xfs_bmap_item.c         |   10 -
 xfs_buf_item.c          |   19 +--
 xfs_dquot_item.c        |    9 -
 xfs_exchmaps_item.c     |   11 -
 xfs_extfree_item.c      |   10 -
 xfs_icreate_item.c      |    6 
 xfs_inode_item.c        |   49 +++-----
 xfs_log.c               |  292 ++++++++++++++++++------------------------------
 xfs_log.h               |   65 +---------
 xfs_log_cil.c           |  111 ++++++++++++++++--
 xfs_log_priv.h          |   20 +++
 xfs_refcount_item.c     |   10 -
 xfs_rmap_item.c         |   10 -
 xfs_trans.h             |    4 
 16 files changed, 313 insertions(+), 347 deletions(-)

[PATCH 01/10] xfs: add a xlog_write_one_vec helper

[Christoph Hellwig]

Add a wrapper for xlog_write for the two callers who need to build a
log_vec and add it to a single-entry chain instead of duplicating the
code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c      | 35 +++++++++++++++++++++--------------
 fs/xfs/xfs_log_cil.c  | 11 +----------
 fs/xfs/xfs_log_priv.h |  2 ++
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index a311385b23d8..ed83a0e3578e 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -848,6 +848,26 @@ xlog_wait_on_iclog(
 	return 0;
 }
 
+int
+xlog_write_one_vec(
+	struct xlog		*log,
+	struct xfs_cil_ctx	*ctx,
+	struct xfs_log_iovec	*reg,
+	struct xlog_ticket	*ticket)
+{
+	struct xfs_log_vec	lv = {
+		.lv_niovecs	= 1,
+		.lv_iovecp	= reg,
+	};
+	LIST_HEAD		(lv_chain);
+
+	/* account for space used by record data */
+	ticket->t_curr_res -= reg->i_len;
+
+	list_add(&lv.lv_list, &lv_chain);
+	return xlog_write(log, ctx, &lv_chain, ticket, reg->i_len);
+}
+
 /*
  * Write out an unmount record using the ticket provided. We have to account for
  * the data space used in the unmount ticket as this write is not done from a
@@ -876,21 +896,8 @@ xlog_write_unmount_record(
 		.i_len = sizeof(unmount_rec),
 		.i_type = XLOG_REG_TYPE_UNMOUNT,
 	};
-	struct xfs_log_vec vec = {
-		.lv_niovecs = 1,
-		.lv_iovecp = &reg,
-	};
-	LIST_HEAD(lv_chain);
-	list_add(&vec.lv_list, &lv_chain);
-
-	BUILD_BUG_ON((sizeof(struct xlog_op_header) +
-		      sizeof(struct xfs_unmount_log_format)) !=
-							sizeof(unmount_rec));
-
-	/* account for space used by record data */
-	ticket->t_curr_res -= sizeof(unmount_rec);
 
-	return xlog_write(log, NULL, &lv_chain, ticket, reg.i_len);
+	return xlog_write_one_vec(log, NULL, &reg, ticket);
 }
 
 /*
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 778ac47adb8c..83aa06e19cfb 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -1098,13 +1098,7 @@ xlog_cil_write_commit_record(
 		.i_len = sizeof(struct xlog_op_header),
 		.i_type = XLOG_REG_TYPE_COMMIT,
 	};
-	struct xfs_log_vec	vec = {
-		.lv_niovecs = 1,
-		.lv_iovecp = &reg,
-	};
 	int			error;
-	LIST_HEAD(lv_chain);
-	list_add(&vec.lv_list, &lv_chain);
 
 	if (xlog_is_shutdown(log))
 		return -EIO;
@@ -1112,10 +1106,7 @@ xlog_cil_write_commit_record(
 	error = xlog_cil_order_write(ctx->cil, ctx->sequence, _COMMIT_RECORD);
 	if (error)
 		return error;
-
-	/* account for space used by record data */
-	ctx->ticket->t_curr_res -= reg.i_len;
-	error = xlog_write(log, ctx, &lv_chain, ctx->ticket, reg.i_len);
+	error = xlog_write_one_vec(log, ctx, &reg, ctx->ticket);
 	if (error)
 		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
 	return error;
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 0fe59f0525aa..d2410e78b7f5 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -507,6 +507,8 @@ void	xlog_print_trans(struct xfs_trans *);
 int	xlog_write(struct xlog *log, struct xfs_cil_ctx *ctx,
 		struct list_head *lv_chain, struct xlog_ticket *tic,
 		uint32_t len);
+int	xlog_write_one_vec(struct xlog *log, struct xfs_cil_ctx *ctx,
+		struct xfs_log_iovec *reg, struct xlog_ticket *ticket);
 void	xfs_log_ticket_ungrant(struct xlog *log, struct xlog_ticket *ticket);
 void	xfs_log_ticket_regrant(struct xlog *log, struct xlog_ticket *ticket);
 
-- 
2.47.3

Re: [PATCH 01/10] xfs: add a xlog_write_one_vec helper

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:11PM +0100, Christoph Hellwig wrote:
> Add a wrapper for xlog_write for the two callers who need to build a
> log_vec and add it to a single-entry chain instead of duplicating the
> code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This looks like a pretty simple hoist...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c      | 35 +++++++++++++++++++++--------------
>  fs/xfs/xfs_log_cil.c  | 11 +----------
>  fs/xfs/xfs_log_priv.h |  2 ++
>  3 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index a311385b23d8..ed83a0e3578e 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -848,6 +848,26 @@ xlog_wait_on_iclog(
>  	return 0;
>  }
>  
> +int
> +xlog_write_one_vec(
> +	struct xlog		*log,
> +	struct xfs_cil_ctx	*ctx,
> +	struct xfs_log_iovec	*reg,
> +	struct xlog_ticket	*ticket)
> +{
> +	struct xfs_log_vec	lv = {
> +		.lv_niovecs	= 1,
> +		.lv_iovecp	= reg,
> +	};
> +	LIST_HEAD		(lv_chain);
> +
> +	/* account for space used by record data */
> +	ticket->t_curr_res -= reg->i_len;
> +
> +	list_add(&lv.lv_list, &lv_chain);
> +	return xlog_write(log, ctx, &lv_chain, ticket, reg->i_len);
> +}
> +
>  /*
>   * Write out an unmount record using the ticket provided. We have to account for
>   * the data space used in the unmount ticket as this write is not done from a
> @@ -876,21 +896,8 @@ xlog_write_unmount_record(
>  		.i_len = sizeof(unmount_rec),
>  		.i_type = XLOG_REG_TYPE_UNMOUNT,
>  	};
> -	struct xfs_log_vec vec = {
> -		.lv_niovecs = 1,
> -		.lv_iovecp = &reg,
> -	};
> -	LIST_HEAD(lv_chain);
> -	list_add(&vec.lv_list, &lv_chain);
> -
> -	BUILD_BUG_ON((sizeof(struct xlog_op_header) +
> -		      sizeof(struct xfs_unmount_log_format)) !=
> -							sizeof(unmount_rec));
> -
> -	/* account for space used by record data */
> -	ticket->t_curr_res -= sizeof(unmount_rec);
>  
> -	return xlog_write(log, NULL, &lv_chain, ticket, reg.i_len);
> +	return xlog_write_one_vec(log, NULL, &reg, ticket);
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 778ac47adb8c..83aa06e19cfb 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -1098,13 +1098,7 @@ xlog_cil_write_commit_record(
>  		.i_len = sizeof(struct xlog_op_header),
>  		.i_type = XLOG_REG_TYPE_COMMIT,
>  	};
> -	struct xfs_log_vec	vec = {
> -		.lv_niovecs = 1,
> -		.lv_iovecp = &reg,
> -	};
>  	int			error;
> -	LIST_HEAD(lv_chain);
> -	list_add(&vec.lv_list, &lv_chain);
>  
>  	if (xlog_is_shutdown(log))
>  		return -EIO;
> @@ -1112,10 +1106,7 @@ xlog_cil_write_commit_record(
>  	error = xlog_cil_order_write(ctx->cil, ctx->sequence, _COMMIT_RECORD);
>  	if (error)
>  		return error;
> -
> -	/* account for space used by record data */
> -	ctx->ticket->t_curr_res -= reg.i_len;
> -	error = xlog_write(log, ctx, &lv_chain, ctx->ticket, reg.i_len);
> +	error = xlog_write_one_vec(log, ctx, &reg, ctx->ticket);
>  	if (error)
>  		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
>  	return error;
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 0fe59f0525aa..d2410e78b7f5 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -507,6 +507,8 @@ void	xlog_print_trans(struct xfs_trans *);
>  int	xlog_write(struct xlog *log, struct xfs_cil_ctx *ctx,
>  		struct list_head *lv_chain, struct xlog_ticket *tic,
>  		uint32_t len);
> +int	xlog_write_one_vec(struct xlog *log, struct xfs_cil_ctx *ctx,
> +		struct xfs_log_iovec *reg, struct xlog_ticket *ticket);
>  void	xfs_log_ticket_ungrant(struct xlog *log, struct xlog_ticket *ticket);
>  void	xfs_log_ticket_regrant(struct xlog *log, struct xlog_ticket *ticket);
>  
> -- 
> 2.47.3
> 
> 

[PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Christoph Hellwig]

lv_bytes is mostly just use by the CIL code, but has crept into the
low-level log writing code to decide on a full or partial iclog
write.  Ensure it is valid even for the special log writes that don't
go through the CIL by initializing it in xlog_write_one_vec.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index ed83a0e3578e..382c55f4d8d2 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -858,14 +858,15 @@ xlog_write_one_vec(
 	struct xfs_log_vec	lv = {
 		.lv_niovecs	= 1,
 		.lv_iovecp	= reg,
+		.lv_bytes	= reg->i_len,
 	};
 	LIST_HEAD		(lv_chain);
 
 	/* account for space used by record data */
-	ticket->t_curr_res -= reg->i_len;
+	ticket->t_curr_res -= lv.lv_bytes;
 
 	list_add(&lv.lv_list, &lv_chain);
-	return xlog_write(log, ctx, &lv_chain, ticket, reg->i_len);
+	return xlog_write(log, ctx, &lv_chain, ticket, lv.lv_bytes);
 }
 
 /*
-- 
2.47.3

Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:12PM +0100, Christoph Hellwig wrote:
> lv_bytes is mostly just use by the CIL code, but has crept into the
> low-level log writing code to decide on a full or partial iclog
> write.  Ensure it is valid even for the special log writes that don't
> go through the CIL by initializing it in xlog_write_one_vec.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index ed83a0e3578e..382c55f4d8d2 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -858,14 +858,15 @@ xlog_write_one_vec(
>  	struct xfs_log_vec	lv = {
>  		.lv_niovecs	= 1,
>  		.lv_iovecp	= reg,
> +		.lv_bytes	= reg->i_len,

I'm surprised that nothing's noticed the zero lv_bytes, but I guess
unmount and commit record writes have always wanted a full write anyway?

Question: if lv_bytes is no longer zero, can this fall into the
xlog_write_partial branch?

--D

>  	};
>  	LIST_HEAD		(lv_chain);
>  
>  	/* account for space used by record data */
> -	ticket->t_curr_res -= reg->i_len;
> +	ticket->t_curr_res -= lv.lv_bytes;
>  
>  	list_add(&lv.lv_list, &lv_chain);
> -	return xlog_write(log, ctx, &lv_chain, ticket, reg->i_len);
> +	return xlog_write(log, ctx, &lv_chain, ticket, lv.lv_bytes);
>  }
>  
>  /*
> -- 
> 2.47.3
> 
> 

Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Christoph Hellwig]

On Fri, Oct 31, 2025 at 05:04:09PM -0700, Darrick J. Wong wrote:
> > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > index ed83a0e3578e..382c55f4d8d2 100644
> > --- a/fs/xfs/xfs_log.c
> > +++ b/fs/xfs/xfs_log.c
> > @@ -858,14 +858,15 @@ xlog_write_one_vec(
> >  	struct xfs_log_vec	lv = {
> >  		.lv_niovecs	= 1,
> >  		.lv_iovecp	= reg,
> > +		.lv_bytes	= reg->i_len,
> 
> I'm surprised that nothing's noticed the zero lv_bytes, but I guess
> unmount and commit record writes have always wanted a full write anyway?
> 
> Question: if lv_bytes is no longer zero, can this fall into the
> xlog_write_partial branch?

The unmount format is smaller than an opheader, so we won't split it
because of that, but unless I'm misreading things it could fix a bug
where we'd not get a new iclog when needed for it otherwise?

The commit record is just an an opheader without any payload, so there
is no way to even split it in theory.

> 

Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Darrick J. Wong]

On Mon, Nov 03, 2025 at 11:43:19AM +0100, Christoph Hellwig wrote:
> On Fri, Oct 31, 2025 at 05:04:09PM -0700, Darrick J. Wong wrote:
> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > > index ed83a0e3578e..382c55f4d8d2 100644
> > > --- a/fs/xfs/xfs_log.c
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -858,14 +858,15 @@ xlog_write_one_vec(
> > >  	struct xfs_log_vec	lv = {
> > >  		.lv_niovecs	= 1,
> > >  		.lv_iovecp	= reg,
> > > +		.lv_bytes	= reg->i_len,
> > 
> > I'm surprised that nothing's noticed the zero lv_bytes, but I guess
> > unmount and commit record writes have always wanted a full write anyway?
> > 
> > Question: if lv_bytes is no longer zero, can this fall into the
> > xlog_write_partial branch?
> 
> The unmount format is smaller than an opheader, so we won't split it
> because of that, but unless I'm misreading things it could fix a bug
> where we'd not get a new iclog when needed for it otherwise?

Yeah, I think that's theoretically possible.  I wonder what that would
look like to generic/388 though?  I haven't seen any problems with log
recovery for a while other than the large xattr thing, so maybe it's not
worth worrying about.

> The commit record is just an an opheader without any payload, so there
> is no way to even split it in theory.

<nod>

--D

Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Christoph Hellwig]

On Tue, Nov 04, 2025 at 03:39:30PM -0800, Darrick J. Wong wrote:
> > The unmount format is smaller than an opheader, so we won't split it
> > because of that, but unless I'm misreading things it could fix a bug
> > where we'd not get a new iclog when needed for it otherwise?
> 
> Yeah, I think that's theoretically possible.  I wonder what that would
> look like to generic/388 though?  I haven't seen any problems with log
> recovery for a while other than the large xattr thing, so maybe it's not
> worth worrying about.

If we don't split a copy we'd need to we'd either overrun the buffer
or something catches it before trying to do that (hopefully).  So I
guess it simply doesn't happen, probably because it is so small that
the accounting slack for the continuation opheaders catches it somehow.

Re: [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec

[Darrick J. Wong]

On Mon, Nov 03, 2025 at 11:43:19AM +0100, Christoph Hellwig wrote:
> On Fri, Oct 31, 2025 at 05:04:09PM -0700, Darrick J. Wong wrote:
> > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > > index ed83a0e3578e..382c55f4d8d2 100644
> > > --- a/fs/xfs/xfs_log.c
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -858,14 +858,15 @@ xlog_write_one_vec(
> > >  	struct xfs_log_vec	lv = {
> > >  		.lv_niovecs	= 1,
> > >  		.lv_iovecp	= reg,
> > > +		.lv_bytes	= reg->i_len,
> > 
> > I'm surprised that nothing's noticed the zero lv_bytes, but I guess
> > unmount and commit record writes have always wanted a full write anyway?
> > 
> > Question: if lv_bytes is no longer zero, can this fall into the
> > xlog_write_partial branch?
> 
> The unmount format is smaller than an opheader, so we won't split it
> because of that, but unless I'm misreading things it could fix a bug
> where we'd not get a new iclog when needed for it otherwise?

Yes, I think you're right about that.  Currently we always call
xlog_write_full on the unmount record, and there's no check that
write_len <= *bytes_left so I guess we just memcpy off the end of the
buffer IF it happens to be the case that the current iclog has exactly
enough space for the unmount ophdr but not the xfs_unmount_log_format.
In that case you'd get the ophdr at the end of the log, but not the
"Un".

Maybe this needs to get turned into a proper bug fix?  Though you'd
think we'd have heard about this by now if it were a real occurrence.
OTOH some people might just xfs_repair -L and get away with it.

> The commit record is just an an opheader without any payload, so there
> is no way to even split it in theory.

Agreed.

--D

[PATCH 03/10] xfs: improve the ->iop_format interface

[Christoph Hellwig]

Export a higher level interface to format log items.  The xlog_format_buf
structure is hidden inside xfs_log_cil.c and only accessed using two
helpers (and a wrapper build on top), hiding details of log iovecs from
the log items.  This also allows simply using an index into lv_iovecp
instead of keeping a cursor vec.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_attr_item.c     |  27 +++++-----
 fs/xfs/xfs_bmap_item.c     |  10 ++--
 fs/xfs/xfs_buf_item.c      |  19 +++----
 fs/xfs/xfs_dquot_item.c    |   9 ++--
 fs/xfs/xfs_exchmaps_item.c |  11 ++--
 fs/xfs/xfs_extfree_item.c  |  10 ++--
 fs/xfs/xfs_icreate_item.c  |   6 +--
 fs/xfs/xfs_inode_item.c    |  49 +++++++++---------
 fs/xfs/xfs_log.c           |  56 ---------------------
 fs/xfs/xfs_log.h           |  53 ++++----------------
 fs/xfs/xfs_log_cil.c       | 100 ++++++++++++++++++++++++++++++++++++-
 fs/xfs/xfs_refcount_item.c |  10 ++--
 fs/xfs/xfs_rmap_item.c     |  10 ++--
 fs/xfs/xfs_trans.h         |   4 +-
 14 files changed, 180 insertions(+), 194 deletions(-)

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index c3a593319bee..02fca5267f53 100644
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -192,10 +192,9 @@ xfs_attri_item_size(
 STATIC void
 xfs_attri_item_format(
 	struct xfs_log_item		*lip,
-	struct xfs_log_vec		*lv)
+	struct xlog_format_buf		*lfb)
 {
 	struct xfs_attri_log_item	*attrip = ATTRI_ITEM(lip);
-	struct xfs_log_iovec		*vecp = NULL;
 	struct xfs_attri_log_nameval	*nv = attrip->attri_nameval;
 
 	attrip->attri_format.alfi_type = XFS_LI_ATTRI;
@@ -220,24 +219,23 @@ xfs_attri_item_format(
 	if (nv->new_value.iov_len > 0)
 		attrip->attri_format.alfi_size++;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTRI_FORMAT,
-			&attrip->attri_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTRI_FORMAT, &attrip->attri_format,
 			sizeof(struct xfs_attri_log_format));
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NAME, nv->name.iov_base,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NAME, nv->name.iov_base,
 			nv->name.iov_len);
 
 	if (nv->new_name.iov_len > 0)
-		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NEWNAME,
-			nv->new_name.iov_base, nv->new_name.iov_len);
+		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NEWNAME,
+				nv->new_name.iov_base, nv->new_name.iov_len);
 
 	if (nv->value.iov_len > 0)
-		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_VALUE,
-			nv->value.iov_base, nv->value.iov_len);
+		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_VALUE,
+				nv->value.iov_base, nv->value.iov_len);
 
 	if (nv->new_value.iov_len > 0)
-		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NEWVALUE,
-			nv->new_value.iov_base, nv->new_value.iov_len);
+		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NEWVALUE,
+				nv->new_value.iov_base, nv->new_value.iov_len);
 }
 
 /*
@@ -322,16 +320,15 @@ xfs_attrd_item_size(
  */
 STATIC void
 xfs_attrd_item_format(
-	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xfs_log_item		*lip,
+	struct xlog_format_buf		*lfb)
 {
 	struct xfs_attrd_log_item	*attrdp = ATTRD_ITEM(lip);
-	struct xfs_log_iovec		*vecp = NULL;
 
 	attrdp->attrd_format.alfd_type = XFS_LI_ATTRD;
 	attrdp->attrd_format.alfd_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTRD_FORMAT,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTRD_FORMAT,
 			&attrdp->attrd_format,
 			sizeof(struct xfs_attrd_log_format));
 }
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 80f0c4bcc483..f38ed63fe86b 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -92,10 +92,9 @@ unsigned int xfs_bui_log_space(unsigned int nr)
 STATIC void
 xfs_bui_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_bui_log_item	*buip = BUI_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&buip->bui_next_extent) ==
 			buip->bui_format.bui_nextents);
@@ -103,7 +102,7 @@ xfs_bui_item_format(
 	buip->bui_format.bui_type = XFS_LI_BUI;
 	buip->bui_format.bui_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_BUI_FORMAT, &buip->bui_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_BUI_FORMAT, &buip->bui_format,
 			xfs_bui_log_format_sizeof(buip->bui_format.bui_nextents));
 }
 
@@ -188,15 +187,14 @@ unsigned int xfs_bud_log_space(void)
 STATIC void
 xfs_bud_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_bud_log_item	*budp = BUD_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	budp->bud_format.bud_type = XFS_LI_BUD;
 	budp->bud_format.bud_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_BUD_FORMAT, &budp->bud_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_BUD_FORMAT, &budp->bud_format,
 			sizeof(struct xfs_bud_log_format));
 }
 
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 8d85b5eee444..c998983ade64 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -263,24 +263,21 @@ xfs_buf_item_size(
 
 static inline void
 xfs_buf_item_copy_iovec(
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp,
+	struct xlog_format_buf	*lfb,
 	struct xfs_buf		*bp,
 	uint			offset,
 	int			first_bit,
 	uint			nbits)
 {
 	offset += first_bit * XFS_BLF_CHUNK;
-	xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BCHUNK,
-			xfs_buf_offset(bp, offset),
+	xlog_format_copy(lfb, XLOG_REG_TYPE_BCHUNK, xfs_buf_offset(bp, offset),
 			nbits * XFS_BLF_CHUNK);
 }
 
 static void
 xfs_buf_item_format_segment(
 	struct xfs_buf_log_item	*bip,
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp,
+	struct xlog_format_buf	*lfb,
 	uint			offset,
 	struct xfs_buf_log_format *blfp)
 {
@@ -308,7 +305,7 @@ xfs_buf_item_format_segment(
 		return;
 	}
 
-	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
+	blfp = xlog_format_copy(lfb, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
 	blfp->blf_size = 1;
 
 	if (bip->bli_flags & XFS_BLI_STALE) {
@@ -331,8 +328,7 @@ xfs_buf_item_format_segment(
 		nbits = xfs_contig_bits(blfp->blf_data_map,
 					blfp->blf_map_size, first_bit);
 		ASSERT(nbits > 0);
-		xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
-					first_bit, nbits);
+		xfs_buf_item_copy_iovec(lfb, bp, offset, first_bit, nbits);
 		blfp->blf_size++;
 
 		/*
@@ -357,11 +353,10 @@ xfs_buf_item_format_segment(
 STATIC void
 xfs_buf_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 	struct xfs_buf		*bp = bip->bli_buf;
-	struct xfs_log_iovec	*vecp = NULL;
 	uint			offset = 0;
 	int			i;
 
@@ -398,7 +393,7 @@ xfs_buf_item_format(
 	}
 
 	for (i = 0; i < bip->bli_format_count; i++) {
-		xfs_buf_item_format_segment(bip, lv, &vecp, offset,
+		xfs_buf_item_format_segment(bip, lfb, offset,
 					    &bip->bli_formats[i]);
 		offset += BBTOB(bp->b_maps[i].bm_len);
 	}
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
index 271b195ebb93..9c2fcfbdf7dc 100644
--- a/fs/xfs/xfs_dquot_item.c
+++ b/fs/xfs/xfs_dquot_item.c
@@ -44,25 +44,24 @@ xfs_qm_dquot_logitem_size(
 STATIC void
 xfs_qm_dquot_logitem_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_disk_dquot	ddq;
 	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 	struct xfs_dq_logformat	*qlf;
 
-	qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT);
+	qlf = xlog_format_start(lfb, XLOG_REG_TYPE_QFORMAT);
 	qlf->qlf_type = XFS_LI_DQUOT;
 	qlf->qlf_size = 2;
 	qlf->qlf_id = qlip->qli_dquot->q_id;
 	qlf->qlf_blkno = qlip->qli_dquot->q_blkno;
 	qlf->qlf_len = 1;
 	qlf->qlf_boffset = qlip->qli_dquot->q_bufoffset;
-	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_dq_logformat));
+	xlog_format_commit(lfb, sizeof(struct xfs_dq_logformat));
 
 	xfs_dquot_to_disk(&ddq, qlip->qli_dquot);
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT, &ddq,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_DQUOT, &ddq,
 			sizeof(struct xfs_disk_dquot));
 }
 
diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
index 229cbe0adf17..10d6fbeff651 100644
--- a/fs/xfs/xfs_exchmaps_item.c
+++ b/fs/xfs/xfs_exchmaps_item.c
@@ -83,16 +83,14 @@ xfs_xmi_item_size(
 STATIC void
 xfs_xmi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_xmi_log_item	*xmi_lip = XMI_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	xmi_lip->xmi_format.xmi_type = XFS_LI_XMI;
 	xmi_lip->xmi_format.xmi_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_XMI_FORMAT,
-			&xmi_lip->xmi_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_XMI_FORMAT, &xmi_lip->xmi_format,
 			sizeof(struct xfs_xmi_log_format));
 }
 
@@ -166,15 +164,14 @@ xfs_xmd_item_size(
 STATIC void
 xfs_xmd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_xmd_log_item	*xmd_lip = XMD_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	xmd_lip->xmd_format.xmd_type = XFS_LI_XMD;
 	xmd_lip->xmd_format.xmd_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_XMD_FORMAT, &xmd_lip->xmd_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_XMD_FORMAT, &xmd_lip->xmd_format,
 			sizeof(struct xfs_xmd_log_format));
 }
 
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index 418ddab590e0..3d1edc43e6fb 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -98,10 +98,9 @@ unsigned int xfs_efi_log_space(unsigned int nr)
 STATIC void
 xfs_efi_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&efip->efi_next_extent) ==
 				efip->efi_format.efi_nextents);
@@ -110,7 +109,7 @@ xfs_efi_item_format(
 	efip->efi_format.efi_type = lip->li_type;
 	efip->efi_format.efi_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFI_FORMAT, &efip->efi_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_EFI_FORMAT, &efip->efi_format,
 			xfs_efi_log_format_sizeof(efip->efi_format.efi_nextents));
 }
 
@@ -277,10 +276,9 @@ unsigned int xfs_efd_log_space(unsigned int nr)
 STATIC void
 xfs_efd_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(efdp->efd_next_extent == efdp->efd_format.efd_nextents);
 	ASSERT(lip->li_type == XFS_LI_EFD || lip->li_type == XFS_LI_EFD_RT);
@@ -288,7 +286,7 @@ xfs_efd_item_format(
 	efdp->efd_format.efd_type = lip->li_type;
 	efdp->efd_format.efd_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFD_FORMAT, &efdp->efd_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_EFD_FORMAT, &efdp->efd_format,
 			xfs_efd_log_format_sizeof(efdp->efd_format.efd_nextents));
 }
 
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index f83ec2bd0583..004dd22393dc 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -49,13 +49,11 @@ xfs_icreate_item_size(
 STATIC void
 xfs_icreate_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICREATE,
-			&icp->ic_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_ICREATE, &icp->ic_format,
 			sizeof(struct xfs_icreate_log));
 }
 
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 1bd411a1114c..54d72234ae32 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -336,8 +336,7 @@ STATIC void
 xfs_inode_item_format_data_fork(
 	struct xfs_inode_log_item *iip,
 	struct xfs_inode_log_format *ilf,
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -354,9 +353,9 @@ xfs_inode_item_format_data_fork(
 
 			ASSERT(xfs_iext_count(&ip->i_df) > 0);
 
-			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IEXT);
+			p = xlog_format_start(lfb, XLOG_REG_TYPE_IEXT);
 			data_bytes = xfs_iextents_copy(ip, p, XFS_DATA_FORK);
-			xlog_finish_iovec(lv, *vecp, data_bytes);
+			xlog_format_commit(lfb, data_bytes);
 
 			ASSERT(data_bytes <= ip->i_df.if_bytes);
 
@@ -374,7 +373,7 @@ xfs_inode_item_format_data_fork(
 		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
 		    ip->i_df.if_broot_bytes > 0) {
 			ASSERT(ip->i_df.if_broot != NULL);
-			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IBROOT,
+			xlog_format_copy(lfb, XLOG_REG_TYPE_IBROOT,
 					ip->i_df.if_broot,
 					ip->i_df.if_broot_bytes);
 			ilf->ilf_dsize = ip->i_df.if_broot_bytes;
@@ -392,8 +391,9 @@ xfs_inode_item_format_data_fork(
 		    ip->i_df.if_bytes > 0) {
 			ASSERT(ip->i_df.if_data != NULL);
 			ASSERT(ip->i_disk_size > 0);
-			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
-					ip->i_df.if_data, ip->i_df.if_bytes);
+			xlog_format_copy(lfb, XLOG_REG_TYPE_ILOCAL,
+					ip->i_df.if_data,
+					ip->i_df.if_bytes);
 			ilf->ilf_dsize = (unsigned)ip->i_df.if_bytes;
 			ilf->ilf_size++;
 		} else {
@@ -416,8 +416,7 @@ STATIC void
 xfs_inode_item_format_attr_fork(
 	struct xfs_inode_log_item *iip,
 	struct xfs_inode_log_format *ilf,
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_inode	*ip = iip->ili_inode;
 	size_t			data_bytes;
@@ -435,9 +434,9 @@ xfs_inode_item_format_attr_fork(
 			ASSERT(xfs_iext_count(&ip->i_af) ==
 				ip->i_af.if_nextents);
 
-			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT);
+			p = xlog_format_start(lfb, XLOG_REG_TYPE_IATTR_EXT);
 			data_bytes = xfs_iextents_copy(ip, p, XFS_ATTR_FORK);
-			xlog_finish_iovec(lv, *vecp, data_bytes);
+			xlog_format_commit(lfb, data_bytes);
 
 			ilf->ilf_asize = data_bytes;
 			ilf->ilf_size++;
@@ -453,7 +452,7 @@ xfs_inode_item_format_attr_fork(
 		    ip->i_af.if_broot_bytes > 0) {
 			ASSERT(ip->i_af.if_broot != NULL);
 
-			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_BROOT,
+			xlog_format_copy(lfb, XLOG_REG_TYPE_IATTR_BROOT,
 					ip->i_af.if_broot,
 					ip->i_af.if_broot_bytes);
 			ilf->ilf_asize = ip->i_af.if_broot_bytes;
@@ -469,8 +468,9 @@ xfs_inode_item_format_attr_fork(
 		if ((iip->ili_fields & XFS_ILOG_ADATA) &&
 		    ip->i_af.if_bytes > 0) {
 			ASSERT(ip->i_af.if_data != NULL);
-			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
-					ip->i_af.if_data, ip->i_af.if_bytes);
+			xlog_format_copy(lfb, XLOG_REG_TYPE_IATTR_LOCAL,
+					ip->i_af.if_data,
+					ip->i_af.if_bytes);
 			ilf->ilf_asize = (unsigned)ip->i_af.if_bytes;
 			ilf->ilf_size++;
 		} else {
@@ -619,14 +619,13 @@ xfs_inode_to_log_dinode(
 static void
 xfs_inode_item_format_core(
 	struct xfs_inode	*ip,
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_log_dinode	*dic;
 
-	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
+	dic = xlog_format_start(lfb, XLOG_REG_TYPE_ICORE);
 	xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
-	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_mount));
+	xlog_format_commit(lfb, xfs_log_dinode_size(ip->i_mount));
 }
 
 /*
@@ -644,14 +643,13 @@ xfs_inode_item_format_core(
 STATIC void
 xfs_inode_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
 	struct xfs_inode	*ip = iip->ili_inode;
-	struct xfs_log_iovec	*vecp = NULL;
 	struct xfs_inode_log_format *ilf;
 
-	ilf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT);
+	ilf = xlog_format_start(lfb, XLOG_REG_TYPE_IFORMAT);
 	ilf->ilf_type = XFS_LI_INODE;
 	ilf->ilf_ino = ip->i_ino;
 	ilf->ilf_blkno = ip->i_imap.im_blkno;
@@ -668,13 +666,12 @@ xfs_inode_item_format(
 	ilf->ilf_asize = 0;
 	ilf->ilf_pad = 0;
 	memset(&ilf->ilf_u, 0, sizeof(ilf->ilf_u));
+	xlog_format_commit(lfb, sizeof(*ilf));
 
-	xlog_finish_iovec(lv, vecp, sizeof(*ilf));
-
-	xfs_inode_item_format_core(ip, lv, &vecp);
-	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
+	xfs_inode_item_format_core(ip, lfb);
+	xfs_inode_item_format_data_fork(iip, ilf, lfb);
 	if (xfs_inode_has_attr_fork(ip)) {
-		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
+		xfs_inode_item_format_attr_fork(iip, ilf, lfb);
 	} else {
 		iip->ili_fields &=
 			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 382c55f4d8d2..93e99d1cc037 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -74,62 +74,6 @@ xlog_iclogs_empty(
 static int
 xfs_log_cover(struct xfs_mount *);
 
-/*
- * We need to make sure the buffer pointer returned is naturally aligned for the
- * biggest basic data type we put into it. We have already accounted for this
- * padding when sizing the buffer.
- *
- * However, this padding does not get written into the log, and hence we have to
- * track the space used by the log vectors separately to prevent log space hangs
- * due to inaccurate accounting (i.e. a leak) of the used log space through the
- * CIL context ticket.
- *
- * We also add space for the xlog_op_header that describes this region in the
- * log. This prepends the data region we return to the caller to copy their data
- * into, so do all the static initialisation of the ophdr now. Because the ophdr
- * is not 8 byte aligned, we have to be careful to ensure that we align the
- * start of the buffer such that the region we return to the call is 8 byte
- * aligned and packed against the tail of the ophdr.
- */
-void *
-xlog_prepare_iovec(
-	struct xfs_log_vec	*lv,
-	struct xfs_log_iovec	**vecp,
-	uint			type)
-{
-	struct xfs_log_iovec	*vec = *vecp;
-	struct xlog_op_header	*oph;
-	uint32_t		len;
-	void			*buf;
-
-	if (vec) {
-		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
-		vec++;
-	} else {
-		vec = &lv->lv_iovecp[0];
-	}
-
-	len = lv->lv_buf_used + sizeof(struct xlog_op_header);
-	if (!IS_ALIGNED(len, sizeof(uint64_t))) {
-		lv->lv_buf_used = round_up(len, sizeof(uint64_t)) -
-					sizeof(struct xlog_op_header);
-	}
-
-	vec->i_type = type;
-	vec->i_addr = lv->lv_buf + lv->lv_buf_used;
-
-	oph = vec->i_addr;
-	oph->oh_clientid = XFS_TRANSACTION;
-	oph->oh_res2 = 0;
-	oph->oh_flags = 0;
-
-	buf = vec->i_addr + sizeof(struct xlog_op_header);
-	ASSERT(IS_ALIGNED((unsigned long)buf, sizeof(uint64_t)));
-
-	*vecp = vec;
-	return buf;
-}
-
 static inline void
 xlog_grant_sub_space(
 	struct xlog_grant_head	*head,
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index dcc1f44ed68f..c4930e925fed 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -6,6 +6,7 @@
 #ifndef	__XFS_LOG_H__
 #define __XFS_LOG_H__
 
+struct xlog_format_buf;
 struct xfs_cil_ctx;
 
 struct xfs_log_vec {
@@ -70,58 +71,24 @@ xlog_calc_iovec_len(int len)
 	return roundup(len, sizeof(uint32_t));
 }
 
-void *xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
-		uint type);
-
-static inline void
-xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec,
-		int data_len)
-{
-	struct xlog_op_header	*oph = vec->i_addr;
-	int			len;
-
-	/*
-	 * Always round up the length to the correct alignment so callers don't
-	 * need to know anything about this log vec layout requirement. This
-	 * means we have to zero the area the data to be written does not cover.
-	 * This is complicated by fact the payload region is offset into the
-	 * logvec region by the opheader that tracks the payload.
-	 */
-	len = xlog_calc_iovec_len(data_len);
-	if (len - data_len != 0) {
-		char	*buf = vec->i_addr + sizeof(struct xlog_op_header);
-
-		memset(buf + data_len, 0, len - data_len);
-	}
-
-	/*
-	 * The opheader tracks aligned payload length, whilst the logvec tracks
-	 * the overall region length.
-	 */
-	oph->oh_len = cpu_to_be32(len);
-
-	len += sizeof(struct xlog_op_header);
-	lv->lv_buf_used += len;
-	lv->lv_bytes += len;
-	vec->i_len = len;
-
-	/* Catch buffer overruns */
-	ASSERT((void *)lv->lv_buf + lv->lv_bytes <=
-		(void *)lv + lv->lv_alloc_size);
-}
+void *xlog_format_start(struct xlog_format_buf *lfb, uint16_t type);
+void xlog_format_commit(struct xlog_format_buf *lfb, unsigned int data_len);
 
 /*
  * Copy the amount of data requested by the caller into a new log iovec.
  */
 static inline void *
-xlog_copy_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
-		uint type, void *data, int len)
+xlog_format_copy(
+	struct xlog_format_buf	*lfb,
+	uint16_t		type,
+	void			*data,
+	unsigned int		len)
 {
 	void *buf;
 
-	buf = xlog_prepare_iovec(lv, vecp, type);
+	buf = xlog_format_start(lfb, type);
 	memcpy(buf, data, len);
-	xlog_finish_iovec(lv, *vecp, len);
+	xlog_format_commit(lfb, len);
 	return buf;
 }
 
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 83aa06e19cfb..bc25012ac5c0 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -409,6 +409,102 @@ xfs_cil_prepare_item(
 		lv->lv_item->li_seq = log->l_cilp->xc_ctx->sequence;
 }
 
+struct xlog_format_buf {
+	struct xfs_log_vec	*lv;
+	unsigned int		idx;
+};
+
+/*
+ * We need to make sure the buffer pointer returned is naturally aligned for the
+ * biggest basic data type we put into it. We have already accounted for this
+ * padding when sizing the buffer.
+ *
+ * However, this padding does not get written into the log, and hence we have to
+ * track the space used by the log vectors separately to prevent log space hangs
+ * due to inaccurate accounting (i.e. a leak) of the used log space through the
+ * CIL context ticket.
+ *
+ * We also add space for the xlog_op_header that describes this region in the
+ * log. This prepends the data region we return to the caller to copy their data
+ * into, so do all the static initialisation of the ophdr now. Because the ophdr
+ * is not 8 byte aligned, we have to be careful to ensure that we align the
+ * start of the buffer such that the region we return to the call is 8 byte
+ * aligned and packed against the tail of the ophdr.
+ */
+void *
+xlog_format_start(
+	struct xlog_format_buf	*lfb,
+	uint16_t		type)
+{
+	struct xfs_log_vec	*lv = lfb->lv;
+	struct xfs_log_iovec	*vec = &lv->lv_iovecp[lfb->idx];
+	struct xlog_op_header	*oph;
+	uint32_t		len;
+	void			*buf;
+
+	ASSERT(lfb->idx < lv->lv_niovecs);
+
+	len = lv->lv_buf_used + sizeof(struct xlog_op_header);
+	if (!IS_ALIGNED(len, sizeof(uint64_t))) {
+		lv->lv_buf_used = round_up(len, sizeof(uint64_t)) -
+					sizeof(struct xlog_op_header);
+	}
+
+	vec->i_type = type;
+	vec->i_addr = lv->lv_buf + lv->lv_buf_used;
+
+	oph = vec->i_addr;
+	oph->oh_clientid = XFS_TRANSACTION;
+	oph->oh_res2 = 0;
+	oph->oh_flags = 0;
+
+	buf = vec->i_addr + sizeof(struct xlog_op_header);
+	ASSERT(IS_ALIGNED((unsigned long)buf, sizeof(uint64_t)));
+	return buf;
+}
+
+void
+xlog_format_commit(
+	struct xlog_format_buf	*lfb,
+	unsigned int		data_len)
+{
+	struct xfs_log_vec	*lv = lfb->lv;
+	struct xfs_log_iovec	*vec = &lv->lv_iovecp[lfb->idx];
+	struct xlog_op_header	*oph = vec->i_addr;
+	int			len;
+
+	/*
+	 * Always round up the length to the correct alignment so callers don't
+	 * need to know anything about this log vec layout requirement. This
+	 * means we have to zero the area the data to be written does not cover.
+	 * This is complicated by fact the payload region is offset into the
+	 * logvec region by the opheader that tracks the payload.
+	 */
+	len = xlog_calc_iovec_len(data_len);
+	if (len - data_len != 0) {
+		char	*buf = vec->i_addr + sizeof(struct xlog_op_header);
+
+		memset(buf + data_len, 0, len - data_len);
+	}
+
+	/*
+	 * The opheader tracks aligned payload length, whilst the logvec tracks
+	 * the overall region length.
+	 */
+	oph->oh_len = cpu_to_be32(len);
+
+	len += sizeof(struct xlog_op_header);
+	lv->lv_buf_used += len;
+	lv->lv_bytes += len;
+	vec->i_len = len;
+
+	/* Catch buffer overruns */
+	ASSERT((void *)lv->lv_buf + lv->lv_bytes <=
+		(void *)lv + lv->lv_alloc_size);
+
+	lfb->idx++;
+}
+
 /*
  * Format log item into a flat buffers
  *
@@ -454,6 +550,7 @@ xlog_cil_insert_format_items(
 	list_for_each_entry(lip, &tp->t_items, li_trans) {
 		struct xfs_log_vec *lv = lip->li_lv;
 		struct xfs_log_vec *shadow = lip->li_lv_shadow;
+		struct xlog_format_buf lfb = { };
 
 		/* Skip items which aren't dirty in this transaction. */
 		if (!test_bit(XFS_LI_DIRTY, &lip->li_flags))
@@ -501,8 +598,9 @@ xlog_cil_insert_format_items(
 			lv->lv_item = lip;
 		}
 
+		lfb.lv = lv;
 		ASSERT(IS_ALIGNED((unsigned long)lv->lv_buf, sizeof(uint64_t)));
-		lip->li_ops->iop_format(lip, lv);
+		lip->li_ops->iop_format(lip, &lfb);
 		xfs_cil_prepare_item(log, lip, lv, diff_len);
 	}
 }
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 3728234699a2..a41f5b577e22 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -93,10 +93,9 @@ unsigned int xfs_cui_log_space(unsigned int nr)
 STATIC void
 xfs_cui_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_cui_log_item	*cuip = CUI_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&cuip->cui_next_extent) ==
 			cuip->cui_format.cui_nextents);
@@ -105,7 +104,7 @@ xfs_cui_item_format(
 	cuip->cui_format.cui_type = lip->li_type;
 	cuip->cui_format.cui_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_CUI_FORMAT, &cuip->cui_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_CUI_FORMAT, &cuip->cui_format,
 			xfs_cui_log_format_sizeof(cuip->cui_format.cui_nextents));
 }
 
@@ -199,17 +198,16 @@ unsigned int xfs_cud_log_space(void)
 STATIC void
 xfs_cud_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_cud_log_item	*cudp = CUD_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(lip->li_type == XFS_LI_CUD || lip->li_type == XFS_LI_CUD_RT);
 
 	cudp->cud_format.cud_type = lip->li_type;
 	cudp->cud_format.cud_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_CUD_FORMAT, &cudp->cud_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_CUD_FORMAT, &cudp->cud_format,
 			sizeof(struct xfs_cud_log_format));
 }
 
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 15f0903f6fd4..8bf04b101156 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -92,10 +92,9 @@ unsigned int xfs_rui_log_space(unsigned int nr)
 STATIC void
 xfs_rui_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_rui_log_item	*ruip = RUI_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(atomic_read(&ruip->rui_next_extent) ==
 			ruip->rui_format.rui_nextents);
@@ -105,7 +104,7 @@ xfs_rui_item_format(
 	ruip->rui_format.rui_type = lip->li_type;
 	ruip->rui_format.rui_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_RUI_FORMAT, &ruip->rui_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_RUI_FORMAT, &ruip->rui_format,
 			xfs_rui_log_format_sizeof(ruip->rui_format.rui_nextents));
 }
 
@@ -200,17 +199,16 @@ unsigned int xfs_rud_log_space(void)
 STATIC void
 xfs_rud_item_format(
 	struct xfs_log_item	*lip,
-	struct xfs_log_vec	*lv)
+	struct xlog_format_buf	*lfb)
 {
 	struct xfs_rud_log_item	*rudp = RUD_ITEM(lip);
-	struct xfs_log_iovec	*vecp = NULL;
 
 	ASSERT(lip->li_type == XFS_LI_RUD || lip->li_type == XFS_LI_RUD_RT);
 
 	rudp->rud_format.rud_type = lip->li_type;
 	rudp->rud_format.rud_size = 1;
 
-	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_RUD_FORMAT, &rudp->rud_format,
+	xlog_format_copy(lfb, XLOG_REG_TYPE_RUD_FORMAT, &rudp->rud_format,
 			sizeof(struct xfs_rud_log_format));
 }
 
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 7fb860f645a3..8830600b3e72 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -9,6 +9,7 @@
 /* kernel only transaction subsystem defines */
 
 struct xlog;
+struct xlog_format_buf;
 struct xfs_buf;
 struct xfs_buftarg;
 struct xfs_efd_log_item;
@@ -70,7 +71,8 @@ struct xfs_log_item {
 struct xfs_item_ops {
 	unsigned flags;
 	void (*iop_size)(struct xfs_log_item *, int *, int *);
-	void (*iop_format)(struct xfs_log_item *, struct xfs_log_vec *);
+	void (*iop_format)(struct xfs_log_item *lip,
+			struct xlog_format_buf *lfb);
 	void (*iop_pin)(struct xfs_log_item *);
 	void (*iop_unpin)(struct xfs_log_item *, int remove);
 	uint64_t (*iop_sort)(struct xfs_log_item *lip);
-- 
2.47.3

Re: [PATCH 03/10] xfs: improve the ->iop_format interface

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:13PM +0100, Christoph Hellwig wrote:
> Export a higher level interface to format log items.  The xlog_format_buf
> structure is hidden inside xfs_log_cil.c and only accessed using two
> helpers (and a wrapper build on top), hiding details of log iovecs from
> the log items.  This also allows simply using an index into lv_iovecp
> instead of keeping a cursor vec.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

That's a nice cleanup!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_attr_item.c     |  27 +++++-----
>  fs/xfs/xfs_bmap_item.c     |  10 ++--
>  fs/xfs/xfs_buf_item.c      |  19 +++----
>  fs/xfs/xfs_dquot_item.c    |   9 ++--
>  fs/xfs/xfs_exchmaps_item.c |  11 ++--
>  fs/xfs/xfs_extfree_item.c  |  10 ++--
>  fs/xfs/xfs_icreate_item.c  |   6 +--
>  fs/xfs/xfs_inode_item.c    |  49 +++++++++---------
>  fs/xfs/xfs_log.c           |  56 ---------------------
>  fs/xfs/xfs_log.h           |  53 ++++----------------
>  fs/xfs/xfs_log_cil.c       | 100 ++++++++++++++++++++++++++++++++++++-
>  fs/xfs/xfs_refcount_item.c |  10 ++--
>  fs/xfs/xfs_rmap_item.c     |  10 ++--
>  fs/xfs/xfs_trans.h         |   4 +-
>  14 files changed, 180 insertions(+), 194 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index c3a593319bee..02fca5267f53 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -192,10 +192,9 @@ xfs_attri_item_size(
>  STATIC void
>  xfs_attri_item_format(
>  	struct xfs_log_item		*lip,
> -	struct xfs_log_vec		*lv)
> +	struct xlog_format_buf		*lfb)
>  {
>  	struct xfs_attri_log_item	*attrip = ATTRI_ITEM(lip);
> -	struct xfs_log_iovec		*vecp = NULL;
>  	struct xfs_attri_log_nameval	*nv = attrip->attri_nameval;
>  
>  	attrip->attri_format.alfi_type = XFS_LI_ATTRI;
> @@ -220,24 +219,23 @@ xfs_attri_item_format(
>  	if (nv->new_value.iov_len > 0)
>  		attrip->attri_format.alfi_size++;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTRI_FORMAT,
> -			&attrip->attri_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTRI_FORMAT, &attrip->attri_format,
>  			sizeof(struct xfs_attri_log_format));
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NAME, nv->name.iov_base,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NAME, nv->name.iov_base,
>  			nv->name.iov_len);
>  
>  	if (nv->new_name.iov_len > 0)
> -		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NEWNAME,
> -			nv->new_name.iov_base, nv->new_name.iov_len);
> +		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NEWNAME,
> +				nv->new_name.iov_base, nv->new_name.iov_len);
>  
>  	if (nv->value.iov_len > 0)
> -		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_VALUE,
> -			nv->value.iov_base, nv->value.iov_len);
> +		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_VALUE,
> +				nv->value.iov_base, nv->value.iov_len);
>  
>  	if (nv->new_value.iov_len > 0)
> -		xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTR_NEWVALUE,
> -			nv->new_value.iov_base, nv->new_value.iov_len);
> +		xlog_format_copy(lfb, XLOG_REG_TYPE_ATTR_NEWVALUE,
> +				nv->new_value.iov_base, nv->new_value.iov_len);
>  }
>  
>  /*
> @@ -322,16 +320,15 @@ xfs_attrd_item_size(
>   */
>  STATIC void
>  xfs_attrd_item_format(
> -	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xfs_log_item		*lip,
> +	struct xlog_format_buf		*lfb)
>  {
>  	struct xfs_attrd_log_item	*attrdp = ATTRD_ITEM(lip);
> -	struct xfs_log_iovec		*vecp = NULL;
>  
>  	attrdp->attrd_format.alfd_type = XFS_LI_ATTRD;
>  	attrdp->attrd_format.alfd_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTRD_FORMAT,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_ATTRD_FORMAT,
>  			&attrdp->attrd_format,
>  			sizeof(struct xfs_attrd_log_format));
>  }
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 80f0c4bcc483..f38ed63fe86b 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -92,10 +92,9 @@ unsigned int xfs_bui_log_space(unsigned int nr)
>  STATIC void
>  xfs_bui_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_bui_log_item	*buip = BUI_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(atomic_read(&buip->bui_next_extent) ==
>  			buip->bui_format.bui_nextents);
> @@ -103,7 +102,7 @@ xfs_bui_item_format(
>  	buip->bui_format.bui_type = XFS_LI_BUI;
>  	buip->bui_format.bui_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_BUI_FORMAT, &buip->bui_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_BUI_FORMAT, &buip->bui_format,
>  			xfs_bui_log_format_sizeof(buip->bui_format.bui_nextents));
>  }
>  
> @@ -188,15 +187,14 @@ unsigned int xfs_bud_log_space(void)
>  STATIC void
>  xfs_bud_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_bud_log_item	*budp = BUD_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	budp->bud_format.bud_type = XFS_LI_BUD;
>  	budp->bud_format.bud_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_BUD_FORMAT, &budp->bud_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_BUD_FORMAT, &budp->bud_format,
>  			sizeof(struct xfs_bud_log_format));
>  }
>  
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 8d85b5eee444..c998983ade64 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -263,24 +263,21 @@ xfs_buf_item_size(
>  
>  static inline void
>  xfs_buf_item_copy_iovec(
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp,
> +	struct xlog_format_buf	*lfb,
>  	struct xfs_buf		*bp,
>  	uint			offset,
>  	int			first_bit,
>  	uint			nbits)
>  {
>  	offset += first_bit * XFS_BLF_CHUNK;
> -	xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BCHUNK,
> -			xfs_buf_offset(bp, offset),
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_BCHUNK, xfs_buf_offset(bp, offset),
>  			nbits * XFS_BLF_CHUNK);
>  }
>  
>  static void
>  xfs_buf_item_format_segment(
>  	struct xfs_buf_log_item	*bip,
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp,
> +	struct xlog_format_buf	*lfb,
>  	uint			offset,
>  	struct xfs_buf_log_format *blfp)
>  {
> @@ -308,7 +305,7 @@ xfs_buf_item_format_segment(
>  		return;
>  	}
>  
> -	blfp = xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
> +	blfp = xlog_format_copy(lfb, XLOG_REG_TYPE_BFORMAT, blfp, base_size);
>  	blfp->blf_size = 1;
>  
>  	if (bip->bli_flags & XFS_BLI_STALE) {
> @@ -331,8 +328,7 @@ xfs_buf_item_format_segment(
>  		nbits = xfs_contig_bits(blfp->blf_data_map,
>  					blfp->blf_map_size, first_bit);
>  		ASSERT(nbits > 0);
> -		xfs_buf_item_copy_iovec(lv, vecp, bp, offset,
> -					first_bit, nbits);
> +		xfs_buf_item_copy_iovec(lfb, bp, offset, first_bit, nbits);
>  		blfp->blf_size++;
>  
>  		/*
> @@ -357,11 +353,10 @@ xfs_buf_item_format_segment(
>  STATIC void
>  xfs_buf_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
>  	struct xfs_buf		*bp = bip->bli_buf;
> -	struct xfs_log_iovec	*vecp = NULL;
>  	uint			offset = 0;
>  	int			i;
>  
> @@ -398,7 +393,7 @@ xfs_buf_item_format(
>  	}
>  
>  	for (i = 0; i < bip->bli_format_count; i++) {
> -		xfs_buf_item_format_segment(bip, lv, &vecp, offset,
> +		xfs_buf_item_format_segment(bip, lfb, offset,
>  					    &bip->bli_formats[i]);
>  		offset += BBTOB(bp->b_maps[i].bm_len);
>  	}
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 271b195ebb93..9c2fcfbdf7dc 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -44,25 +44,24 @@ xfs_qm_dquot_logitem_size(
>  STATIC void
>  xfs_qm_dquot_logitem_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_disk_dquot	ddq;
>  	struct xfs_dq_logitem	*qlip = DQUOT_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  	struct xfs_dq_logformat	*qlf;
>  
> -	qlf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_QFORMAT);
> +	qlf = xlog_format_start(lfb, XLOG_REG_TYPE_QFORMAT);
>  	qlf->qlf_type = XFS_LI_DQUOT;
>  	qlf->qlf_size = 2;
>  	qlf->qlf_id = qlip->qli_dquot->q_id;
>  	qlf->qlf_blkno = qlip->qli_dquot->q_blkno;
>  	qlf->qlf_len = 1;
>  	qlf->qlf_boffset = qlip->qli_dquot->q_bufoffset;
> -	xlog_finish_iovec(lv, vecp, sizeof(struct xfs_dq_logformat));
> +	xlog_format_commit(lfb, sizeof(struct xfs_dq_logformat));
>  
>  	xfs_dquot_to_disk(&ddq, qlip->qli_dquot);
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_DQUOT, &ddq,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_DQUOT, &ddq,
>  			sizeof(struct xfs_disk_dquot));
>  }
>  
> diff --git a/fs/xfs/xfs_exchmaps_item.c b/fs/xfs/xfs_exchmaps_item.c
> index 229cbe0adf17..10d6fbeff651 100644
> --- a/fs/xfs/xfs_exchmaps_item.c
> +++ b/fs/xfs/xfs_exchmaps_item.c
> @@ -83,16 +83,14 @@ xfs_xmi_item_size(
>  STATIC void
>  xfs_xmi_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_xmi_log_item	*xmi_lip = XMI_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	xmi_lip->xmi_format.xmi_type = XFS_LI_XMI;
>  	xmi_lip->xmi_format.xmi_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_XMI_FORMAT,
> -			&xmi_lip->xmi_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_XMI_FORMAT, &xmi_lip->xmi_format,
>  			sizeof(struct xfs_xmi_log_format));
>  }
>  
> @@ -166,15 +164,14 @@ xfs_xmd_item_size(
>  STATIC void
>  xfs_xmd_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_xmd_log_item	*xmd_lip = XMD_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	xmd_lip->xmd_format.xmd_type = XFS_LI_XMD;
>  	xmd_lip->xmd_format.xmd_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_XMD_FORMAT, &xmd_lip->xmd_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_XMD_FORMAT, &xmd_lip->xmd_format,
>  			sizeof(struct xfs_xmd_log_format));
>  }
>  
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 418ddab590e0..3d1edc43e6fb 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -98,10 +98,9 @@ unsigned int xfs_efi_log_space(unsigned int nr)
>  STATIC void
>  xfs_efi_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_efi_log_item	*efip = EFI_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(atomic_read(&efip->efi_next_extent) ==
>  				efip->efi_format.efi_nextents);
> @@ -110,7 +109,7 @@ xfs_efi_item_format(
>  	efip->efi_format.efi_type = lip->li_type;
>  	efip->efi_format.efi_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFI_FORMAT, &efip->efi_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_EFI_FORMAT, &efip->efi_format,
>  			xfs_efi_log_format_sizeof(efip->efi_format.efi_nextents));
>  }
>  
> @@ -277,10 +276,9 @@ unsigned int xfs_efd_log_space(unsigned int nr)
>  STATIC void
>  xfs_efd_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_efd_log_item	*efdp = EFD_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(efdp->efd_next_extent == efdp->efd_format.efd_nextents);
>  	ASSERT(lip->li_type == XFS_LI_EFD || lip->li_type == XFS_LI_EFD_RT);
> @@ -288,7 +286,7 @@ xfs_efd_item_format(
>  	efdp->efd_format.efd_type = lip->li_type;
>  	efdp->efd_format.efd_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_EFD_FORMAT, &efdp->efd_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_EFD_FORMAT, &efdp->efd_format,
>  			xfs_efd_log_format_sizeof(efdp->efd_format.efd_nextents));
>  }
>  
> diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
> index f83ec2bd0583..004dd22393dc 100644
> --- a/fs/xfs/xfs_icreate_item.c
> +++ b/fs/xfs/xfs_icreate_item.c
> @@ -49,13 +49,11 @@ xfs_icreate_item_size(
>  STATIC void
>  xfs_icreate_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_icreate_item	*icp = ICR_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ICREATE,
> -			&icp->ic_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_ICREATE, &icp->ic_format,
>  			sizeof(struct xfs_icreate_log));
>  }
>  
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 1bd411a1114c..54d72234ae32 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -336,8 +336,7 @@ STATIC void
>  xfs_inode_item_format_data_fork(
>  	struct xfs_inode_log_item *iip,
>  	struct xfs_inode_log_format *ilf,
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_inode	*ip = iip->ili_inode;
>  	size_t			data_bytes;
> @@ -354,9 +353,9 @@ xfs_inode_item_format_data_fork(
>  
>  			ASSERT(xfs_iext_count(&ip->i_df) > 0);
>  
> -			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IEXT);
> +			p = xlog_format_start(lfb, XLOG_REG_TYPE_IEXT);
>  			data_bytes = xfs_iextents_copy(ip, p, XFS_DATA_FORK);
> -			xlog_finish_iovec(lv, *vecp, data_bytes);
> +			xlog_format_commit(lfb, data_bytes);
>  
>  			ASSERT(data_bytes <= ip->i_df.if_bytes);
>  
> @@ -374,7 +373,7 @@ xfs_inode_item_format_data_fork(
>  		if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
>  		    ip->i_df.if_broot_bytes > 0) {
>  			ASSERT(ip->i_df.if_broot != NULL);
> -			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IBROOT,
> +			xlog_format_copy(lfb, XLOG_REG_TYPE_IBROOT,
>  					ip->i_df.if_broot,
>  					ip->i_df.if_broot_bytes);
>  			ilf->ilf_dsize = ip->i_df.if_broot_bytes;
> @@ -392,8 +391,9 @@ xfs_inode_item_format_data_fork(
>  		    ip->i_df.if_bytes > 0) {
>  			ASSERT(ip->i_df.if_data != NULL);
>  			ASSERT(ip->i_disk_size > 0);
> -			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
> -					ip->i_df.if_data, ip->i_df.if_bytes);
> +			xlog_format_copy(lfb, XLOG_REG_TYPE_ILOCAL,
> +					ip->i_df.if_data,
> +					ip->i_df.if_bytes);
>  			ilf->ilf_dsize = (unsigned)ip->i_df.if_bytes;
>  			ilf->ilf_size++;
>  		} else {
> @@ -416,8 +416,7 @@ STATIC void
>  xfs_inode_item_format_attr_fork(
>  	struct xfs_inode_log_item *iip,
>  	struct xfs_inode_log_format *ilf,
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_inode	*ip = iip->ili_inode;
>  	size_t			data_bytes;
> @@ -435,9 +434,9 @@ xfs_inode_item_format_attr_fork(
>  			ASSERT(xfs_iext_count(&ip->i_af) ==
>  				ip->i_af.if_nextents);
>  
> -			p = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_EXT);
> +			p = xlog_format_start(lfb, XLOG_REG_TYPE_IATTR_EXT);
>  			data_bytes = xfs_iextents_copy(ip, p, XFS_ATTR_FORK);
> -			xlog_finish_iovec(lv, *vecp, data_bytes);
> +			xlog_format_commit(lfb, data_bytes);
>  
>  			ilf->ilf_asize = data_bytes;
>  			ilf->ilf_size++;
> @@ -453,7 +452,7 @@ xfs_inode_item_format_attr_fork(
>  		    ip->i_af.if_broot_bytes > 0) {
>  			ASSERT(ip->i_af.if_broot != NULL);
>  
> -			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_BROOT,
> +			xlog_format_copy(lfb, XLOG_REG_TYPE_IATTR_BROOT,
>  					ip->i_af.if_broot,
>  					ip->i_af.if_broot_bytes);
>  			ilf->ilf_asize = ip->i_af.if_broot_bytes;
> @@ -469,8 +468,9 @@ xfs_inode_item_format_attr_fork(
>  		if ((iip->ili_fields & XFS_ILOG_ADATA) &&
>  		    ip->i_af.if_bytes > 0) {
>  			ASSERT(ip->i_af.if_data != NULL);
> -			xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
> -					ip->i_af.if_data, ip->i_af.if_bytes);
> +			xlog_format_copy(lfb, XLOG_REG_TYPE_IATTR_LOCAL,
> +					ip->i_af.if_data,
> +					ip->i_af.if_bytes);
>  			ilf->ilf_asize = (unsigned)ip->i_af.if_bytes;
>  			ilf->ilf_size++;
>  		} else {
> @@ -619,14 +619,13 @@ xfs_inode_to_log_dinode(
>  static void
>  xfs_inode_item_format_core(
>  	struct xfs_inode	*ip,
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_log_dinode	*dic;
>  
> -	dic = xlog_prepare_iovec(lv, vecp, XLOG_REG_TYPE_ICORE);
> +	dic = xlog_format_start(lfb, XLOG_REG_TYPE_ICORE);
>  	xfs_inode_to_log_dinode(ip, dic, ip->i_itemp->ili_item.li_lsn);
> -	xlog_finish_iovec(lv, *vecp, xfs_log_dinode_size(ip->i_mount));
> +	xlog_format_commit(lfb, xfs_log_dinode_size(ip->i_mount));
>  }
>  
>  /*
> @@ -644,14 +643,13 @@ xfs_inode_item_format_core(
>  STATIC void
>  xfs_inode_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_inode_log_item *iip = INODE_ITEM(lip);
>  	struct xfs_inode	*ip = iip->ili_inode;
> -	struct xfs_log_iovec	*vecp = NULL;
>  	struct xfs_inode_log_format *ilf;
>  
> -	ilf = xlog_prepare_iovec(lv, &vecp, XLOG_REG_TYPE_IFORMAT);
> +	ilf = xlog_format_start(lfb, XLOG_REG_TYPE_IFORMAT);
>  	ilf->ilf_type = XFS_LI_INODE;
>  	ilf->ilf_ino = ip->i_ino;
>  	ilf->ilf_blkno = ip->i_imap.im_blkno;
> @@ -668,13 +666,12 @@ xfs_inode_item_format(
>  	ilf->ilf_asize = 0;
>  	ilf->ilf_pad = 0;
>  	memset(&ilf->ilf_u, 0, sizeof(ilf->ilf_u));
> +	xlog_format_commit(lfb, sizeof(*ilf));
>  
> -	xlog_finish_iovec(lv, vecp, sizeof(*ilf));
> -
> -	xfs_inode_item_format_core(ip, lv, &vecp);
> -	xfs_inode_item_format_data_fork(iip, ilf, lv, &vecp);
> +	xfs_inode_item_format_core(ip, lfb);
> +	xfs_inode_item_format_data_fork(iip, ilf, lfb);
>  	if (xfs_inode_has_attr_fork(ip)) {
> -		xfs_inode_item_format_attr_fork(iip, ilf, lv, &vecp);
> +		xfs_inode_item_format_attr_fork(iip, ilf, lfb);
>  	} else {
>  		iip->ili_fields &=
>  			~(XFS_ILOG_ADATA | XFS_ILOG_ABROOT | XFS_ILOG_AEXT);
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 382c55f4d8d2..93e99d1cc037 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -74,62 +74,6 @@ xlog_iclogs_empty(
>  static int
>  xfs_log_cover(struct xfs_mount *);
>  
> -/*
> - * We need to make sure the buffer pointer returned is naturally aligned for the
> - * biggest basic data type we put into it. We have already accounted for this
> - * padding when sizing the buffer.
> - *
> - * However, this padding does not get written into the log, and hence we have to
> - * track the space used by the log vectors separately to prevent log space hangs
> - * due to inaccurate accounting (i.e. a leak) of the used log space through the
> - * CIL context ticket.
> - *
> - * We also add space for the xlog_op_header that describes this region in the
> - * log. This prepends the data region we return to the caller to copy their data
> - * into, so do all the static initialisation of the ophdr now. Because the ophdr
> - * is not 8 byte aligned, we have to be careful to ensure that we align the
> - * start of the buffer such that the region we return to the call is 8 byte
> - * aligned and packed against the tail of the ophdr.
> - */
> -void *
> -xlog_prepare_iovec(
> -	struct xfs_log_vec	*lv,
> -	struct xfs_log_iovec	**vecp,
> -	uint			type)
> -{
> -	struct xfs_log_iovec	*vec = *vecp;
> -	struct xlog_op_header	*oph;
> -	uint32_t		len;
> -	void			*buf;
> -
> -	if (vec) {
> -		ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> -		vec++;
> -	} else {
> -		vec = &lv->lv_iovecp[0];
> -	}
> -
> -	len = lv->lv_buf_used + sizeof(struct xlog_op_header);
> -	if (!IS_ALIGNED(len, sizeof(uint64_t))) {
> -		lv->lv_buf_used = round_up(len, sizeof(uint64_t)) -
> -					sizeof(struct xlog_op_header);
> -	}
> -
> -	vec->i_type = type;
> -	vec->i_addr = lv->lv_buf + lv->lv_buf_used;
> -
> -	oph = vec->i_addr;
> -	oph->oh_clientid = XFS_TRANSACTION;
> -	oph->oh_res2 = 0;
> -	oph->oh_flags = 0;
> -
> -	buf = vec->i_addr + sizeof(struct xlog_op_header);
> -	ASSERT(IS_ALIGNED((unsigned long)buf, sizeof(uint64_t)));
> -
> -	*vecp = vec;
> -	return buf;
> -}
> -
>  static inline void
>  xlog_grant_sub_space(
>  	struct xlog_grant_head	*head,
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index dcc1f44ed68f..c4930e925fed 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -6,6 +6,7 @@
>  #ifndef	__XFS_LOG_H__
>  #define __XFS_LOG_H__
>  
> +struct xlog_format_buf;
>  struct xfs_cil_ctx;
>  
>  struct xfs_log_vec {
> @@ -70,58 +71,24 @@ xlog_calc_iovec_len(int len)
>  	return roundup(len, sizeof(uint32_t));
>  }
>  
> -void *xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> -		uint type);
> -
> -static inline void
> -xlog_finish_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec *vec,
> -		int data_len)
> -{
> -	struct xlog_op_header	*oph = vec->i_addr;
> -	int			len;
> -
> -	/*
> -	 * Always round up the length to the correct alignment so callers don't
> -	 * need to know anything about this log vec layout requirement. This
> -	 * means we have to zero the area the data to be written does not cover.
> -	 * This is complicated by fact the payload region is offset into the
> -	 * logvec region by the opheader that tracks the payload.
> -	 */
> -	len = xlog_calc_iovec_len(data_len);
> -	if (len - data_len != 0) {
> -		char	*buf = vec->i_addr + sizeof(struct xlog_op_header);
> -
> -		memset(buf + data_len, 0, len - data_len);
> -	}
> -
> -	/*
> -	 * The opheader tracks aligned payload length, whilst the logvec tracks
> -	 * the overall region length.
> -	 */
> -	oph->oh_len = cpu_to_be32(len);
> -
> -	len += sizeof(struct xlog_op_header);
> -	lv->lv_buf_used += len;
> -	lv->lv_bytes += len;
> -	vec->i_len = len;
> -
> -	/* Catch buffer overruns */
> -	ASSERT((void *)lv->lv_buf + lv->lv_bytes <=
> -		(void *)lv + lv->lv_alloc_size);
> -}
> +void *xlog_format_start(struct xlog_format_buf *lfb, uint16_t type);
> +void xlog_format_commit(struct xlog_format_buf *lfb, unsigned int data_len);
>  
>  /*
>   * Copy the amount of data requested by the caller into a new log iovec.
>   */
>  static inline void *
> -xlog_copy_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> -		uint type, void *data, int len)
> +xlog_format_copy(
> +	struct xlog_format_buf	*lfb,
> +	uint16_t		type,
> +	void			*data,
> +	unsigned int		len)
>  {
>  	void *buf;
>  
> -	buf = xlog_prepare_iovec(lv, vecp, type);
> +	buf = xlog_format_start(lfb, type);
>  	memcpy(buf, data, len);
> -	xlog_finish_iovec(lv, *vecp, len);
> +	xlog_format_commit(lfb, len);
>  	return buf;
>  }
>  
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 83aa06e19cfb..bc25012ac5c0 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -409,6 +409,102 @@ xfs_cil_prepare_item(
>  		lv->lv_item->li_seq = log->l_cilp->xc_ctx->sequence;
>  }
>  
> +struct xlog_format_buf {
> +	struct xfs_log_vec	*lv;
> +	unsigned int		idx;
> +};
> +
> +/*
> + * We need to make sure the buffer pointer returned is naturally aligned for the
> + * biggest basic data type we put into it. We have already accounted for this
> + * padding when sizing the buffer.
> + *
> + * However, this padding does not get written into the log, and hence we have to
> + * track the space used by the log vectors separately to prevent log space hangs
> + * due to inaccurate accounting (i.e. a leak) of the used log space through the
> + * CIL context ticket.
> + *
> + * We also add space for the xlog_op_header that describes this region in the
> + * log. This prepends the data region we return to the caller to copy their data
> + * into, so do all the static initialisation of the ophdr now. Because the ophdr
> + * is not 8 byte aligned, we have to be careful to ensure that we align the
> + * start of the buffer such that the region we return to the call is 8 byte
> + * aligned and packed against the tail of the ophdr.
> + */
> +void *
> +xlog_format_start(
> +	struct xlog_format_buf	*lfb,
> +	uint16_t		type)
> +{
> +	struct xfs_log_vec	*lv = lfb->lv;
> +	struct xfs_log_iovec	*vec = &lv->lv_iovecp[lfb->idx];
> +	struct xlog_op_header	*oph;
> +	uint32_t		len;
> +	void			*buf;
> +
> +	ASSERT(lfb->idx < lv->lv_niovecs);
> +
> +	len = lv->lv_buf_used + sizeof(struct xlog_op_header);
> +	if (!IS_ALIGNED(len, sizeof(uint64_t))) {
> +		lv->lv_buf_used = round_up(len, sizeof(uint64_t)) -
> +					sizeof(struct xlog_op_header);
> +	}
> +
> +	vec->i_type = type;
> +	vec->i_addr = lv->lv_buf + lv->lv_buf_used;
> +
> +	oph = vec->i_addr;
> +	oph->oh_clientid = XFS_TRANSACTION;
> +	oph->oh_res2 = 0;
> +	oph->oh_flags = 0;
> +
> +	buf = vec->i_addr + sizeof(struct xlog_op_header);
> +	ASSERT(IS_ALIGNED((unsigned long)buf, sizeof(uint64_t)));
> +	return buf;
> +}
> +
> +void
> +xlog_format_commit(
> +	struct xlog_format_buf	*lfb,
> +	unsigned int		data_len)
> +{
> +	struct xfs_log_vec	*lv = lfb->lv;
> +	struct xfs_log_iovec	*vec = &lv->lv_iovecp[lfb->idx];
> +	struct xlog_op_header	*oph = vec->i_addr;
> +	int			len;
> +
> +	/*
> +	 * Always round up the length to the correct alignment so callers don't
> +	 * need to know anything about this log vec layout requirement. This
> +	 * means we have to zero the area the data to be written does not cover.
> +	 * This is complicated by fact the payload region is offset into the
> +	 * logvec region by the opheader that tracks the payload.
> +	 */
> +	len = xlog_calc_iovec_len(data_len);
> +	if (len - data_len != 0) {
> +		char	*buf = vec->i_addr + sizeof(struct xlog_op_header);
> +
> +		memset(buf + data_len, 0, len - data_len);
> +	}
> +
> +	/*
> +	 * The opheader tracks aligned payload length, whilst the logvec tracks
> +	 * the overall region length.
> +	 */
> +	oph->oh_len = cpu_to_be32(len);
> +
> +	len += sizeof(struct xlog_op_header);
> +	lv->lv_buf_used += len;
> +	lv->lv_bytes += len;
> +	vec->i_len = len;
> +
> +	/* Catch buffer overruns */
> +	ASSERT((void *)lv->lv_buf + lv->lv_bytes <=
> +		(void *)lv + lv->lv_alloc_size);
> +
> +	lfb->idx++;
> +}
> +
>  /*
>   * Format log item into a flat buffers
>   *
> @@ -454,6 +550,7 @@ xlog_cil_insert_format_items(
>  	list_for_each_entry(lip, &tp->t_items, li_trans) {
>  		struct xfs_log_vec *lv = lip->li_lv;
>  		struct xfs_log_vec *shadow = lip->li_lv_shadow;
> +		struct xlog_format_buf lfb = { };
>  
>  		/* Skip items which aren't dirty in this transaction. */
>  		if (!test_bit(XFS_LI_DIRTY, &lip->li_flags))
> @@ -501,8 +598,9 @@ xlog_cil_insert_format_items(
>  			lv->lv_item = lip;
>  		}
>  
> +		lfb.lv = lv;
>  		ASSERT(IS_ALIGNED((unsigned long)lv->lv_buf, sizeof(uint64_t)));
> -		lip->li_ops->iop_format(lip, lv);
> +		lip->li_ops->iop_format(lip, &lfb);
>  		xfs_cil_prepare_item(log, lip, lv, diff_len);
>  	}
>  }
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 3728234699a2..a41f5b577e22 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -93,10 +93,9 @@ unsigned int xfs_cui_log_space(unsigned int nr)
>  STATIC void
>  xfs_cui_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_cui_log_item	*cuip = CUI_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(atomic_read(&cuip->cui_next_extent) ==
>  			cuip->cui_format.cui_nextents);
> @@ -105,7 +104,7 @@ xfs_cui_item_format(
>  	cuip->cui_format.cui_type = lip->li_type;
>  	cuip->cui_format.cui_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_CUI_FORMAT, &cuip->cui_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_CUI_FORMAT, &cuip->cui_format,
>  			xfs_cui_log_format_sizeof(cuip->cui_format.cui_nextents));
>  }
>  
> @@ -199,17 +198,16 @@ unsigned int xfs_cud_log_space(void)
>  STATIC void
>  xfs_cud_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_cud_log_item	*cudp = CUD_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(lip->li_type == XFS_LI_CUD || lip->li_type == XFS_LI_CUD_RT);
>  
>  	cudp->cud_format.cud_type = lip->li_type;
>  	cudp->cud_format.cud_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_CUD_FORMAT, &cudp->cud_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_CUD_FORMAT, &cudp->cud_format,
>  			sizeof(struct xfs_cud_log_format));
>  }
>  
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index 15f0903f6fd4..8bf04b101156 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -92,10 +92,9 @@ unsigned int xfs_rui_log_space(unsigned int nr)
>  STATIC void
>  xfs_rui_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_rui_log_item	*ruip = RUI_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(atomic_read(&ruip->rui_next_extent) ==
>  			ruip->rui_format.rui_nextents);
> @@ -105,7 +104,7 @@ xfs_rui_item_format(
>  	ruip->rui_format.rui_type = lip->li_type;
>  	ruip->rui_format.rui_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_RUI_FORMAT, &ruip->rui_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_RUI_FORMAT, &ruip->rui_format,
>  			xfs_rui_log_format_sizeof(ruip->rui_format.rui_nextents));
>  }
>  
> @@ -200,17 +199,16 @@ unsigned int xfs_rud_log_space(void)
>  STATIC void
>  xfs_rud_item_format(
>  	struct xfs_log_item	*lip,
> -	struct xfs_log_vec	*lv)
> +	struct xlog_format_buf	*lfb)
>  {
>  	struct xfs_rud_log_item	*rudp = RUD_ITEM(lip);
> -	struct xfs_log_iovec	*vecp = NULL;
>  
>  	ASSERT(lip->li_type == XFS_LI_RUD || lip->li_type == XFS_LI_RUD_RT);
>  
>  	rudp->rud_format.rud_type = lip->li_type;
>  	rudp->rud_format.rud_size = 1;
>  
> -	xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_RUD_FORMAT, &rudp->rud_format,
> +	xlog_format_copy(lfb, XLOG_REG_TYPE_RUD_FORMAT, &rudp->rud_format,
>  			sizeof(struct xfs_rud_log_format));
>  }
>  
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 7fb860f645a3..8830600b3e72 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -9,6 +9,7 @@
>  /* kernel only transaction subsystem defines */
>  
>  struct xlog;
> +struct xlog_format_buf;
>  struct xfs_buf;
>  struct xfs_buftarg;
>  struct xfs_efd_log_item;
> @@ -70,7 +71,8 @@ struct xfs_log_item {
>  struct xfs_item_ops {
>  	unsigned flags;
>  	void (*iop_size)(struct xfs_log_item *, int *, int *);
> -	void (*iop_format)(struct xfs_log_item *, struct xfs_log_vec *);
> +	void (*iop_format)(struct xfs_log_item *lip,
> +			struct xlog_format_buf *lfb);
>  	void (*iop_pin)(struct xfs_log_item *);
>  	void (*iop_unpin)(struct xfs_log_item *, int remove);
>  	uint64_t (*iop_sort)(struct xfs_log_item *lip);
> -- 
> 2.47.3
> 
> 

[PATCH 04/10] xfs: move struct xfs_log_iovec to xfs_log_priv.h

[Christoph Hellwig]

This structure is now only used by the core logging and CIL code.

Also remove the unused typedef.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_log_format.h | 7 -------
 fs/xfs/xfs_log_priv.h          | 6 ++++++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 908e7060428c..3f5a24dda907 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -184,13 +184,6 @@ struct xlog_rec_header {
 #define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_size)
 #endif /* __i386__ */
 
-/* not an on-disk structure, but needed by log recovery in userspace */
-struct xfs_log_iovec {
-	void		*i_addr;	/* beginning address of region */
-	int		i_len;		/* length in bytes of region */
-	uint		i_type;		/* type of region */
-};
-
 /*
  * Transaction Header definitions.
  *
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index d2410e78b7f5..b7b3f61aa2ae 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -13,6 +13,12 @@ struct xlog;
 struct xlog_ticket;
 struct xfs_mount;
 
+struct xfs_log_iovec {
+	void			*i_addr;/* beginning address of region */
+	int			i_len;	/* length in bytes of region */
+	uint			i_type;	/* type of region */
+};
+
 /*
  * get client id from packed copy.
  *
-- 
2.47.3

Re: [PATCH 04/10] xfs: move struct xfs_log_iovec to xfs_log_priv.h

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:14PM +0100, Christoph Hellwig wrote:
> This structure is now only used by the core logging and CIL code.
> 
> Also remove the unused typedef.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

wooo, -typedef
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_log_format.h | 7 -------
>  fs/xfs/xfs_log_priv.h          | 6 ++++++
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 908e7060428c..3f5a24dda907 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -184,13 +184,6 @@ struct xlog_rec_header {
>  #define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_size)
>  #endif /* __i386__ */
>  
> -/* not an on-disk structure, but needed by log recovery in userspace */
> -struct xfs_log_iovec {
> -	void		*i_addr;	/* beginning address of region */
> -	int		i_len;		/* length in bytes of region */
> -	uint		i_type;		/* type of region */
> -};
> -
>  /*
>   * Transaction Header definitions.
>   *
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index d2410e78b7f5..b7b3f61aa2ae 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -13,6 +13,12 @@ struct xlog;
>  struct xlog_ticket;
>  struct xfs_mount;
>  
> +struct xfs_log_iovec {
> +	void			*i_addr;/* beginning address of region */
> +	int			i_len;	/* length in bytes of region */
> +	uint			i_type;	/* type of region */
> +};
> +
>  /*
>   * get client id from packed copy.
>   *
> -- 
> 2.47.3
> 
> 

[PATCH 05/10] xfs: move struct xfs_log_vec to xfs_log_priv.h

[Christoph Hellwig]

The log_vec is a private type for the log/CIL code and should not be
exposed to anything else.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.h      | 12 ------------
 fs/xfs/xfs_log_priv.h | 12 ++++++++++++
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index c4930e925fed..0f23812b0b31 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -9,18 +9,6 @@
 struct xlog_format_buf;
 struct xfs_cil_ctx;
 
-struct xfs_log_vec {
-	struct list_head	lv_list;	/* CIL lv chain ptrs */
-	uint32_t		lv_order_id;	/* chain ordering info */
-	int			lv_niovecs;	/* number of iovecs in lv */
-	struct xfs_log_iovec	*lv_iovecp;	/* iovec array */
-	struct xfs_log_item	*lv_item;	/* owner */
-	char			*lv_buf;	/* formatted buffer */
-	int			lv_bytes;	/* accounted space in buffer */
-	int			lv_buf_used;	/* buffer space used so far */
-	int			lv_alloc_size;	/* size of allocated lv */
-};
-
 /* Region types for iovec's i_type */
 #define XLOG_REG_TYPE_BFORMAT		1
 #define XLOG_REG_TYPE_BCHUNK		2
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index b7b3f61aa2ae..cf1e4ce61a8c 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -19,6 +19,18 @@ struct xfs_log_iovec {
 	uint			i_type;	/* type of region */
 };
 
+struct xfs_log_vec {
+	struct list_head	lv_list;	/* CIL lv chain ptrs */
+	uint32_t		lv_order_id;	/* chain ordering info */
+	int			lv_niovecs;	/* number of iovecs in lv */
+	struct xfs_log_iovec	*lv_iovecp;	/* iovec array */
+	struct xfs_log_item	*lv_item;	/* owner */
+	char			*lv_buf;	/* formatted buffer */
+	int			lv_bytes;	/* accounted space in buffer */
+	int			lv_buf_used;	/* buffer space used so far */
+	int			lv_alloc_size;	/* size of allocated lv */
+};
+
 /*
  * get client id from packed copy.
  *
-- 
2.47.3

Re: [PATCH 05/10] xfs: move struct xfs_log_vec to xfs_log_priv.h

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:15PM +0100, Christoph Hellwig wrote:
> The log_vec is a private type for the log/CIL code and should not be
> exposed to anything else.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.h      | 12 ------------
>  fs/xfs/xfs_log_priv.h | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index c4930e925fed..0f23812b0b31 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -9,18 +9,6 @@
>  struct xlog_format_buf;
>  struct xfs_cil_ctx;
>  
> -struct xfs_log_vec {
> -	struct list_head	lv_list;	/* CIL lv chain ptrs */
> -	uint32_t		lv_order_id;	/* chain ordering info */
> -	int			lv_niovecs;	/* number of iovecs in lv */
> -	struct xfs_log_iovec	*lv_iovecp;	/* iovec array */
> -	struct xfs_log_item	*lv_item;	/* owner */
> -	char			*lv_buf;	/* formatted buffer */
> -	int			lv_bytes;	/* accounted space in buffer */
> -	int			lv_buf_used;	/* buffer space used so far */
> -	int			lv_alloc_size;	/* size of allocated lv */
> -};
> -
>  /* Region types for iovec's i_type */
>  #define XLOG_REG_TYPE_BFORMAT		1
>  #define XLOG_REG_TYPE_BCHUNK		2
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index b7b3f61aa2ae..cf1e4ce61a8c 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -19,6 +19,18 @@ struct xfs_log_iovec {
>  	uint			i_type;	/* type of region */
>  };
>  
> +struct xfs_log_vec {
> +	struct list_head	lv_list;	/* CIL lv chain ptrs */
> +	uint32_t		lv_order_id;	/* chain ordering info */
> +	int			lv_niovecs;	/* number of iovecs in lv */
> +	struct xfs_log_iovec	*lv_iovecp;	/* iovec array */
> +	struct xfs_log_item	*lv_item;	/* owner */
> +	char			*lv_buf;	/* formatted buffer */
> +	int			lv_bytes;	/* accounted space in buffer */
> +	int			lv_buf_used;	/* buffer space used so far */
> +	int			lv_alloc_size;	/* size of allocated lv */
> +};
> +
>  /*
>   * get client id from packed copy.
>   *
> -- 
> 2.47.3
> 
> 

[PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial

[Christoph Hellwig]

When xlog_write_partial splits a log region over multiple iclogs, it
has to include the continuation ophder in the length requested for the
new iclog.  Currently is simply adds that to the request, which makes
the accounting of the used space below look slightly different from the
other users of iclog space that decrement it.  To prepare for more code
sharing, adding the ophdr size to the len variable before the call to
xlog_write_get_more_iclog_space and then decrement it later.

This changes the contents of len when xlog_write_get_more_iclog_space
returns an error, but as nothing looks at len in that case the
difference doesn't matter.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 93e99d1cc037..539b22dff2d1 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2048,10 +2048,10 @@ xlog_write_partial(
 			 * consumes hasn't been accounted to the lv we are
 			 * writing.
 			 */
+			*len += sizeof(struct xlog_op_header);
 			error = xlog_write_get_more_iclog_space(ticket,
-					&iclog, log_offset,
-					*len + sizeof(struct xlog_op_header),
-					record_cnt, data_cnt);
+					&iclog, log_offset, *len, record_cnt,
+					data_cnt);
 			if (error)
 				return error;
 
@@ -2064,6 +2064,7 @@ xlog_write_partial(
 			ticket->t_curr_res -= sizeof(struct xlog_op_header);
 			*log_offset += sizeof(struct xlog_op_header);
 			*data_cnt += sizeof(struct xlog_op_header);
+			*len -= sizeof(struct xlog_op_header);
 
 			/*
 			 * If rlen fits in the iclog, then end the region
-- 
2.47.3

Re: [PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:16PM +0100, Christoph Hellwig wrote:
> When xlog_write_partial splits a log region over multiple iclogs, it
> has to include the continuation ophder in the length requested for the
> new iclog.  Currently is simply adds that to the request, which makes
> the accounting of the used space below look slightly different from the
> other users of iclog space that decrement it.  To prepare for more code
> sharing, adding the ophdr size to the len variable before the call to
> xlog_write_get_more_iclog_space and then decrement it later.
> 
> This changes the contents of len when xlog_write_get_more_iclog_space
> returns an error, but as nothing looks at len in that case the
> difference doesn't matter.

Ooops, sorry I missed this patch. :(

It makes sense to me that we have to account for the continuation when
asking for a fresh iclog, but now I have a question.  In "xfs: improve
the calling convention for the xlog_write helpers", the len pointer
becomes xlog_write_data::bytes_left.  In the context of this patch, I
guess that means "len" is the amount of log data that we still have to
write to the log, correct?

--D

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 93e99d1cc037..539b22dff2d1 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2048,10 +2048,10 @@ xlog_write_partial(
>  			 * consumes hasn't been accounted to the lv we are
>  			 * writing.
>  			 */
> +			*len += sizeof(struct xlog_op_header);
>  			error = xlog_write_get_more_iclog_space(ticket,
> -					&iclog, log_offset,
> -					*len + sizeof(struct xlog_op_header),
> -					record_cnt, data_cnt);
> +					&iclog, log_offset, *len, record_cnt,
> +					data_cnt);
>  			if (error)
>  				return error;
>  
> @@ -2064,6 +2064,7 @@ xlog_write_partial(
>  			ticket->t_curr_res -= sizeof(struct xlog_op_header);
>  			*log_offset += sizeof(struct xlog_op_header);
>  			*data_cnt += sizeof(struct xlog_op_header);
> +			*len -= sizeof(struct xlog_op_header);
>  
>  			/*
>  			 * If rlen fits in the iclog, then end the region
> -- 
> 2.47.3
> 
> 

Re: [PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial

[Christoph Hellwig]

On Tue, Nov 04, 2025 at 03:53:09PM -0800, Darrick J. Wong wrote:
> On Thu, Oct 30, 2025 at 03:49:16PM +0100, Christoph Hellwig wrote:
> > When xlog_write_partial splits a log region over multiple iclogs, it
> > has to include the continuation ophder in the length requested for the
> > new iclog.  Currently is simply adds that to the request, which makes
> > the accounting of the used space below look slightly different from the
> > other users of iclog space that decrement it.  To prepare for more code
> > sharing, adding the ophdr size to the len variable before the call to
> > xlog_write_get_more_iclog_space and then decrement it later.
> > 
> > This changes the contents of len when xlog_write_get_more_iclog_space
> > returns an error, but as nothing looks at len in that case the
> > difference doesn't matter.
> 
> Ooops, sorry I missed this patch. :(
> 
> It makes sense to me that we have to account for the continuation when
> asking for a fresh iclog, but now I have a question.  In "xfs: improve
> the calling convention for the xlog_write helpers", the len pointer
> becomes xlog_write_data::bytes_left.  In the context of this patch, I
> guess that means "len" is the amount of log data that we still have to
> write to the log, correct?

Yes.

[PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers

[Christoph Hellwig]

The xlog_write chain passes around the same seven variables that are
often passed by reference. Add a xlog_write_data structure to contain
them to improve code generation and readability.

This change reduces the generated code size by almost 200 bytes for my
x86_64 build:

$ size fs/xfs/xfs_log.o*
   text	   data	    bss	    dec	    hex	filename
  26330	   1292	      8	  27630	   6bee	fs/xfs/xfs_log.o
  26158	   1292	      8	  27458	   6b42	fs/xfs/xfs_log.o.old

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 187 +++++++++++++++++++----------------------------
 1 file changed, 77 insertions(+), 110 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 539b22dff2d1..2b1744af8a67 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -22,6 +22,15 @@
 #include "xfs_health.h"
 #include "xfs_zone_alloc.h"
 
+struct xlog_write_data {
+	struct xlog_ticket	*ticket;
+	struct xlog_in_core	*iclog;
+	uint32_t		bytes_left;
+	uint32_t		record_cnt;
+	uint32_t		data_cnt;
+	int			log_offset;
+};
+
 struct kmem_cache	*xfs_log_ticket_cache;
 
 /* Local miscellaneous function prototypes */
@@ -43,10 +52,7 @@ STATIC void xlog_state_do_callback(
 STATIC int
 xlog_state_get_iclog_space(
 	struct xlog		*log,
-	int			len,
-	struct xlog_in_core	**iclog,
-	struct xlog_ticket	*ticket,
-	int			*logoffsetp);
+	struct xlog_write_data	*data);
 STATIC void
 xlog_sync(
 	struct xlog		*log,
@@ -1874,23 +1880,19 @@ 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)
+	struct xlog_write_data	*data,
+	void			*buf,
+	uint32_t		buf_len)
 {
-	ASSERT(*log_offset < iclog->ic_log->l_iclog_size);
-	ASSERT(*log_offset % sizeof(int32_t) == 0);
-	ASSERT(write_len % sizeof(int32_t) == 0);
+	ASSERT(data->log_offset < data->iclog->ic_log->l_iclog_size);
+	ASSERT(data->log_offset % sizeof(int32_t) == 0);
+	ASSERT(buf_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;
+	memcpy(data->iclog->ic_datap + data->log_offset, buf, buf_len);
+	data->log_offset += buf_len;
+	data->bytes_left -= buf_len;
+	data->record_cnt++;
+	data->data_cnt += buf_len;
 }
 
 /*
@@ -1900,17 +1902,12 @@ xlog_write_iovec(
 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)
+	struct xlog_write_data	*data)
 {
 	int			index;
 
-	ASSERT(*log_offset + *len <= iclog->ic_size ||
-		iclog->ic_state == XLOG_STATE_WANT_SYNC);
+	ASSERT(data->log_offset + data->bytes_left <= data->iclog->ic_size ||
+		data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
 
 	/*
 	 * Ordered log vectors have no regions to write so this
@@ -1920,40 +1917,32 @@ xlog_write_full(
 		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);
-		xlog_write_iovec(iclog, log_offset, reg->i_addr,
-				reg->i_len, len, record_cnt, data_cnt);
+		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
+		xlog_write_iovec(data, reg->i_addr, reg->i_len);
 	}
 }
 
 static int
 xlog_write_get_more_iclog_space(
-	struct xlog_ticket	*ticket,
-	struct xlog_in_core	**iclogp,
-	uint32_t		*log_offset,
-	uint32_t		len,
-	uint32_t		*record_cnt,
-	uint32_t		*data_cnt)
+	struct xlog_write_data	*data)
 {
-	struct xlog_in_core	*iclog = *iclogp;
-	struct xlog		*log = iclog->ic_log;
+	struct xlog		*log = data->iclog->ic_log;
 	int			error;
 
 	spin_lock(&log->l_icloglock);
-	ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC);
-	xlog_state_finish_copy(log, iclog, *record_cnt, *data_cnt);
-	error = xlog_state_release_iclog(log, iclog, ticket);
+	ASSERT(data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
+	xlog_state_finish_copy(log, data->iclog, data->record_cnt,
+			data->data_cnt);
+	error = xlog_state_release_iclog(log, data->iclog, data->ticket);
 	spin_unlock(&log->l_icloglock);
 	if (error)
 		return error;
 
-	error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
-					log_offset);
+	error = xlog_state_get_iclog_space(log, data);
 	if (error)
 		return error;
-	*record_cnt = 0;
-	*data_cnt = 0;
-	*iclogp = iclog;
+	data->record_cnt = 0;
+	data->data_cnt = 0;
 	return 0;
 }
 
@@ -1966,14 +1955,8 @@ xlog_write_get_more_iclog_space(
 static int
 xlog_write_partial(
 	struct xfs_log_vec	*lv,
-	struct xlog_ticket	*ticket,
-	struct xlog_in_core	**iclogp,
-	uint32_t		*log_offset,
-	uint32_t		*len,
-	uint32_t		*record_cnt,
-	uint32_t		*data_cnt)
+	struct xlog_write_data	*data)
 {
-	struct xlog_in_core	*iclog = *iclogp;
 	struct xlog_op_header	*ophdr;
 	int			index = 0;
 	uint32_t		rlen;
@@ -1995,25 +1978,23 @@ xlog_write_partial(
 		 * Hence if there isn't space for region data after the
 		 * opheader, then we need to start afresh with a new iclog.
 		 */
-		if (iclog->ic_size - *log_offset <=
+		if (data->iclog->ic_size - data->log_offset <=
 					sizeof(struct xlog_op_header)) {
-			error = xlog_write_get_more_iclog_space(ticket,
-					&iclog, log_offset, *len, record_cnt,
-					data_cnt);
+			error = xlog_write_get_more_iclog_space(data);
 			if (error)
 				return error;
 		}
 
 		ophdr = reg->i_addr;
-		rlen = min_t(uint32_t, reg->i_len, iclog->ic_size - *log_offset);
+		rlen = min_t(uint32_t, reg->i_len,
+			data->iclog->ic_size - data->log_offset);
 
-		ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
+		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
 		ophdr->oh_len = cpu_to_be32(rlen - sizeof(struct xlog_op_header));
 		if (rlen != reg->i_len)
 			ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
 
-		xlog_write_iovec(iclog, log_offset, reg->i_addr,
-				rlen, len, record_cnt, data_cnt);
+		xlog_write_iovec(data, reg->i_addr, rlen);
 
 		/* If we wrote the whole region, move to the next. */
 		if (rlen == reg->i_len)
@@ -2048,23 +2029,22 @@ xlog_write_partial(
 			 * consumes hasn't been accounted to the lv we are
 			 * writing.
 			 */
-			*len += sizeof(struct xlog_op_header);
-			error = xlog_write_get_more_iclog_space(ticket,
-					&iclog, log_offset, *len, record_cnt,
-					data_cnt);
+			data->bytes_left += sizeof(struct xlog_op_header);
+			error = xlog_write_get_more_iclog_space(data);
 			if (error)
 				return error;
 
-			ophdr = iclog->ic_datap + *log_offset;
-			ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
+			ophdr = data->iclog->ic_datap + data->log_offset;
+			ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
 			ophdr->oh_clientid = XFS_TRANSACTION;
 			ophdr->oh_res2 = 0;
 			ophdr->oh_flags = XLOG_WAS_CONT_TRANS;
 
-			ticket->t_curr_res -= sizeof(struct xlog_op_header);
-			*log_offset += sizeof(struct xlog_op_header);
-			*data_cnt += sizeof(struct xlog_op_header);
-			*len -= sizeof(struct xlog_op_header);
+			data->ticket->t_curr_res -=
+				sizeof(struct xlog_op_header);
+			data->log_offset += sizeof(struct xlog_op_header);
+			data->data_cnt += sizeof(struct xlog_op_header);
+			data->bytes_left -= sizeof(struct xlog_op_header);
 
 			/*
 			 * If rlen fits in the iclog, then end the region
@@ -2072,26 +2052,19 @@ xlog_write_partial(
 			 */
 			reg_offset += rlen;
 			rlen = reg->i_len - reg_offset;
-			if (rlen <= iclog->ic_size - *log_offset)
+			if (rlen <= data->iclog->ic_size - data->log_offset)
 				ophdr->oh_flags |= XLOG_END_TRANS;
 			else
 				ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
 
-			rlen = min_t(uint32_t, rlen, iclog->ic_size - *log_offset);
+			rlen = min_t(uint32_t, rlen,
+				data->iclog->ic_size - data->log_offset);
 			ophdr->oh_len = cpu_to_be32(rlen);
 
-			xlog_write_iovec(iclog, log_offset,
-					reg->i_addr + reg_offset,
-					rlen, len, record_cnt, data_cnt);
-
+			xlog_write_iovec(data, reg->i_addr + reg_offset, rlen);
 		} while (ophdr->oh_flags & XLOG_CONTINUE_TRANS);
 	}
 
-	/*
-	 * No more iovecs remain in this logvec so return the next log vec to
-	 * the caller so it can go back to fast path copying.
-	 */
-	*iclogp = iclog;
 	return 0;
 }
 
@@ -2144,12 +2117,12 @@ xlog_write(
 	uint32_t		len)
 
 {
-	struct xlog_in_core	*iclog = NULL;
 	struct xfs_log_vec	*lv;
-	uint32_t		record_cnt = 0;
-	uint32_t		data_cnt = 0;
-	int			error = 0;
-	int			log_offset;
+	struct xlog_write_data	data = {
+		.ticket		= ticket,
+		.bytes_left	= len,
+	};
+	int			error;
 
 	if (ticket->t_curr_res < 0) {
 		xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
@@ -2158,12 +2131,11 @@ xlog_write(
 		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
 	}
 
-	error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
-					   &log_offset);
+	error = xlog_state_get_iclog_space(log, &data);
 	if (error)
 		return error;
 
-	ASSERT(log_offset <= iclog->ic_size - 1);
+	ASSERT(data.log_offset <= data.iclog->ic_size - 1);
 
 	/*
 	 * If we have a context pointer, pass it the first iclog we are
@@ -2171,7 +2143,7 @@ xlog_write(
 	 * ordering.
 	 */
 	if (ctx)
-		xlog_cil_set_ctx_write_state(ctx, iclog);
+		xlog_cil_set_ctx_write_state(ctx, data.iclog);
 
 	list_for_each_entry(lv, lv_chain, lv_list) {
 		/*
@@ -2179,10 +2151,8 @@ xlog_write(
 		 * the partial copy loop which can handle this case.
 		 */
 		if (lv->lv_niovecs &&
-		    lv->lv_bytes > iclog->ic_size - log_offset) {
-			error = xlog_write_partial(lv, ticket, &iclog,
-					&log_offset, &len, &record_cnt,
-					&data_cnt);
+		    lv->lv_bytes > data.iclog->ic_size - data.log_offset) {
+			error = xlog_write_partial(lv, &data);
 			if (error) {
 				/*
 				 * We have no iclog to release, so just return
@@ -2191,11 +2161,10 @@ xlog_write(
 				return error;
 			}
 		} else {
-			xlog_write_full(lv, ticket, iclog, &log_offset,
-					 &len, &record_cnt, &data_cnt);
+			xlog_write_full(lv, &data);
 		}
 	}
-	ASSERT(len == 0);
+	ASSERT(data.bytes_left == 0);
 
 	/*
 	 * We've already been guaranteed that the last writes will fit inside
@@ -2204,8 +2173,8 @@ xlog_write(
 	 * iclog with the number of bytes written here.
 	 */
 	spin_lock(&log->l_icloglock);
-	xlog_state_finish_copy(log, iclog, record_cnt, 0);
-	error = xlog_state_release_iclog(log, iclog, ticket);
+	xlog_state_finish_copy(log, data.iclog, data.record_cnt, 0);
+	error = xlog_state_release_iclog(log, data.iclog, ticket);
 	spin_unlock(&log->l_icloglock);
 
 	return error;
@@ -2527,10 +2496,7 @@ xlog_state_done_syncing(
 STATIC int
 xlog_state_get_iclog_space(
 	struct xlog		*log,
-	int			len,
-	struct xlog_in_core	**iclogp,
-	struct xlog_ticket	*ticket,
-	int			*logoffsetp)
+	struct xlog_write_data	*data)
 {
 	int			log_offset;
 	struct xlog_rec_header	*head;
@@ -2565,7 +2531,7 @@ xlog_state_get_iclog_space(
 	 * must be written.
 	 */
 	if (log_offset == 0) {
-		ticket->t_curr_res -= log->l_iclog_hsize;
+		data->ticket->t_curr_res -= log->l_iclog_hsize;
 		head->h_cycle = cpu_to_be32(log->l_curr_cycle);
 		head->h_lsn = cpu_to_be64(
 			xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block));
@@ -2595,7 +2561,8 @@ xlog_state_get_iclog_space(
 		 * reference to the iclog.
 		 */
 		if (!atomic_add_unless(&iclog->ic_refcnt, -1, 1))
-			error = xlog_state_release_iclog(log, iclog, ticket);
+			error = xlog_state_release_iclog(log, iclog,
+					data->ticket);
 		spin_unlock(&log->l_icloglock);
 		if (error)
 			return error;
@@ -2608,16 +2575,16 @@ xlog_state_get_iclog_space(
 	 * iclogs (to mark it taken), this particular iclog will release/sync
 	 * to disk in xlog_write().
 	 */
-	if (len <= iclog->ic_size - iclog->ic_offset)
-		iclog->ic_offset += len;
+	if (data->bytes_left <= iclog->ic_size - iclog->ic_offset)
+		iclog->ic_offset += data->bytes_left;
 	else
 		xlog_state_switch_iclogs(log, iclog, iclog->ic_size);
-	*iclogp = iclog;
+	data->iclog = iclog;
 
 	ASSERT(iclog->ic_offset <= iclog->ic_size);
 	spin_unlock(&log->l_icloglock);
 
-	*logoffsetp = log_offset;
+	data->log_offset = log_offset;
 	return 0;
 }
 
-- 
2.47.3

Re: [PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:17PM +0100, Christoph Hellwig wrote:
> The xlog_write chain passes around the same seven variables that are
> often passed by reference. Add a xlog_write_data structure to contain
> them to improve code generation and readability.
> 
> This change reduces the generated code size by almost 200 bytes for my
> x86_64 build:
> 
> $ size fs/xfs/xfs_log.o*
>    text	   data	    bss	    dec	    hex	filename
>   26330	   1292	      8	  27630	   6bee	fs/xfs/xfs_log.o
>   26158	   1292	      8	  27458	   6b42	fs/xfs/xfs_log.o.old

Um... assuming xfs_log.o is the post-patch object file, this shows the
text size going up by 172 bytes, right?

--D

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c | 187 +++++++++++++++++++----------------------------
>  1 file changed, 77 insertions(+), 110 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 539b22dff2d1..2b1744af8a67 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -22,6 +22,15 @@
>  #include "xfs_health.h"
>  #include "xfs_zone_alloc.h"
>  
> +struct xlog_write_data {
> +	struct xlog_ticket	*ticket;
> +	struct xlog_in_core	*iclog;
> +	uint32_t		bytes_left;
> +	uint32_t		record_cnt;
> +	uint32_t		data_cnt;
> +	int			log_offset;
> +};
> +
>  struct kmem_cache	*xfs_log_ticket_cache;
>  
>  /* Local miscellaneous function prototypes */
> @@ -43,10 +52,7 @@ STATIC void xlog_state_do_callback(
>  STATIC int
>  xlog_state_get_iclog_space(
>  	struct xlog		*log,
> -	int			len,
> -	struct xlog_in_core	**iclog,
> -	struct xlog_ticket	*ticket,
> -	int			*logoffsetp);
> +	struct xlog_write_data	*data);
>  STATIC void
>  xlog_sync(
>  	struct xlog		*log,
> @@ -1874,23 +1880,19 @@ 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)
> +	struct xlog_write_data	*data,
> +	void			*buf,
> +	uint32_t		buf_len)
>  {
> -	ASSERT(*log_offset < iclog->ic_log->l_iclog_size);
> -	ASSERT(*log_offset % sizeof(int32_t) == 0);
> -	ASSERT(write_len % sizeof(int32_t) == 0);
> +	ASSERT(data->log_offset < data->iclog->ic_log->l_iclog_size);
> +	ASSERT(data->log_offset % sizeof(int32_t) == 0);
> +	ASSERT(buf_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;
> +	memcpy(data->iclog->ic_datap + data->log_offset, buf, buf_len);
> +	data->log_offset += buf_len;
> +	data->bytes_left -= buf_len;
> +	data->record_cnt++;
> +	data->data_cnt += buf_len;
>  }
>  
>  /*
> @@ -1900,17 +1902,12 @@ xlog_write_iovec(
>  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)
> +	struct xlog_write_data	*data)
>  {
>  	int			index;
>  
> -	ASSERT(*log_offset + *len <= iclog->ic_size ||
> -		iclog->ic_state == XLOG_STATE_WANT_SYNC);
> +	ASSERT(data->log_offset + data->bytes_left <= data->iclog->ic_size ||
> +		data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
>  
>  	/*
>  	 * Ordered log vectors have no regions to write so this
> @@ -1920,40 +1917,32 @@ xlog_write_full(
>  		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);
> -		xlog_write_iovec(iclog, log_offset, reg->i_addr,
> -				reg->i_len, len, record_cnt, data_cnt);
> +		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
> +		xlog_write_iovec(data, reg->i_addr, reg->i_len);
>  	}
>  }
>  
>  static int
>  xlog_write_get_more_iclog_space(
> -	struct xlog_ticket	*ticket,
> -	struct xlog_in_core	**iclogp,
> -	uint32_t		*log_offset,
> -	uint32_t		len,
> -	uint32_t		*record_cnt,
> -	uint32_t		*data_cnt)
> +	struct xlog_write_data	*data)
>  {
> -	struct xlog_in_core	*iclog = *iclogp;
> -	struct xlog		*log = iclog->ic_log;
> +	struct xlog		*log = data->iclog->ic_log;
>  	int			error;
>  
>  	spin_lock(&log->l_icloglock);
> -	ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC);
> -	xlog_state_finish_copy(log, iclog, *record_cnt, *data_cnt);
> -	error = xlog_state_release_iclog(log, iclog, ticket);
> +	ASSERT(data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
> +	xlog_state_finish_copy(log, data->iclog, data->record_cnt,
> +			data->data_cnt);
> +	error = xlog_state_release_iclog(log, data->iclog, data->ticket);
>  	spin_unlock(&log->l_icloglock);
>  	if (error)
>  		return error;
>  
> -	error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
> -					log_offset);
> +	error = xlog_state_get_iclog_space(log, data);
>  	if (error)
>  		return error;
> -	*record_cnt = 0;
> -	*data_cnt = 0;
> -	*iclogp = iclog;
> +	data->record_cnt = 0;
> +	data->data_cnt = 0;
>  	return 0;
>  }
>  
> @@ -1966,14 +1955,8 @@ xlog_write_get_more_iclog_space(
>  static int
>  xlog_write_partial(
>  	struct xfs_log_vec	*lv,
> -	struct xlog_ticket	*ticket,
> -	struct xlog_in_core	**iclogp,
> -	uint32_t		*log_offset,
> -	uint32_t		*len,
> -	uint32_t		*record_cnt,
> -	uint32_t		*data_cnt)
> +	struct xlog_write_data	*data)
>  {
> -	struct xlog_in_core	*iclog = *iclogp;
>  	struct xlog_op_header	*ophdr;
>  	int			index = 0;
>  	uint32_t		rlen;
> @@ -1995,25 +1978,23 @@ xlog_write_partial(
>  		 * Hence if there isn't space for region data after the
>  		 * opheader, then we need to start afresh with a new iclog.
>  		 */
> -		if (iclog->ic_size - *log_offset <=
> +		if (data->iclog->ic_size - data->log_offset <=
>  					sizeof(struct xlog_op_header)) {
> -			error = xlog_write_get_more_iclog_space(ticket,
> -					&iclog, log_offset, *len, record_cnt,
> -					data_cnt);
> +			error = xlog_write_get_more_iclog_space(data);
>  			if (error)
>  				return error;
>  		}
>  
>  		ophdr = reg->i_addr;
> -		rlen = min_t(uint32_t, reg->i_len, iclog->ic_size - *log_offset);
> +		rlen = min_t(uint32_t, reg->i_len,
> +			data->iclog->ic_size - data->log_offset);
>  
> -		ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> +		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
>  		ophdr->oh_len = cpu_to_be32(rlen - sizeof(struct xlog_op_header));
>  		if (rlen != reg->i_len)
>  			ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
>  
> -		xlog_write_iovec(iclog, log_offset, reg->i_addr,
> -				rlen, len, record_cnt, data_cnt);
> +		xlog_write_iovec(data, reg->i_addr, rlen);
>  
>  		/* If we wrote the whole region, move to the next. */
>  		if (rlen == reg->i_len)
> @@ -2048,23 +2029,22 @@ xlog_write_partial(
>  			 * consumes hasn't been accounted to the lv we are
>  			 * writing.
>  			 */
> -			*len += sizeof(struct xlog_op_header);
> -			error = xlog_write_get_more_iclog_space(ticket,
> -					&iclog, log_offset, *len, record_cnt,
> -					data_cnt);
> +			data->bytes_left += sizeof(struct xlog_op_header);
> +			error = xlog_write_get_more_iclog_space(data);
>  			if (error)
>  				return error;
>  
> -			ophdr = iclog->ic_datap + *log_offset;
> -			ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> +			ophdr = data->iclog->ic_datap + data->log_offset;
> +			ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
>  			ophdr->oh_clientid = XFS_TRANSACTION;
>  			ophdr->oh_res2 = 0;
>  			ophdr->oh_flags = XLOG_WAS_CONT_TRANS;
>  
> -			ticket->t_curr_res -= sizeof(struct xlog_op_header);
> -			*log_offset += sizeof(struct xlog_op_header);
> -			*data_cnt += sizeof(struct xlog_op_header);
> -			*len -= sizeof(struct xlog_op_header);
> +			data->ticket->t_curr_res -=
> +				sizeof(struct xlog_op_header);
> +			data->log_offset += sizeof(struct xlog_op_header);
> +			data->data_cnt += sizeof(struct xlog_op_header);
> +			data->bytes_left -= sizeof(struct xlog_op_header);
>  
>  			/*
>  			 * If rlen fits in the iclog, then end the region
> @@ -2072,26 +2052,19 @@ xlog_write_partial(
>  			 */
>  			reg_offset += rlen;
>  			rlen = reg->i_len - reg_offset;
> -			if (rlen <= iclog->ic_size - *log_offset)
> +			if (rlen <= data->iclog->ic_size - data->log_offset)
>  				ophdr->oh_flags |= XLOG_END_TRANS;
>  			else
>  				ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
>  
> -			rlen = min_t(uint32_t, rlen, iclog->ic_size - *log_offset);
> +			rlen = min_t(uint32_t, rlen,
> +				data->iclog->ic_size - data->log_offset);
>  			ophdr->oh_len = cpu_to_be32(rlen);
>  
> -			xlog_write_iovec(iclog, log_offset,
> -					reg->i_addr + reg_offset,
> -					rlen, len, record_cnt, data_cnt);
> -
> +			xlog_write_iovec(data, reg->i_addr + reg_offset, rlen);
>  		} while (ophdr->oh_flags & XLOG_CONTINUE_TRANS);
>  	}
>  
> -	/*
> -	 * No more iovecs remain in this logvec so return the next log vec to
> -	 * the caller so it can go back to fast path copying.
> -	 */
> -	*iclogp = iclog;
>  	return 0;
>  }
>  
> @@ -2144,12 +2117,12 @@ xlog_write(
>  	uint32_t		len)
>  
>  {
> -	struct xlog_in_core	*iclog = NULL;
>  	struct xfs_log_vec	*lv;
> -	uint32_t		record_cnt = 0;
> -	uint32_t		data_cnt = 0;
> -	int			error = 0;
> -	int			log_offset;
> +	struct xlog_write_data	data = {
> +		.ticket		= ticket,
> +		.bytes_left	= len,
> +	};
> +	int			error;
>  
>  	if (ticket->t_curr_res < 0) {
>  		xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES,
> @@ -2158,12 +2131,11 @@ xlog_write(
>  		xlog_force_shutdown(log, SHUTDOWN_LOG_IO_ERROR);
>  	}
>  
> -	error = xlog_state_get_iclog_space(log, len, &iclog, ticket,
> -					   &log_offset);
> +	error = xlog_state_get_iclog_space(log, &data);
>  	if (error)
>  		return error;
>  
> -	ASSERT(log_offset <= iclog->ic_size - 1);
> +	ASSERT(data.log_offset <= data.iclog->ic_size - 1);
>  
>  	/*
>  	 * If we have a context pointer, pass it the first iclog we are
> @@ -2171,7 +2143,7 @@ xlog_write(
>  	 * ordering.
>  	 */
>  	if (ctx)
> -		xlog_cil_set_ctx_write_state(ctx, iclog);
> +		xlog_cil_set_ctx_write_state(ctx, data.iclog);
>  
>  	list_for_each_entry(lv, lv_chain, lv_list) {
>  		/*
> @@ -2179,10 +2151,8 @@ xlog_write(
>  		 * the partial copy loop which can handle this case.
>  		 */
>  		if (lv->lv_niovecs &&
> -		    lv->lv_bytes > iclog->ic_size - log_offset) {
> -			error = xlog_write_partial(lv, ticket, &iclog,
> -					&log_offset, &len, &record_cnt,
> -					&data_cnt);
> +		    lv->lv_bytes > data.iclog->ic_size - data.log_offset) {
> +			error = xlog_write_partial(lv, &data);
>  			if (error) {
>  				/*
>  				 * We have no iclog to release, so just return
> @@ -2191,11 +2161,10 @@ xlog_write(
>  				return error;
>  			}
>  		} else {
> -			xlog_write_full(lv, ticket, iclog, &log_offset,
> -					 &len, &record_cnt, &data_cnt);
> +			xlog_write_full(lv, &data);
>  		}
>  	}
> -	ASSERT(len == 0);
> +	ASSERT(data.bytes_left == 0);
>  
>  	/*
>  	 * We've already been guaranteed that the last writes will fit inside
> @@ -2204,8 +2173,8 @@ xlog_write(
>  	 * iclog with the number of bytes written here.
>  	 */
>  	spin_lock(&log->l_icloglock);
> -	xlog_state_finish_copy(log, iclog, record_cnt, 0);
> -	error = xlog_state_release_iclog(log, iclog, ticket);
> +	xlog_state_finish_copy(log, data.iclog, data.record_cnt, 0);
> +	error = xlog_state_release_iclog(log, data.iclog, ticket);
>  	spin_unlock(&log->l_icloglock);
>  
>  	return error;
> @@ -2527,10 +2496,7 @@ xlog_state_done_syncing(
>  STATIC int
>  xlog_state_get_iclog_space(
>  	struct xlog		*log,
> -	int			len,
> -	struct xlog_in_core	**iclogp,
> -	struct xlog_ticket	*ticket,
> -	int			*logoffsetp)
> +	struct xlog_write_data	*data)
>  {
>  	int			log_offset;
>  	struct xlog_rec_header	*head;
> @@ -2565,7 +2531,7 @@ xlog_state_get_iclog_space(
>  	 * must be written.
>  	 */
>  	if (log_offset == 0) {
> -		ticket->t_curr_res -= log->l_iclog_hsize;
> +		data->ticket->t_curr_res -= log->l_iclog_hsize;
>  		head->h_cycle = cpu_to_be32(log->l_curr_cycle);
>  		head->h_lsn = cpu_to_be64(
>  			xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block));
> @@ -2595,7 +2561,8 @@ xlog_state_get_iclog_space(
>  		 * reference to the iclog.
>  		 */
>  		if (!atomic_add_unless(&iclog->ic_refcnt, -1, 1))
> -			error = xlog_state_release_iclog(log, iclog, ticket);
> +			error = xlog_state_release_iclog(log, iclog,
> +					data->ticket);
>  		spin_unlock(&log->l_icloglock);
>  		if (error)
>  			return error;
> @@ -2608,16 +2575,16 @@ xlog_state_get_iclog_space(
>  	 * iclogs (to mark it taken), this particular iclog will release/sync
>  	 * to disk in xlog_write().
>  	 */
> -	if (len <= iclog->ic_size - iclog->ic_offset)
> -		iclog->ic_offset += len;
> +	if (data->bytes_left <= iclog->ic_size - iclog->ic_offset)
> +		iclog->ic_offset += data->bytes_left;
>  	else
>  		xlog_state_switch_iclogs(log, iclog, iclog->ic_size);
> -	*iclogp = iclog;
> +	data->iclog = iclog;
>  
>  	ASSERT(iclog->ic_offset <= iclog->ic_size);
>  	spin_unlock(&log->l_icloglock);
>  
> -	*logoffsetp = log_offset;
> +	data->log_offset = log_offset;
>  	return 0;
>  }
>  
> -- 
> 2.47.3
> 
> 

Re: [PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers

[Christoph Hellwig]

On Fri, Oct 31, 2025 at 08:26:32PM -0700, Darrick J. Wong wrote:
> > $ size fs/xfs/xfs_log.o*
> >    text	   data	    bss	    dec	    hex	filename
> >   26330	   1292	      8	  27630	   6bee	fs/xfs/xfs_log.o
> >   26158	   1292	      8	  27458	   6b42	fs/xfs/xfs_log.o.old
> 
> Um... assuming xfs_log.o is the post-patch object file, this shows the
> text size going up by 172 bytes, right?

That one does.  I was pretty sure it was the other way around, so І
re-run this with the current tree:

$ size fs/xfs/xfs_log.o*
   text	   data	    bss	    dec	    hex	filename
  29300	   1730	    176	  31206	   79e6	fs/xfs/xfs_log.o
  29160	   1730	    176	  31066	   795a	fs/xfs/xfs_log.o.old

but yes, this grows the text size somewhat unexpectedly.

I still like the new code, but I guess this now counts as a pessimization :(

Re: [PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers

[Darrick J. Wong]

On Mon, Nov 03, 2025 at 11:46:58AM +0100, Christoph Hellwig wrote:
> On Fri, Oct 31, 2025 at 08:26:32PM -0700, Darrick J. Wong wrote:
> > > $ size fs/xfs/xfs_log.o*
> > >    text	   data	    bss	    dec	    hex	filename
> > >   26330	   1292	      8	  27630	   6bee	fs/xfs/xfs_log.o
> > >   26158	   1292	      8	  27458	   6b42	fs/xfs/xfs_log.o.old
> > 
> > Um... assuming xfs_log.o is the post-patch object file, this shows the
> > text size going up by 172 bytes, right?
> 
> That one does.  I was pretty sure it was the other way around, so І
> re-run this with the current tree:
> 
> $ size fs/xfs/xfs_log.o*
>    text	   data	    bss	    dec	    hex	filename
>   29300	   1730	    176	  31206	   79e6	fs/xfs/xfs_log.o
>   29160	   1730	    176	  31066	   795a	fs/xfs/xfs_log.o.old
> 
> but yes, this grows the text size somewhat unexpectedly.
> 
> I still like the new code, but I guess this now counts as a pessimization :(

"Cleaning up the method signatures is worth 140 bytes" :)

--D

Re: [PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers

[Christoph Hellwig]

On Tue, Nov 04, 2025 at 03:40:09PM -0800, Darrick J. Wong wrote:
> On Mon, Nov 03, 2025 at 11:46:58AM +0100, Christoph Hellwig wrote:
> > On Fri, Oct 31, 2025 at 08:26:32PM -0700, Darrick J. Wong wrote:
> > > > $ size fs/xfs/xfs_log.o*
> > > >    text	   data	    bss	    dec	    hex	filename
> > > >   26330	   1292	      8	  27630	   6bee	fs/xfs/xfs_log.o
> > > >   26158	   1292	      8	  27458	   6b42	fs/xfs/xfs_log.o.old
> > > 
> > > Um... assuming xfs_log.o is the post-patch object file, this shows the
> > > text size going up by 172 bytes, right?
> > 
> > That one does.  I was pretty sure it was the other way around, so І
> > re-run this with the current tree:
> > 
> > $ size fs/xfs/xfs_log.o*
> >    text	   data	    bss	    dec	    hex	filename
> >   29300	   1730	    176	  31206	   79e6	fs/xfs/xfs_log.o
> >   29160	   1730	    176	  31066	   795a	fs/xfs/xfs_log.o.old
> > 
> > but yes, this grows the text size somewhat unexpectedly.
> > 
> > I still like the new code, but I guess this now counts as a pessimization :(
> 
> "Cleaning up the method signatures is worth 140 bytes" :)

Hopefuly.  I have to say I really much, much prefer the new calling
conventions.  But I'm also confused that it increased code size, as
usually this kind of struct packing for deep call chains reduces code
size.

[PATCH 08/10] xfs: add a xlog_write_space_left helper

[Christoph Hellwig]

Various places check how much space is left in the current iclog,
add a helper for that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 2b1744af8a67..7c751665bc44 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1878,13 +1878,18 @@ xlog_print_trans(
 	}
 }
 
+static inline uint32_t xlog_write_space_left(struct xlog_write_data *data)
+{
+	return data->iclog->ic_size - data->log_offset;
+}
+
 static inline void
 xlog_write_iovec(
 	struct xlog_write_data	*data,
 	void			*buf,
 	uint32_t		buf_len)
 {
-	ASSERT(data->log_offset < data->iclog->ic_log->l_iclog_size);
+	ASSERT(xlog_write_space_left(data) > 0);
 	ASSERT(data->log_offset % sizeof(int32_t) == 0);
 	ASSERT(buf_len % sizeof(int32_t) == 0);
 
@@ -1906,7 +1911,7 @@ xlog_write_full(
 {
 	int			index;
 
-	ASSERT(data->log_offset + data->bytes_left <= data->iclog->ic_size ||
+	ASSERT(data->bytes_left <= xlog_write_space_left(data) ||
 		data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
 
 	/*
@@ -1978,7 +1983,7 @@ xlog_write_partial(
 		 * Hence if there isn't space for region data after the
 		 * opheader, then we need to start afresh with a new iclog.
 		 */
-		if (data->iclog->ic_size - data->log_offset <=
+		if (xlog_write_space_left(data) <=
 					sizeof(struct xlog_op_header)) {
 			error = xlog_write_get_more_iclog_space(data);
 			if (error)
@@ -1986,8 +1991,7 @@ xlog_write_partial(
 		}
 
 		ophdr = reg->i_addr;
-		rlen = min_t(uint32_t, reg->i_len,
-			data->iclog->ic_size - data->log_offset);
+		rlen = min_t(uint32_t, reg->i_len, xlog_write_space_left(data));
 
 		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
 		ophdr->oh_len = cpu_to_be32(rlen - sizeof(struct xlog_op_header));
@@ -2052,13 +2056,13 @@ xlog_write_partial(
 			 */
 			reg_offset += rlen;
 			rlen = reg->i_len - reg_offset;
-			if (rlen <= data->iclog->ic_size - data->log_offset)
+			if (rlen <= xlog_write_space_left(data))
 				ophdr->oh_flags |= XLOG_END_TRANS;
 			else
 				ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
 
 			rlen = min_t(uint32_t, rlen,
-				data->iclog->ic_size - data->log_offset);
+					xlog_write_space_left(data));
 			ophdr->oh_len = cpu_to_be32(rlen);
 
 			xlog_write_iovec(data, reg->i_addr + reg_offset, rlen);
@@ -2135,7 +2139,7 @@ xlog_write(
 	if (error)
 		return error;
 
-	ASSERT(data.log_offset <= data.iclog->ic_size - 1);
+	ASSERT(xlog_write_space_left(&data) > 0);
 
 	/*
 	 * If we have a context pointer, pass it the first iclog we are
@@ -2151,7 +2155,7 @@ xlog_write(
 		 * the partial copy loop which can handle this case.
 		 */
 		if (lv->lv_niovecs &&
-		    lv->lv_bytes > data.iclog->ic_size - data.log_offset) {
+		    lv->lv_bytes > xlog_write_space_left(&data)) {
 			error = xlog_write_partial(lv, &data);
 			if (error) {
 				/*
-- 
2.47.3

Re: [PATCH 08/10] xfs: add a xlog_write_space_left helper

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:18PM +0100, Christoph Hellwig wrote:
> Various places check how much space is left in the current iclog,
> add a helper for that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 2b1744af8a67..7c751665bc44 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1878,13 +1878,18 @@ xlog_print_trans(
>  	}
>  }
>  
> +static inline uint32_t xlog_write_space_left(struct xlog_write_data *data)
> +{
> +	return data->iclog->ic_size - data->log_offset;
> +}
> +
>  static inline void
>  xlog_write_iovec(
>  	struct xlog_write_data	*data,
>  	void			*buf,
>  	uint32_t		buf_len)
>  {
> -	ASSERT(data->log_offset < data->iclog->ic_log->l_iclog_size);
> +	ASSERT(xlog_write_space_left(data) > 0);
>  	ASSERT(data->log_offset % sizeof(int32_t) == 0);
>  	ASSERT(buf_len % sizeof(int32_t) == 0);
>  
> @@ -1906,7 +1911,7 @@ xlog_write_full(
>  {
>  	int			index;
>  
> -	ASSERT(data->log_offset + data->bytes_left <= data->iclog->ic_size ||
> +	ASSERT(data->bytes_left <= xlog_write_space_left(data) ||
>  		data->iclog->ic_state == XLOG_STATE_WANT_SYNC);
>  
>  	/*
> @@ -1978,7 +1983,7 @@ xlog_write_partial(
>  		 * Hence if there isn't space for region data after the
>  		 * opheader, then we need to start afresh with a new iclog.
>  		 */
> -		if (data->iclog->ic_size - data->log_offset <=
> +		if (xlog_write_space_left(data) <=
>  					sizeof(struct xlog_op_header)) {
>  			error = xlog_write_get_more_iclog_space(data);
>  			if (error)
> @@ -1986,8 +1991,7 @@ xlog_write_partial(
>  		}
>  
>  		ophdr = reg->i_addr;
> -		rlen = min_t(uint32_t, reg->i_len,
> -			data->iclog->ic_size - data->log_offset);
> +		rlen = min_t(uint32_t, reg->i_len, xlog_write_space_left(data));
>  
>  		ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
>  		ophdr->oh_len = cpu_to_be32(rlen - sizeof(struct xlog_op_header));
> @@ -2052,13 +2056,13 @@ xlog_write_partial(
>  			 */
>  			reg_offset += rlen;
>  			rlen = reg->i_len - reg_offset;
> -			if (rlen <= data->iclog->ic_size - data->log_offset)
> +			if (rlen <= xlog_write_space_left(data))
>  				ophdr->oh_flags |= XLOG_END_TRANS;
>  			else
>  				ophdr->oh_flags |= XLOG_CONTINUE_TRANS;
>  
>  			rlen = min_t(uint32_t, rlen,
> -				data->iclog->ic_size - data->log_offset);
> +					xlog_write_space_left(data));
>  			ophdr->oh_len = cpu_to_be32(rlen);
>  
>  			xlog_write_iovec(data, reg->i_addr + reg_offset, rlen);
> @@ -2135,7 +2139,7 @@ xlog_write(
>  	if (error)
>  		return error;
>  
> -	ASSERT(data.log_offset <= data.iclog->ic_size - 1);
> +	ASSERT(xlog_write_space_left(&data) > 0);
>  
>  	/*
>  	 * If we have a context pointer, pass it the first iclog we are
> @@ -2151,7 +2155,7 @@ xlog_write(
>  		 * the partial copy loop which can handle this case.
>  		 */
>  		if (lv->lv_niovecs &&
> -		    lv->lv_bytes > data.iclog->ic_size - data.log_offset) {
> +		    lv->lv_bytes > xlog_write_space_left(&data)) {
>  			error = xlog_write_partial(lv, &data);
>  			if (error) {
>  				/*
> -- 
> 2.47.3
> 
> 

[PATCH 09/10] xfs: improve the iclog space assert in xlog_write_iovec

[Christoph Hellwig]

We need enough space for the length we copy into the iclog, not just
some space, so tighten up the check a bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 7c751665bc44..8b8fdef6414d 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1889,7 +1889,7 @@ xlog_write_iovec(
 	void			*buf,
 	uint32_t		buf_len)
 {
-	ASSERT(xlog_write_space_left(data) > 0);
+	ASSERT(xlog_write_space_left(data) >= buf_len);
 	ASSERT(data->log_offset % sizeof(int32_t) == 0);
 	ASSERT(buf_len % sizeof(int32_t) == 0);
 
-- 
2.47.3

Re: [PATCH 09/10] xfs: improve the iclog space assert in xlog_write_iovec

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:19PM +0100, Christoph Hellwig wrote:
> We need enough space for the length we copy into the iclog, not just
> some space, so tighten up the check a bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Makes sense to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 7c751665bc44..8b8fdef6414d 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1889,7 +1889,7 @@ xlog_write_iovec(
>  	void			*buf,
>  	uint32_t		buf_len)
>  {
> -	ASSERT(xlog_write_space_left(data) > 0);
> +	ASSERT(xlog_write_space_left(data) >= buf_len);
>  	ASSERT(data->log_offset % sizeof(int32_t) == 0);
>  	ASSERT(buf_len % sizeof(int32_t) == 0);
>  
> -- 
> 2.47.3
> 
> 

[PATCH 10/10] xfs: factor out a xlog_write_space_advance helper

[Christoph Hellwig]

Add a new xlog_write_space_advance that returns the current place in the
iclog that data is written to, and advances the various counters by the
amount taken from xlog_write_iovec, and also use it xlog_write_partial,
which open codes the counter adjustments, but misses the asserts.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 8b8fdef6414d..511756429336 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1883,21 +1883,31 @@ static inline uint32_t xlog_write_space_left(struct xlog_write_data *data)
 	return data->iclog->ic_size - data->log_offset;
 }
 
+static void *
+xlog_write_space_advance(
+	struct xlog_write_data	*data,
+	unsigned int		len)
+{
+	void			*p = data->iclog->ic_datap + data->log_offset;
+
+	ASSERT(xlog_write_space_left(data) >= len);
+	ASSERT(data->log_offset % sizeof(int32_t) == 0);
+	ASSERT(len % sizeof(int32_t) == 0);
+
+	data->data_cnt += len;
+	data->log_offset += len;
+	data->bytes_left -= len;
+	return p;
+}
+
 static inline void
 xlog_write_iovec(
 	struct xlog_write_data	*data,
 	void			*buf,
 	uint32_t		buf_len)
 {
-	ASSERT(xlog_write_space_left(data) >= buf_len);
-	ASSERT(data->log_offset % sizeof(int32_t) == 0);
-	ASSERT(buf_len % sizeof(int32_t) == 0);
-
-	memcpy(data->iclog->ic_datap + data->log_offset, buf, buf_len);
-	data->log_offset += buf_len;
-	data->bytes_left -= buf_len;
+	memcpy(xlog_write_space_advance(data, buf_len), buf, buf_len);
 	data->record_cnt++;
-	data->data_cnt += buf_len;
 }
 
 /*
@@ -2038,7 +2048,8 @@ xlog_write_partial(
 			if (error)
 				return error;
 
-			ophdr = data->iclog->ic_datap + data->log_offset;
+			ophdr = xlog_write_space_advance(data,
+					sizeof(struct xlog_op_header));
 			ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
 			ophdr->oh_clientid = XFS_TRANSACTION;
 			ophdr->oh_res2 = 0;
@@ -2046,9 +2057,6 @@ xlog_write_partial(
 
 			data->ticket->t_curr_res -=
 				sizeof(struct xlog_op_header);
-			data->log_offset += sizeof(struct xlog_op_header);
-			data->data_cnt += sizeof(struct xlog_op_header);
-			data->bytes_left -= sizeof(struct xlog_op_header);
 
 			/*
 			 * If rlen fits in the iclog, then end the region
-- 
2.47.3

Re: [PATCH 10/10] xfs: factor out a xlog_write_space_advance helper

[Darrick J. Wong]

On Thu, Oct 30, 2025 at 03:49:20PM +0100, Christoph Hellwig wrote:
> Add a new xlog_write_space_advance that returns the current place in the
> iclog that data is written to, and advances the various counters by the
> amount taken from xlog_write_iovec, and also use it xlog_write_partial,
> which open codes the counter adjustments, but misses the asserts.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks decent,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 8b8fdef6414d..511756429336 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1883,21 +1883,31 @@ static inline uint32_t xlog_write_space_left(struct xlog_write_data *data)
>  	return data->iclog->ic_size - data->log_offset;
>  }
>  
> +static void *
> +xlog_write_space_advance(
> +	struct xlog_write_data	*data,
> +	unsigned int		len)
> +{
> +	void			*p = data->iclog->ic_datap + data->log_offset;
> +
> +	ASSERT(xlog_write_space_left(data) >= len);
> +	ASSERT(data->log_offset % sizeof(int32_t) == 0);
> +	ASSERT(len % sizeof(int32_t) == 0);
> +
> +	data->data_cnt += len;
> +	data->log_offset += len;
> +	data->bytes_left -= len;
> +	return p;
> +}
> +
>  static inline void
>  xlog_write_iovec(
>  	struct xlog_write_data	*data,
>  	void			*buf,
>  	uint32_t		buf_len)
>  {
> -	ASSERT(xlog_write_space_left(data) >= buf_len);
> -	ASSERT(data->log_offset % sizeof(int32_t) == 0);
> -	ASSERT(buf_len % sizeof(int32_t) == 0);
> -
> -	memcpy(data->iclog->ic_datap + data->log_offset, buf, buf_len);
> -	data->log_offset += buf_len;
> -	data->bytes_left -= buf_len;
> +	memcpy(xlog_write_space_advance(data, buf_len), buf, buf_len);
>  	data->record_cnt++;
> -	data->data_cnt += buf_len;
>  }
>  
>  /*
> @@ -2038,7 +2048,8 @@ xlog_write_partial(
>  			if (error)
>  				return error;
>  
> -			ophdr = data->iclog->ic_datap + data->log_offset;
> +			ophdr = xlog_write_space_advance(data,
> +					sizeof(struct xlog_op_header));
>  			ophdr->oh_tid = cpu_to_be32(data->ticket->t_tid);
>  			ophdr->oh_clientid = XFS_TRANSACTION;
>  			ophdr->oh_res2 = 0;
> @@ -2046,9 +2057,6 @@ xlog_write_partial(
>  
>  			data->ticket->t_curr_res -=
>  				sizeof(struct xlog_op_header);
> -			data->log_offset += sizeof(struct xlog_op_header);
> -			data->data_cnt += sizeof(struct xlog_op_header);
> -			data->bytes_left -= sizeof(struct xlog_op_header);
>  
>  			/*
>  			 * If rlen fits in the iclog, then end the region
> -- 
> 2.47.3
> 
> 

cleanup log item formatting v3

[Christoph Hellwig]

Hi all,

I dug into a rabit hole about the log item formatting recently,
and noticed that the handling of the opheaders is still pretty
ugly because it leaks pre-delayed logging implementation
details into the log item implementations.

The core of this series is to remove the to reserve space in the
CIL buffers/shadow buffers for the opheaders that already were
generated more or less on the fly by the lowlevel log write
code anyway, but there's lots of other cleanups around it.

Changes since v2:
 - rebased to the latest xfs-6.19-merge branch
 - expand and improve a few commit messages
 - add a Fixes tag

Changes since v1:
 - rebased and dropped the already merged patches

Diffstat:
 libxfs/xfs_log_format.h |    7 -
 xfs_attr_item.c         |   27 +---
 xfs_bmap_item.c         |   10 -
 xfs_buf_item.c          |   19 +--
 xfs_dquot_item.c        |    9 -
 xfs_exchmaps_item.c     |   11 -
 xfs_extfree_item.c      |   10 -
 xfs_icreate_item.c      |    6 
 xfs_inode_item.c        |   49 +++-----
 xfs_log.c               |  292 ++++++++++++++++++------------------------------
 xfs_log.h               |   65 +---------
 xfs_log_cil.c           |  111 ++++++++++++++++--
 xfs_log_priv.h          |   20 +++
 xfs_refcount_item.c     |   10 -
 xfs_rmap_item.c         |   10 -
 xfs_trans.h             |    4 
 16 files changed, 313 insertions(+), 347 deletions(-)

[PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial

[Christoph Hellwig]

When xlog_write_partial splits a log region over multiple iclogs, it
has to include the continuation ophder in the length requested for the
new iclog.  Currently is simply adds that to the request, which makes
the accounting of the used space below look slightly different from the
other users of iclog space that decrement it.

To prepare for more code sharing, add the ophdr size to the len variable
that tracks the number of bytes still are left in this xlog_write
operation before the calling xlog_write_get_more_iclog_space, and then
decrement it later when consuming that space.

This changes the value of len when xlog_write_get_more_iclog_space
returns an error, but as nothing looks at len in that case the
difference doesn't matter.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 93e99d1cc037..539b22dff2d1 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2048,10 +2048,10 @@ xlog_write_partial(
 			 * consumes hasn't been accounted to the lv we are
 			 * writing.
 			 */
+			*len += sizeof(struct xlog_op_header);
 			error = xlog_write_get_more_iclog_space(ticket,
-					&iclog, log_offset,
-					*len + sizeof(struct xlog_op_header),
-					record_cnt, data_cnt);
+					&iclog, log_offset, *len, record_cnt,
+					data_cnt);
 			if (error)
 				return error;
 
@@ -2064,6 +2064,7 @@ xlog_write_partial(
 			ticket->t_curr_res -= sizeof(struct xlog_op_header);
 			*log_offset += sizeof(struct xlog_op_header);
 			*data_cnt += sizeof(struct xlog_op_header);
+			*len -= sizeof(struct xlog_op_header);
 
 			/*
 			 * If rlen fits in the iclog, then end the region
-- 
2.47.3

Re: [PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial

[Darrick J. Wong]

On Wed, Nov 12, 2025 at 01:14:22PM +0100, Christoph Hellwig wrote:
> When xlog_write_partial splits a log region over multiple iclogs, it
> has to include the continuation ophder in the length requested for the
> new iclog.  Currently is simply adds that to the request, which makes
> the accounting of the used space below look slightly different from the
> other users of iclog space that decrement it.
> 
> To prepare for more code sharing, add the ophdr size to the len variable
> that tracks the number of bytes still are left in this xlog_write
> operation before the calling xlog_write_get_more_iclog_space, and then
> decrement it later when consuming that space.
> 
> This changes the value of len when xlog_write_get_more_iclog_space
> returns an error, but as nothing looks at len in that case the
> difference doesn't matter.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Ok, glad I took the time to ask questions for v2;
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 93e99d1cc037..539b22dff2d1 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2048,10 +2048,10 @@ xlog_write_partial(
>  			 * consumes hasn't been accounted to the lv we are
>  			 * writing.
>  			 */
> +			*len += sizeof(struct xlog_op_header);
>  			error = xlog_write_get_more_iclog_space(ticket,
> -					&iclog, log_offset,
> -					*len + sizeof(struct xlog_op_header),
> -					record_cnt, data_cnt);
> +					&iclog, log_offset, *len, record_cnt,
> +					data_cnt);
>  			if (error)
>  				return error;
>  
> @@ -2064,6 +2064,7 @@ xlog_write_partial(
>  			ticket->t_curr_res -= sizeof(struct xlog_op_header);
>  			*log_offset += sizeof(struct xlog_op_header);
>  			*data_cnt += sizeof(struct xlog_op_header);
> +			*len -= sizeof(struct xlog_op_header);
>  
>  			/*
>  			 * If rlen fits in the iclog, then end the region
> -- 
> 2.47.3
> 
>