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

Andrey Albershteyn
2026-04-02T14:02:55Z

On 2026-04-01 15:02:41, Eric Biggers wrote:
> 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?

sure, I will add a patch on top replacing ovl's function

--
- Andrey