Re: [PATCH v4 19/25] xfs: remove unwritten extents after preallocations in fsverity metadata

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Mar 09, 2026 at 08:23:34PM +0100, Andrey Albershteyn wrote:
> XFS preallocates spaces during writes. In normal I/O this space, if
> unused, is removed by truncate. For files with fsverity, XFS does not
> use truncate as fsverity metadata is stored past EOF.
> 
> After we're done with writing fsverity metadata iterate over extents in
> that region and remove any unwritten ones. These would be preallocation
> leftovers in the merkle tree holes and past fsverity descriptor.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
>  fs/xfs/xfs_fsverity.c | 62 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> index e5cd17ec15b6..f78e5f0c2fd0 100644
> --- a/fs/xfs/xfs_fsverity.c
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -21,6 +21,8 @@
>  #include "xfs_iomap.h"
>  #include "xfs_error.h"
>  #include "xfs_health.h"
> +#include "xfs_bmap.h"
> +#include "xfs_bmap_util.h"
>  #include <linux/fsverity.h>
>  #include <linux/iomap.h>
>  #include <linux/pagemap.h>
> @@ -189,6 +191,58 @@ xfs_fsverity_delete_metadata(
>  	return error;
>  }
>  
> +static int
> +xfs_fsverity_cancel_unwritten(
> +	struct xfs_inode	*ip,
> +	xfs_fileoff_t		start,
> +	xfs_fileoff_t		end)
> +{
> +	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_trans	*tp;
> +	xfs_fileoff_t		offset_fsb = XFS_B_TO_FSB(mp, start);
> +	xfs_fileoff_t		end_fsb = XFS_B_TO_FSB(mp, end);
> +	struct xfs_bmbt_irec	imap;
> +	int			nimaps;
> +	int			error = 0;
> +	int			done;
> +
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, 0, &tp);
> +	if (error)
> +		return error;
> +
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +	xfs_trans_ijoin(tp, ip, 0);
> +
> +	while (offset_fsb < end_fsb) {
> +		nimaps = 1;
> +
> +		error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
> +							  &imap, &nimaps, 0);

Indent two tabs, plz. :)

> +		if (error)
> +			goto out_cancel;
> +
> +		if (nimaps == 0)
> +			break;
> +
> +		if (imap.br_state == XFS_EXT_UNWRITTEN) {
> +			error = xfs_bunmapi(tp, ip, imap.br_startoff,
> +					    imap.br_blockcount, 0, 1, &done);
> +			if (error)
> +				goto out_cancel;
> +		}
> +
> +		offset_fsb = imap.br_startoff + imap.br_blockcount;

There's an upper limit on the number of blocks you can unmap/free in a
single transaction.  Maybe move the xfs_trans_{alloc,commit} into the
loop body?

Oh wait, you skip the written extents.  Ok, so maybe just roll it after
you've done a bunmapi.

Do you need to purge the cow fork too?

--D

> +	}
> +
> +	error = xfs_trans_commit(tp);
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +	return error;
> +out_cancel:
> +	xfs_trans_cancel(tp);
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +	return error;
> +}
> +
>  
>  /*
>   * Prepare to enable fsverity by clearing old metadata.
> @@ -264,6 +318,14 @@ xfs_fsverity_end_enable(
>  	if (error)
>  		goto out;
>  
> +	/*
> +	 * Remove unwritten extents left by preallocations in the merkle tree
> +	 * holes and past descriptor
> +	 */
> +	error = xfs_fsverity_cancel_unwritten(ip, range_start, LLONG_MAX);
> +	if (error)
> +		goto out;
> +
>  	/*
>  	 * Set fsverity inode flag
>  	 */
> -- 
> 2.51.2
> 
>