Re: [PATCHv5 6/8] blk-mq-dma: add support for mapping integrity metadata

[Christoph Hellwig <hch@lst.de>]

On Fri, Aug 08, 2025 at 08:58:24AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Provide integrity metadata helpers equivalent to the data payload
> helpers for iterating a request for dma setup.

Can you please also convert the SG mapping helpers to use the low-level
iterator first like I've done for the data path?  That ensures we have
less code to maintain, common behavior and also smaller kernel binaries.

> +static bool __blk_map_iter_next(struct blk_map_iter *iter)
> +{
> +	if (iter->iter.bi_size)
> +		return true;
> +	if (!iter->bio || !iter->bio->bi_next)
> +		return false;
> +
> +	iter->bio = iter->bio->bi_next;
> +#ifdef CONFIG_BLK_DEV_INTEGRITY
> +	if (iter->is_integrity) {
> +		iter->iter = iter->bio->bi_integrity->bip_iter;
> +		iter->bvec = iter->bio->bi_integrity->bip_vec;
> +		return true;
> +	}
> +#endif
> +	iter->iter = iter->bio->bi_iter;
> +	iter->bvec = iter->bio->bi_io_vec;
> +	return true;

I wonder if we should use the bio_integrity() that would introduce two
(probably optimized down to one by the compiler) extra branches for the 
integrity mapping that are easily predicted, but make the think look much
nicer as it would kill the ifdef and the ugly structure around it:

	if (iter->is_integrity) {
		iter->iter = bio_integrity(iter->bio)->bip_iter;
		iter->bvec = bio_integrity(iter->bio)->bip_vec;
	} else {
		iter->iter = iter->bio->bi_iter;
		iter->bvec = iter->bio->bi_io_vec;
	}

	return true;

> +#ifdef CONFIG_BLK_DEV_INTEGRITY
> +bool blk_rq_integrity_dma_map_iter_start(struct request *req,
> +		struct device *dma_dev,  struct dma_iova_state *state,
> +		struct blk_dma_iter *iter)

Can you please add a kerneldoc comment here, which could be easily
adapter from the data mapping path.

> +bool blk_rq_integrity_dma_map_iter_next(struct request *req,
> +               struct device *dma_dev, struct blk_dma_iter *iter)

Same here.