[Added back Cc's. Please use Reply-All instead of Reply!] On Wed, Jan 25, 2023 at 01:22:27PM +0100, Andrey Albershteyn wrote: > On Wed, Dec 14, 2022 at 02:43:04PM -0800, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@xxxxxxxxxx> > > > > fsverity_operations::write_merkle_tree_block is passed the index of the > > block to write and the log base 2 of the block size. However, all > > implementations of it use these parameters only to calculate the > > position and the size of the block, in bytes. > > > > Therefore, make ->write_merkle_tree_block take 'pos' and 'size' > > parameters instead of 'index' and 'log_blocksize'. > > Hi Eric, > > Thanks for the quick responses with changes to fs-verity! > > With this patch shouldn't the read_merkle_tree_block() also change > to [pos, size] args? I see that ext4 uses index to read the page at > that index + file size. But I suppose that when Merkle tree block > size will vary (e.g. 8k) it will require position + size. Not yet. It's actually read_merkle_tree_page(), not read_merkle_tree_block(). The callees want a page index, and pages always have size PAGE_SIZE. So the current function prototype is appropriate for the current design. > In XFS as we store the page under the xattr with "pos" as a name > we also need a "pos" while reading the page. So, currently XFS can > use index << log2(PAGE_SHIFT) or will need to get also log_blocksize > from descriptor. But you definitely need to think about what changes should be made to allow XFS to do the Merkle tree caching the way the other XFS developers want it to do. There will be significantly more to that than potentially changing a function prototype. There's been some discussion of this on the "fs-verity support for XFS" thread, but there's not a detailed proposal yet. Note: you should store Merkle tree blocks in the xattrs instead of "pages", so that the on-disk format isn't tied to the page size. - Eric
