Re: [PATCH v11 2/2] xfs: add FALLOC_FL_WRITE_ZEROES to XFS code base

[Dave Chinner <dgc@kernel.org>]

On Thu, Mar 12, 2026 at 09:36:50PM +0000, Pankaj Raghav (Samsung) wrote:
> > > +
> > > +	len = round_up(offset + len, blksize) - round_down(offset, blksize);
> > > +	offset = round_down(offset, blksize);
> > > +	error = xfs_alloc_file_space(ip, offset, len, XFS_BMAPI_PREALLOC);
> > 
> > xfs_alloc_file_space() already rounds the offset down and the range
> > end upwards to block size boundaries (XFS_B_TO_FSBT() rounds down,
> > XFS_B_TO_FSB rounds up). There is no need to do it here.
> > 
> > > +
> > > +set_filesize:
> > > +	if (error)
> > > +		return error;
> > > +
> > > +	error = xfs_falloc_setsize(file, new_size);
> > > +	if (error)
> > > +		return error;
> > > +	if (need_convert)
> > > +		error = xfs_alloc_file_space(ip, offset, len,
> > > +				XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO);
> > > +	return error;
> > 
> > Honestly, this "prealloc, set file size, then convert and zero" thing
> > seems a bit ... clunky. 
> > 
> > I mean, this is trying to work around an issue with
> > xfs_falloc_setsize() operating on written extents beyond EOF to set
> > EOF. But why are we even using a generic truncate operation to set
> > the EOF when we have already done most of the complex truncate work
> > (exclusion, page cache invalidation, extent removal, etc) already?
> 
> I agree.
> 
> > 
> > My logic:
> > 
> > xfs_bmapi_write(off, len, XFS_BMAPI_ZERO) will allocate all the
> > holes and delalloc extents over the given range as written extents
> > that are initialised to contain zeroes.
> > 
> > xfs_bmapi_write(off, len, XFS_BMAPI_CONVERT) will convert all the
> > unwritten extents over a given range to written extents, but will
> > not touch the data in those extents.
> > 
> > xfs_bmapi_write(off, len, XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO) will
> > allocate holes and delalloc extents and convert unwritten extents
> > all to written extents, and it will write zeros to all of those
> > extents.
> > 
> > This latter case is exactly what we want, and if gives us zeroes on
> > disk to the end of the extent beyond the new EOF. i.e. we've
> > completed all the data IO, and so now all we need
> > to do is update the file size transactionally, just like we do in
> > IO completion.
> > 
> > Indeed, xfs_iomap_write_unwritten() integrates this size update
> > into post-IO unwritten extent conversion. i.e we already have code
> > that does exactly what we need, except for tweaking the
> > xfs_bmapi_write() flags for the zeroing behaviour we need.
> 
> This makes sense.
> 
> > 
> > So, a helper function in fs/xfs/xfs_bmap_util.c such as this:
> > 
> > /*
> >  * This function is used to allocate written extents over holes
> >  * and/or convert unwritten extents to written extents based on the
> >  * @flags passed to it.
> >  *
> >  * If @flags is zero, if will allocate written extents for holes and
> 
> s/zero/XFS_BMAPI_ZERO ?

No, I explicitly meant flags = 0.

> >  * delalloc extents across the range.
> >  *
> >  * If XFS_BMAPI_CONVERT is specified in @flags, then it will also do
> >  * conversion of unwritten extents in the range to written extents.
> >  *
> >  * If XFS_BMAPI_ZERO is specified in @flags, then both newly
> 
> s/XFS_BMAPI_ZERO/(XFS_BMAPI_ZERO | XFS_BMAPI_CONVERT) ?

No I explictly describing what happens when you do "flags |=
XFS_BMAPI_ZERO;" to any other xfs_bmapi_write() operation
flag specification.

i.e. XFS_BMAPI_ZERO is an optional behavioural modifier for
allocation and/or unwritten extent conversion operations. i.e all it
does is add initialisation of the new/modified extents to contain
real zeroes.

e.g. flags = 0 means "allocate holes/delalloc as uninitialised
written extents". flags = 0 | XFS_BMAPI_ZERO
means "allocate holes/delalloc and initialise them to contain
zeros". Unwritten extents will not be modified by either operation.

Cheers,

Dave.
-- 
Dave Chinner
dgc@kernel.org