On Thu, Jun 18, 2026 at 12:26:27PM +0200, Christoph Hellwig wrote:
> On Wed, Jun 17, 2026 at 04:32:35PM -0700, Keith Busch wrote:
> > @@ -1251,6 +1251,11 @@ int bio_iov_iter_get_pages(struct bio *bio, struct
> > iov_iter *iter,
> >
> > if (iov_iter_is_bvec(iter)) {
> > bio_iov_bvec_set(bio, iter);
> > +
> > + if (mp_bvec_iter_offset(bio->bi_io_vec, bio->bi_iter) &
> > + vec_align_mask)
> > + return -EINVAL;
>
> Can you add a comment here? Especially as the bvec iter doesn't actually
> require all individual bvecs to be aligned and I'm not entirely sure this
> handles all case - writing down the rules might help a bit with that.
The rationale is that the only iter_bvec users come from io_uring
registered buffers, which are virtually contiguous. Subsequent IO
referencing it provides only an offset and a length, so the only
possible unlaignment could bne the first offset (we've already verified
the total length earlier). Every subsequent vector must be page aligned
at a minimum, which is the largest possible dma alignment the block
layer allows, so we don't need to check the rest.
> > ret = iov_iter_extract_bvecs(iter, bio->bi_io_vec,
> > BIO_MAX_SIZE - bio->bi_iter.bi_size,
> > - &bio->bi_vcnt, bio->bi_max_vecs, flags);
> > + &bio->bi_vcnt, bio->bi_max_vecs,
> > + vec_align_mask, flags);
> > if (ret <= 0) {
> > + if (ret == -EINVAL) {
> > + bio_release_pages(bio, false);
> > + bio_clear_flag(bio, BIO_PAGE_PINNED);
> > + bio->bi_iter.bi_size = 0;
> > + bio->bi_vcnt = 0;
> > + return ret;
> > + }
>
> Do we need all this cleanups beyoned the bio_release_pages()? Most
> callers just free the bio, so should not care about it, and the error
> handling in __blkdev_direct_IO that calls bio_endio looks buggy for
> other reasons..
Yeah, it's exactly for the __blkdev_direct_IO() error handling, though I
think clearing either the PINNED flag or bi_vcnt is sufficient after
bio_release_pages(). The rest is just resetting the bio to the initial
state since I didn't want to return both an error and something that
looks like a partially constructed bio, even if no one currently cares.
But since you mention it, __blkdev_direct_IO's handling does look wrong,
so maybe I can clean that up first.
