Re: [PATCH 17/22] block: implement bio helper to add iter bvec pages to bio

[Christoph Hellwig <hch@xxxxxx>]

I don't think we need the loop for the ITER_BVEC case as bio_Add_page
will only fail when the bio is full.  Something like this would simplify
the implementation:

diff --git a/block/bio.c b/block/bio.c
index cc1ddf173aaf..658dcfc8d216 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -831,18 +831,18 @@ EXPORT_SYMBOL(bio_add_page);
 static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
 {
 	const struct bio_vec *bv = iter->bvec;
-	unsigned int len;
-	size_t size;
-
-	len = min_t(size_t, bv->bv_len, iter->count);
-	size = bio_add_page(bio, bv->bv_page, len,
-				bv->bv_offset + iter->iov_offset);
-	if (size == len) {
-		iov_iter_advance(iter, size);
-		return 0;
-	}
+	unsigned int len = min_t(size_t, bv->bv_len, iter->count);
+	unsigned int offset = bv->bv_offset + iter->iov_offset;
 
-	return -EINVAL;
+	/*
+	 * If this is a BVEC iter, then the pages are kernel pages. Don't
+	 * release them on IO completion.
+	 */
+	bio_set_flag(bio, BIO_HOLD_PAGES);
+	if (bio_add_page(bio, bv->bv_page, len, offset) != len)
+		return -EINVAL;
+	iov_iter_advance(iter, len);
+	return 0;
 }
 
 #define PAGE_PTRS_PER_BVEC     (sizeof(struct bio_vec) / sizeof(struct page *))
@@ -913,24 +913,13 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
  */
 int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 {
-	const bool is_bvec = iov_iter_is_bvec(iter);
 	unsigned short orig_vcnt = bio->bi_vcnt;
 
-	/*
-	 * If this is a BVEC iter, then the pages are kernel pages. Don't
-	 * release them on IO completion.
-	 */
-	if (is_bvec)
-		bio_set_flag(bio, BIO_HOLD_PAGES);
+	if (iov_iter_is_bvec(iter))
+		return __bio_iov_bvec_add_pages(bio, iter);
 
 	do {
-		int ret;
-
-		if (is_bvec)
-			ret = __bio_iov_bvec_add_pages(bio, iter);
-		else
-			ret = __bio_iov_iter_get_pages(bio, iter);
-
+		int ret = __bio_iov_iter_get_pages(bio, iter);
 		if (unlikely(ret))
 			return bio->bi_vcnt > orig_vcnt ? 0 : ret;