Re: [PATCH 1/2 v3] block: change rq_integrity_vec to respect the iterator

[Christoph Hellwig <hch@lst.de>]

On Thu, May 23, 2024 at 06:54:47PM +0200, Mikulas Patocka wrote:
> 
> However, the function rq_integrity_vec has a bug - it returns the first
> vector of the bio's metadata and completely disregards the metadata
> iterator that was advanced when the bio was split. Thus, the second bio
> uses the same metadata as the first bio and this leads to metadata
> corruption.
> 
> This commit changes rq_integrity_vec, so that it calls mp_bvec_iter_bvec
> instead of returning the first vector. mp_bvec_iter_bvec reads the
> iterator and advances the vector by the iterator.

mp_bvec_iter_bvec does not advance the bvec_iter, it just uses the
iter to build a bvec for the current position in the iter.

Also please fix the commit log to not use more than 73 characters,
as-is it will be unreadable in git show output or email replies.

> -static inline struct bio_vec *rq_integrity_vec(struct request *rq)
> +static inline struct bio_vec rq_integrity_vec(struct request *rq)
>  {
>  	if (WARN_ON_ONCE(queue_max_integrity_segments(rq->q) > 1))
> +		return (struct bio_vec){ };
> +	return mp_bvec_iter_bvec(rq->bio->bi_integrity->bip_vec,
> +				 rq->bio->bi_integrity->bip_iter);

The queue_max_integrity_segments check can go away now.  Once you
use the bvec_iter the function works just fine for multiple
integrity segments and always returns the one at the current iter
position.   That should preferably also documented in a comment.

(I'm also pretty sure I've already written this in reply to Anuj's
version of the patch)