Re: [PATCH v15 17/25] xfs: use read ioend for fsverity data verification

[Christoph Hellwig <hch@lst.de>]

On Fri, Aug 14, 2026 at 11:24:34AM +0200, Andrey Albershteyn wrote:
> Use read ioends for fsverity verification. Do not issue fsverity
> metadata I/O through the same workqueue due to risk of a deadlock by a
> filled workqueue.
> 
> Pass fsverity_info from iomap context down to the ioend as hashtable
> lookups are expensive.
> 
> Add a simple helper to check that this is not fsverity metadata but file
> data that needs verification.

> -	const struct address_space	*mapping)
> +	const struct address_space	*mapping,
> +	loff_t				position)
>  {

Hmm, "position" is new for these kinds of arguments.  We tend to call
them "pos", "off", or "offset", but I guess this completes the matrix :)

But maybe stick to pos to match the naming of the helpers used by the
callers.


>  	struct xfs_inode		*ip = XFS_I(mapping->host);
>  
> -	if (bdev_has_integrity_csum(xfs_inode_buftarg(ip)->bt_bdev))
> +	if (bdev_has_integrity_csum(xfs_inode_buftarg(ip)->bt_bdev) ||
> +			xfs_fsverity_is_file_data(ip, position))
>  		return &xfs_iomap_read_ops;
>  	return &iomap_bio_read_ops;

Nit: While this is one of the standard Linux indent styles for long
ifs, the other one would seem more readable here:

	if (bdev_has_integrity_csum(xfs_inode_buftarg(ip)->bt_bdev) ||
	    xfs_fsverity_is_file_data(ip, position))

> +#include "xfs_errortag.h"
> +#include "xfs_fsverity.h"
>  #include <linux/bio-integrity.h>
> +#include <linux/fsverity.h>
> +
> +static void
> +xfs_end_fsverity_io_read(
> +	struct work_struct	*work)
> +{
> +	struct iomap_ioend	*ioend =
> +		container_of(work, struct iomap_ioend, io_work);
> +
> +	if (!ioend->io_bio.bi_status)
> +		fsverity_verify_bio(ioend->io_vi, &ioend->io_bio);
> +
> +	iomap_finish_ioends(
> +		ioend, blk_status_to_errno(ioend->io_bio.bi_status));

Indentation looks odd here, this should be:

	iomap_finish_ioends(ioend,
		blk_status_to_errno(ioend->io_bio.bi_status));

or maybe add a local bio variable given that you use ioend->io_bio
three times, and this would fit onto a single line.

> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 0959b97e641b..f329a57d6ee9 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -454,6 +454,7 @@ struct iomap_ioend {
>  	sector_t		io_sector;	/* start sector of ioend */
>  	void			*io_private;	/* file system private data */
>  	struct fsverity_info	*io_vi;		/* fsverity info */
> +	struct work_struct	io_work;	/* fsverity blocking I/O */
>  	struct bio		io_bio;		/* MUST BE LAST! */
>  };

Please don't add new fields to iomap structures in xfs patches.

And I really don't like adding it here given that struct work_struct is
rather big and not useful in other ways here.  So maybe just do the
alloc a struct for the workqueue and queue it up using
fsverity_enqueue_verify_work approach the other file systems do.
Or add something like the block complete in task thing to fsverity
and simplify all these so that they only need a list entry
(which we already have in the ioend).