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

[Christoph Hellwig <hch@lst.de>]

On Tue, Aug 12, 2025 at 06:52:08AM -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.

This actually does two things:

  1) convert the existing SG helpers
  2) add the new helpers

Which probably should be split into a separate patches, and all of them
could use a more detailed commit log.

> +	struct blk_map_iter iter;
> +	struct phys_vec vec;
> +
> +	iter = (struct blk_map_iter) {
> +		.bio = bio,
> +		.iter = bio_integrity(bio)->bip_iter,
> +		.bvecs = bio_integrity(bio)->bip_vec,
> +		.is_integrity = true,
> +	};

Just initialize iter at declaration time:

> +	struct blk_map_iter iter = {
> +		.bio = bio,
> +		.iter = bio_integrity(bio)->bip_iter,
> +		.bvecs = bio_integrity(bio)->bip_vec,
> +		.is_integrity = true,
> +	};

> +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;
> +	if (iter->is_integrity) {
> +		iter->iter = bio_integrity(iter->bio)->bip_iter;
> +		iter->bvecs = bio_integrity(iter->bio)->bip_vec;
> +	} else {
> +		iter->iter = iter->bio->bi_iter;
> +		iter->bvecs = iter->bio->bi_io_vec;
> +	}
> +	return true;
> +}

This seems unused, even with all patches applied.

> -static inline struct scatterlist *
> -blk_next_sg(struct scatterlist **sg, struct scatterlist *sglist)
> -{
> -	if (!*sg)
> -		return sglist;
> -
> -	/*
> -	 * If the driver previously mapped a shorter list, we could see a
> -	 * termination bit prematurely unless it fully inits the sg table
> -	 * on each mapping. We KNOW that there must be more entries here
> -	 * or the driver would be buggy, so force clear the termination bit
> -	 * to avoid doing a full sg_init_table() in drivers for each command.
> -	 */
> -	sg_unmark_end(*sg);
> -	return sg_next(*sg);
> -}

Maybe just add the metadata mapping helpers into this file instead of
moving stuff around?

> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -449,4 +449,30 @@ static inline bool blk_mq_can_poll(struct request_queue *q)
>  		q->tag_set->map[HCTX_TYPE_POLL].nr_queues;
>  }
>  
> +struct phys_vec {
> +	phys_addr_t	paddr;
> +	u32		len;
> +};

Leon has a patch to move this to linux/types.h, which if we have to
move it is probably a better place.

Otherwise this looks good.