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