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

Andrey Albershteyn
2025-07-31T11:43:04Z

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().

--
- Andrey