On Sat, Aug 24, 2024 at 10:31:16AM +0200, Christoph Hellwig wrote:
> On Fri, Aug 23, 2024 at 04:08:03PM +0530, Anuj Gupta wrote:
> > Copy back the bounce buffer to user-space in entirety when the parent
> > bio completes.
>
> This looks odd to me. The usual way to handle iterating the entire
> submitter controlled data is to just iterate over the bvec array, as
> done by bio_for_each_segment_all/bio_for_each_bvec_all for the bio
> data. I think you want to do the same here, probably with a
> similar bip_for_each_bvec_all or similar helper. That way you don't
> need to stash away the iter. Currently we have the field for that,
> but I really want to split up struct bio_integrity_payload into
> what is actually needed for the payload and stuff only needed for
> the block layer autogenerated PI (bip_bio/bio_iter/bip_work).
I can add it [*], to iterate over the entire bvec array. But the original
bio_iter still needs to be stored during submission, to calculate the
number of bytes in the original integrity/metadata iter (as it could have
gotten split, and I don't have original integrity iter stored anywhere).
Do you have a different opinion?
[*]
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index ff7de4fe74c4..f1690c644e70 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -125,11 +125,23 @@ static void bio_integrity_uncopy_user(struct bio_integrity_payload *bip)
struct bio_vec *copy = &bip->bip_vec[1];
size_t bytes = bio_iter_integrity_bytes(bi, bip->bio_iter);
struct iov_iter iter;
- int ret;
+ struct bio_vec *bvec;
+ struct bvec_iter_all iter_all;
iov_iter_bvec(&iter, ITER_DEST, copy, nr_vecs, bytes);
- ret = copy_to_iter(bvec_virt(bip->bip_vec), bytes, &iter);
- WARN_ON_ONCE(ret != bytes);
+ bip_for_each_segment_all(bvec, bip, iter_all) {
+ ssize_t ret;
+
+ ret = copy_page_to_iter(bvec->bv_page,
+ bvec->bv_offset,
+ bvec->bv_len,
+ &iter);
+
+ if (!iov_iter_count(&iter))
+ break;
+
+ WARN_ON_ONCE(ret < bvec->bv_len);
+ }
bio_integrity_unpin_bvec(copy, nr_vecs, true);
}
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 22ff2ae16444..3132ef6f27e0 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -46,6 +46,19 @@ struct uio_meta {
struct iov_iter iter;
};
+static inline bool bip_next_segment(const struct bio_integrity_payload *bip,
+ struct bvec_iter_all *iter)
+{
+ if (iter->idx >= bip->bip_vcnt)
+ return false;
+
+ bvec_advance(&bip->bip_vec[iter->idx], iter);
+ return true;
+}
+
+#define bip_for_each_segment_all(bvl, bip, iter) \
+ for (bvl = bvec_init_iter_all(&iter); bip_next_segment((bip), &iter); )
+
#define BIP_CLONE_FLAGS (BIP_MAPPED_INTEGRITY | BIP_CTRL_NOCHECK | \
BIP_DISK_NOCHECK | BIP_IP_CHECKSUM | \
BIP_CHECK_GUARD | BIP_CHECK_REFTAG | \
--
2.25.1
