Re: [PATCH 2/2] fsverity: add tracepoints

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Jan 12, 2026 at 01:15:02PM +0100, Andrey Albershteyn wrote:
> fs-verity previously had debug printk but it was removed. This patch
> adds trace points to the same places where printk were used (with a
> few additional ones).

They're not the same places.  Which is the right choice: the locations
of the old debug messages aren't necessarily the places the tracepoints
should be.  But the commit message shouldn't claim otherwise.

> diff --git a/fs/verity/verify.c b/fs/verity/verify.c
> index 86067c8b40..47a66f088f 100644
> --- a/fs/verity/verify.c
> +++ b/fs/verity/verify.c
> @@ -135,6 +135,9 @@
>  		/* Byte offset of the wanted hash relative to @addr */
>  		unsigned int hoffset;
>  	} hblocks[FS_VERITY_MAX_LEVELS];
> +
> +	trace_fsverity_verify_data_block(inode, params, data_pos);
> +
>  	/*
>  	 * The index of the previous level's block within that level; also the
>  	 * index of that block's hash within the current level.
> @@ -214,6 +217,9 @@
>  			want_hash = _want_hash;
>  			kunmap_local(haddr);
>  			put_page(hpage);
> +			trace_fsverity_merkle_hit(inode, data_pos, hblock_idx,
> +					level,
> +					hoffset >> params->log_digestsize);
>  			goto descend;
>  		}
>  		hblocks[level].page = hpage;
> @@ -248,6 +254,9 @@
>  		want_hash = _want_hash;
>  		kunmap_local(haddr);
>  		put_page(hpage);
> +		trace_fsverity_verify_merkle_block(inode,
> +				hblock_idx << params->log_blocksize,
> +				level, hoffset >> params->log_digestsize);
>  	}

The second argument to trace_fsverity_verify_merkle_block() is wrong.
It's "unsigned long index", but the caller passes a byte position.  Note
that byte positions should be u64.  Perhaps you intended for it to be
"unsigned long hblock_idx", like trace_fsverity_merkle_hit(), and for
the caller to pass hblock_idx?

Also note that if the hash doesn't match, then
trace_fsverity_verify_merkle_block() isn't called.  Perhaps it should be
called before the hash check is done?  That's the case for
trace_fsverity_verify_data_block().

- Eric