Re: [PATCH v6 02/22] fsverity: expose ensure_fsverity_info()

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Mar 31, 2026 at 11:28:03PM +0200, Andrey Albershteyn wrote:
> This function will be used by XFS's scrub to force fsverity activation,
> therefore, to read fsverity context.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> ---

Acked-by: Eric Biggers <ebiggers@kernel.org>

> +/**
> + * fsverity_ensure_verity_info() - create verity info if it's not in memory yet
> + * @inode: the inode for which verity info should be created
> + *
> + * Ensure this inode has verity info attached to it. Read fsverity descriptor
> + * and creates verity based on that. Inodes opened outside of
> + * file_operations->open will not have any verity info attached. This
> + * info is required for any fsverity related operations.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_ensure_verity_info(struct inode *inode);

As Christoph mentioned, fs/verity/ uses the convention of the kerneldoc
for functions being above the function definition.

I think the comment could also be clearer:

> create verity info if it's not in memory yet

Maybe "cache verity info if it's not already cached", to avoid potential
confusion with enabling fsverity on the file.

> Ensure this inode has verity info attached to it.

Maybe add: "It's assumed the inode already has fsverity enabled."

> Inodes opened outside of file_operations->open will not have any
> verity info attached. This info is required for any fsverity
> related operations.

The first sentence could be misinterpreted as saying that this function
won't do anything in that case.  The second sentence isn't clear what
counts as "any fsverity related operation".  Also "opened" doesn't seem
like the right word to use when talking about a filesystem-internal read
that occurs without a file descriptor having been opened.

Maybe replace with:

* This needs to be called at least once before any of the inode's data
* can be verified (and thus read at all) or the inode's fsverity digest
* retrieved.  fsverity_file_open() calls this already, which handles
* normal file accesses.  If a filesystem does any internal (i.e. not
* associated with a file descriptor) reads of the file's data or
* fsverity digest, it must call this explicitly before doing so.

By the way, should there be a patch that converts
ovl_ensure_verity_loaded() to use this?

- Eric