diff mbox series

[39/40] xfs: don't bother storing merkle tree blocks for zeroed data blocks

Message ID 171069246533.2684506.10607368938981182877.stgit@frogsfrogsfrogs (mailing list archive)
State New
Headers show
Series [01/40] fsverity: remove hash page spin lock | expand

Commit Message

Darrick J. Wong March 17, 2024, 4:33 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Now that fsverity tells our merkle tree io functions about what a hash
of a data block full of zeroes looks like, we can use this information
to avoid writing out merkle tree blocks for sparse regions of the file.
For verified gold master images this can save quite a bit of overhead.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_verity.c |   37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

Comments

Andrey Albershteyn March 18, 2024, 5:56 p.m. UTC | #1
On 2024-03-17 09:33:34, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Now that fsverity tells our merkle tree io functions about what a hash
> of a data block full of zeroes looks like, we can use this information
> to avoid writing out merkle tree blocks for sparse regions of the file.
> For verified gold master images this can save quite a bit of overhead.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

> ---
>  fs/xfs/xfs_verity.c |   37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c
> index abd95bc1ba6e..ba96e7049f61 100644
> --- a/fs/xfs/xfs_verity.c
> +++ b/fs/xfs/xfs_verity.c
> @@ -619,6 +619,20 @@ xfs_verity_read_merkle(
>  	xfs_verity_merkle_key_to_disk(&name, block->offset);
>  
>  	error = xfs_attr_get(&args);
> +	if (error == -ENOATTR) {
> +		u8		*p;
> +		unsigned int	i;
> +
> +		/*
> +		 * No attribute found.  Synthesize a buffer full of the zero
> +		 * digests on the assumption that we elided them at write time.
> +		 */
> +		for (i = 0, p = new_mk->data;
> +		     i < block->size;
> +		     i += req->digest_size, p += req->digest_size)
> +			memcpy(p, req->zero_digest, req->digest_size);
> +		error = 0;
> +	}
>  	if (error)
>  		goto out_new_mk;
>  
> @@ -676,12 +690,29 @@ xfs_verity_write_merkle(
>  		.value			= (void *)buf,
>  		.valuelen		= size,
>  	};
> -	const char			*p = buf + size - 1;
> +	const char			*p;
> +	unsigned int			i;
>  
> -	/* Don't store trailing zeroes. */
> +	/*
> +	 * If this is a block full of hashes of zeroed blocks, don't bother
> +	 * storing the block.  We can synthesize them later.
> +	 */
> +	for (i = 0, p = buf;
> +	     i < size;
> +	     i += req->digest_size, p += req->digest_size)
> +		if (memcmp(p, req->zero_digest, req->digest_size))
> +			break;
> +	if (i == size)
> +		return 0;
> +
> +	/*
> +	 * Don't store trailing zeroes.  Store at least one byte so that the
> +	 * block cannot be mistaken for an elided one.
> +	 */
> +	p = buf + size - 1;
>  	while (p >= (const char *)buf && *p == 0)
>  		p--;
> -	args.valuelen = p - (const char *)buf + 1;
> +	args.valuelen = max(1, p - (const char *)buf + 1);
>  
>  	xfs_verity_merkle_key_to_disk(&name, pos);
>  	return xfs_attr_set(&args);
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c
index abd95bc1ba6e..ba96e7049f61 100644
--- a/fs/xfs/xfs_verity.c
+++ b/fs/xfs/xfs_verity.c
@@ -619,6 +619,20 @@  xfs_verity_read_merkle(
 	xfs_verity_merkle_key_to_disk(&name, block->offset);
 
 	error = xfs_attr_get(&args);
+	if (error == -ENOATTR) {
+		u8		*p;
+		unsigned int	i;
+
+		/*
+		 * No attribute found.  Synthesize a buffer full of the zero
+		 * digests on the assumption that we elided them at write time.
+		 */
+		for (i = 0, p = new_mk->data;
+		     i < block->size;
+		     i += req->digest_size, p += req->digest_size)
+			memcpy(p, req->zero_digest, req->digest_size);
+		error = 0;
+	}
 	if (error)
 		goto out_new_mk;
 
@@ -676,12 +690,29 @@  xfs_verity_write_merkle(
 		.value			= (void *)buf,
 		.valuelen		= size,
 	};
-	const char			*p = buf + size - 1;
+	const char			*p;
+	unsigned int			i;
 
-	/* Don't store trailing zeroes. */
+	/*
+	 * If this is a block full of hashes of zeroed blocks, don't bother
+	 * storing the block.  We can synthesize them later.
+	 */
+	for (i = 0, p = buf;
+	     i < size;
+	     i += req->digest_size, p += req->digest_size)
+		if (memcmp(p, req->zero_digest, req->digest_size))
+			break;
+	if (i == size)
+		return 0;
+
+	/*
+	 * Don't store trailing zeroes.  Store at least one byte so that the
+	 * block cannot be mistaken for an elided one.
+	 */
+	p = buf + size - 1;
 	while (p >= (const char *)buf && *p == 0)
 		p--;
-	args.valuelen = p - (const char *)buf + 1;
+	args.valuelen = max(1, p - (const char *)buf + 1);
 
 	xfs_verity_merkle_key_to_disk(&name, pos);
 	return xfs_attr_set(&args);