Re: [PATCH v7] xfs: add FALLOC_FL_WRITE_ZEROES to XFS code base

[Christoph Hellwig <hch@lst.de>]

On Tue, Jan 20, 2026 at 02:20:50PM +0100, cem@kernel.org wrote:
> From: Lukas Herbolt <lukas@herbolt.com>
> 
> Add support for FALLOC_FL_WRITE_ZEROES if the underlying device enable
> the unmap write zeroes operation.
> 
> Signed-off-by: Lukas Herbolt <lukas@herbolt.com>
> [cem: rewrite xfs_falloc_zero_range() bits]

Nit: once you modify something substantially and add your marker
you also need to sign off on it.

> ---
> 
> Christoph, Darrick, could you please review/ack this patch again? I
> needed to rewrite the xfs_falloc_zero_range() bits, because it
> conflicted with 66d78a11479c and 8dc15b7a6e59. This version aims mostly
> to remove one of the if-else nested levels to keep it a bit cleaner.

Maybe mention the "merge conflict" in the above note?

> index d36a9aafa8ab..b23f1373116e 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1302,16 +1302,29 @@ xfs_falloc_zero_range(
>  
>  	if (xfs_falloc_force_zero(ip, ac)) {
>  		error = xfs_zero_range(ip, offset, len, ac, NULL);
> +		goto out;
> +	}
>  
> +	error = xfs_free_file_space(ip, offset, len, ac);
> +	if (error)
> +		return error;
> +
> +	len = round_up(offset + len, blksize) - round_down(offset, blksize);
> +	offset = round_down(offset, blksize);
> +
> +	if (mode & FALLOC_FL_WRITE_ZEROES) {
> +		if (xfs_is_always_cow_inode(ip) ||
> +		    !bdev_write_zeroes_unmap_sectors(
> +				xfs_inode_buftarg(ip)->bt_bdev))
> +			return -EOPNOTSUPP;
> +		error = xfs_alloc_file_space(ip, offset, len,
> +					     XFS_BMAPI_ZERO);

Darrick made a good point that we should check the not supported cases
earlier, even if that is an issue in the original version.  Also I don't
think we should hit the force zero case for FALLOC_FL_WRITE_ZEROES.
I.e., this should probably become something like:

	if (mode & FALLOC_FL_WRITE_ZEROES) {
		if (xfs_is_always_cow_inode(ip) ||
		    !bdev_write_zeroes_unmap_sectors(
				xfs_inode_buftarg(ip)->bt_bdev))
			return -EOPNOTSUPP;
		bmapi_flags = XFS_BMAPI_ZERO;
	} else {
	  	if (xfs_falloc_force_zero(ip, ac)) {
	  		error = xfs_zero_range(ip, offset, len, ac, NULL);
			goto set_filesize;
		}
		bmapi_flags = XFS_BMAPI_PREALLOC;
	}

	< free file space, round, etc.. >

	error = xfs_alloc_file_space(ip, offset, len, bmapi_flags);