On 7/29/26 08:07, Christoph Hellwig wrote:
- bio->bi_io_vec = bio_src->bi_io_vec;
+
+ if (op_is_dmabuf(bio->bi_opf)) {
+ bio->bi_dmabuf_map = bio_src->bi_dmabuf_map;
+ } else {
+ bio->bi_io_vec = bio_src->bi_io_vec;
+ }
No need for the braces.
Got it,
But given that these are the fields why
even bother doing both sides and not rely on the union?
Compiles into the same, but comparing to a comment makes
the compiler to check it for us if anything happens.
Noticed static_assert() below, that works as well, though I
think the current version is nicer b/c it preserves assignment
types for the compiler. Strict aliasing is obviously not a
thing, so don't have a strong opinion.
+void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter)
+{
+ WARN_ON_ONCE(bio->bi_max_vecs);
+
+ bio->bi_dmabuf_map = iter->dmabuf_map;
+ bio->bi_vcnt = 0;
+ bio->bi_iter.bi_offset = iter->iov_offset;
+ bio->bi_iter.bi_size = iov_iter_count(iter);
+ bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;
This seems to be largely copied from bio_iov_bvec_set, but misses
the REQ_CLONE there that we should probably set as well to indicate
that the data descriptor is not owned (although not setting it is
not really a bug).
What about merging these two into a single and easier to use interface
like this:
No opinion on this, I can add it, probably as another prep patch.
diff --git a/block/bio.c b/block/bio.c
index cc2bb9183c1a..25393bdd119d 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -860,12 +860,9 @@ static int __bio_clone(struct bio *bio, struct bio
*bio_src, gfp_t gfp)
bio->bi_write_hint = bio_src->bi_write_hint;
bio->bi_write_stream = bio_src->bi_write_stream;
bio->bi_iter = bio_src->bi_iter;
-
- if (op_is_dmabuf(bio->bi_opf)) {
- bio->bi_dmabuf_map = bio_src->bi_dmabuf_map;
- } else {
- bio->bi_io_vec = bio_src->bi_io_vec;
- }
+ static_assert(offsetof(struct bio, bi_io_vec) ==
+ offsetof(struct bio, bi_dmabuf_map));
+ bio->bi_io_vec = bio_src->bi_io_vec;
if (bio->bi_bdev) {
if (bio->bi_bdev == bio_src->bi_bdev &&
@@ -1186,26 +1183,21 @@ void __bio_release_pages(struct bio *bio, bool
mark_dirty)
}
EXPORT_SYMBOL_GPL(__bio_release_pages);
-void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
+bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter)
{
WARN_ON_ONCE(bio->bi_max_vecs);
+ if (!iov_iter_is_bvec(iter) && !iov_iter_is_dmabuf_map(iter))
+ return false;
+
bio->bi_io_vec = (struct bio_vec *)iter->bvec;
bio->bi_iter.bi_idx = 0;
bio->bi_iter.bi_offset = iter->iov_offset;
bio->bi_iter.bi_size = iov_iter_count(iter);
bio_set_flag(bio, BIO_CLONED);
-}
-
-void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter)
-{
- WARN_ON_ONCE(bio->bi_max_vecs);
-
- bio->bi_dmabuf_map = iter->dmabuf_map;
- bio->bi_vcnt = 0;
- bio->bi_iter.bi_offset = iter->iov_offset;
- bio->bi_iter.bi_size = iov_iter_count(iter);
- bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;
+ if (iov_iter_is_dmabuf_map(iter))
+ bio->bi_opf |= REQ_NOMERGE | REQ_DMABUF;
+ return true;
}
/*
@@ -1265,13 +1257,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct
iov_iter *iter,
if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
return -EIO;
- if (iov_iter_is_bvec(iter)) {
- bio_iov_bvec_set(bio, iter);
- iov_iter_advance(iter, bio->bi_iter.bi_size);
- return 0;
- }
- if (iov_iter_is_dmabuf_map(iter)) {
- bio_dmabuf_map_set(bio, iter);
+ if (bio_iov_iter_set(bio, iter)) {
iov_iter_advance(iter, bio->bi_iter.bi_size);
return 0;
}
diff --git a/block/blk-map.c b/block/blk-map.c
index d1d6bbe0ecf1..34816e6e64de 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -473,7 +473,7 @@ static int blk_rq_map_user_bvec(struct request *rq, const
struct iov_iter *iter)
bio = blk_rq_map_bio_alloc(rq, 0, GFP_KERNEL);
if (!bio)
return -ENOMEM;
- bio_iov_bvec_set(bio, iter);
+ bio_iov_iter_set(bio, iter);
ret = blk_rq_append_bio(rq, bio);
if (ret)
diff --git a/block/fops.c b/block/fops.c
index d83cdbab65a6..97fb124a19b2 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -340,17 +340,12 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb
*iocb,
bio->bi_end_io = blkdev_bio_end_io_async;
bio->bi_ioprio = iocb->ki_ioprio;
- if (iov_iter_is_bvec(iter)) {
- /*
- * Users don't rely on the iterator being in any particular
- * state for async I/O returning -EIOCBQUEUED, hence we can
- * avoid expensive iov_iter_advance(). Bypass
- * bio_iov_iter_get_pages() and set the bvec directly.
- */
- bio_iov_bvec_set(bio, iter);
- } else if (iov_iter_is_dmabuf_map(iter)) {
- bio_dmabuf_map_set(bio, iter);
- } else {
+ /*
+ * Users don't rely on the iterator being in any particular state for
+ * async I/O returning -EIOCBQUEUED, hence we can avoid the expensive
+ * iov_iter_advance() if we could set the bvec/dmabuf directly.
+ */
+ if (!bio_iov_iter_set(bio, iter)) {
ret = blkdev_iov_iter_get_pages(bio, iter, bdev);
if (unlikely(ret))
goto out_bio_put;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index c053e9444514..1eff02a843ef 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -484,8 +484,7 @@ int bdev_rw_virt(struct block_device *bdev, sector_t
sector, void *data,
int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
unsigned len_align_mask);
-void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter);
-void bio_dmabuf_map_set(struct bio *bio, struct iov_iter *iter);
+bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter);
void __bio_release_pages(struct bio *bio, bool mark_dirty);
extern void bio_set_pages_dirty(struct bio *bio);
extern void bio_check_pages_dirty(struct bio *bio);
--
Pavel Begunkov
