From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Carlos Maiolino <cem@kernel.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 03/10] xfs: improve the ->iop_format interface
Date: Fri, 31 Oct 2025 17:15:56 -0700 [thread overview]
Message-ID: <20251101001556.GS3356773@frogsfrogsfrogs> (raw)
In-Reply-To: <20251030144946.1372887-4-hch@lst.de>
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
>
>
next prev parent reply other threads:[~2025-11-01 0:15 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-30 14:49 cleanup log item formatting v2 Christoph Hellwig
2025-10-30 14:49 ` [PATCH 01/10] xfs: add a xlog_write_one_vec helper Christoph Hellwig
2025-10-31 23:59 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 02/10] xfs: set lv_bytes in xlog_write_one_vec Christoph Hellwig
2025-11-01 0:04 ` Darrick J. Wong
2025-11-03 10:43 ` Christoph Hellwig
2025-11-04 23:39 ` Darrick J. Wong
2025-11-05 13:27 ` Christoph Hellwig
2025-11-05 22:13 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 03/10] xfs: improve the ->iop_format interface Christoph Hellwig
2025-11-01 0:15 ` Darrick J. Wong [this message]
2025-10-30 14:49 ` [PATCH 04/10] xfs: move struct xfs_log_iovec to xfs_log_priv.h Christoph Hellwig
2025-11-01 1:16 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 05/10] xfs: move struct xfs_log_vec " Christoph Hellwig
2025-11-01 1:16 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 06/10] xfs: regularize iclog space accounting in xlog_write_partial Christoph Hellwig
2025-11-04 23:53 ` Darrick J. Wong
2025-11-05 13:27 ` Christoph Hellwig
2025-10-30 14:49 ` [PATCH 07/10] xfs: improve the calling convention for the xlog_write helpers Christoph Hellwig
2025-11-01 3:26 ` Darrick J. Wong
2025-11-03 10:46 ` Christoph Hellwig
2025-11-04 23:40 ` Darrick J. Wong
2025-11-05 13:28 ` Christoph Hellwig
2025-10-30 14:49 ` [PATCH 08/10] xfs: add a xlog_write_space_left helper Christoph Hellwig
2025-11-01 3:27 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 09/10] xfs: improve the iclog space assert in xlog_write_iovec Christoph Hellwig
2025-11-04 23:45 ` Darrick J. Wong
2025-10-30 14:49 ` [PATCH 10/10] xfs: factor out a xlog_write_space_advance helper Christoph Hellwig
2025-11-04 23:46 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2025-11-12 12:14 cleanup log item formatting v3 Christoph Hellwig
2025-11-12 12:14 ` [PATCH 03/10] xfs: improve the ->iop_format interface Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251101001556.GS3356773@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.