Re: [PATCH RFC 20/29] xfs: disable preallocations for fsverity Merkle tree writes

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

On Thu, Jul 31, 2025 at 01:42:54PM +0200, Andrey Albershteyn wrote:
> On 2025-07-29 15:27:36, Darrick J. Wong wrote:
> > On Mon, Jul 28, 2025 at 10:30:24PM +0200, Andrey Albershteyn wrote:
> > > While writing Merkle tree, file is read-only and there's no further
> > > writes except Merkle tree building. The file is truncated beforehand to
> > > remove any preallocated extents.
> > > 
> > > The Merkle tree is the only data XFS will write. As we don't want XFS to
> > > truncate file after we done writing, let's also skip truncation on
> > > fsverity files. Therefore, we also need to disable preallocations while
> > > writing merkle tree as we don't want any unused extents past the tree.
> > > 
> > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> > > ---
> > >  fs/xfs/xfs_iomap.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > > index ff05e6b1b0bb..00ec1a738b39 100644
> > > --- a/fs/xfs/xfs_iomap.c
> > > +++ b/fs/xfs/xfs_iomap.c
> > > @@ -32,6 +32,8 @@
> > >  #include "xfs_rtbitmap.h"
> > >  #include "xfs_icache.h"
> > >  #include "xfs_zone_alloc.h"
> > > +#include "xfs_fsverity.h"
> > > +#include <linux/fsverity.h>
> > 
> > What do these includes pull in for the iflags tests below?
> 
> Probably need to be removed, thanks for noting
> 
> > 
> > >  #define XFS_ALLOC_ALIGN(mp, off) \
> > >  	(((off) >> mp->m_allocsize_log) << mp->m_allocsize_log)
> > > @@ -1849,7 +1851,9 @@ xfs_buffered_write_iomap_begin(
> > >  		 * Determine the initial size of the preallocation.
> > >  		 * We clean up any extra preallocation when the file is closed.
> > >  		 */
> > > -		if (xfs_has_allocsize(mp))
> > > +		if (xfs_iflags_test(ip, XFS_VERITY_CONSTRUCTION))
> > > +			prealloc_blocks = 0;
> > > +		else if (xfs_has_allocsize(mp))
> > >  			prealloc_blocks = mp->m_allocsize_blocks;
> > >  		else if (allocfork == XFS_DATA_FORK)
> > >  			prealloc_blocks = xfs_iomap_prealloc_size(ip, allocfork,
> > > @@ -1976,6 +1980,13 @@ xfs_buffered_write_iomap_end(
> > >  	if (flags & IOMAP_FAULT)
> > >  		return 0;
> > >  
> > > +	/*
> > > +	 * While writing Merkle tree to disk we would not have any other
> > > +	 * delayed allocations
> > > +	 */
> > > +	if (xfs_iflags_test(XFS_I(inode), XFS_VERITY_CONSTRUCTION))
> > > +		return 0;
> > 
> > I assume XFS_VERITY_CONSTRUCTION doesn't get set until after we've
> > locked the inode, flushed the dirty pagecache, and truncated the file to
> > EOF?  In which case I guess this is ok -- we're never going to have new
> > delalloc reservations,
> 
> yes, this is my thinking here
> 
> > and verity data can't be straddling the EOF
> > folio, no matter how large it is.  Right?
> 
> Not sure, what you mean here. In page cache merkle tree is stored
> at (1 << 53) offset, and there's check for file overlapping this in
> patch 22 xfs_fsverity_begin_enable().

Yeah, I hadn't gotten to that patch yet.  That part looks fine to me,
though I sorta wondered why not put it at offset 1<<62 to allow for even
bigger files.  But the impression I've gotten from Eric is that they
don't really want to handle files that huge with merkle trees that big
so the loss of file range address space probably doesn't matter.

--D

> -- 
> - Andrey
> 
>