On Thu, 23 May 2024, Christoph Hellwig wrote:
> 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.
OK. I thought that the limit was 74.
> > -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.
Yes, I dropped this and I am resending a new version.
> (I'm also pretty sure I've already written this in reply to Anuj's
> version of the patch)
Mikulas
