Re: [PATCH] block: support zone append bvecs

[Christoph Hellwig <hch@xxxxxx>]

On Wed, Mar 24, 2021 at 10:09:32AM +0000, Johannes Thumshirn wrote:
> Stupid question, but wouldn't it be sufficient if I did (this can still be
> simplified):

No, this loses data if the iter is bigger than what you truncate it to.
Just with your last patch you probably did not test with larger
enough iters to be beyond the zone append limit.

> diff --git a/block/bio.c b/block/bio.c
> index 26b7f721cda8..9c529b2db8fa 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -964,6 +964,16 @@ static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
>         return 0;
>  }
>  
> +static int bio_iov_append_bvec_set(struct bio *bio, struct iov_iter *iter)
> +{
> +       struct request_queue *q = bio->bi_bdev->bd_disk->queue;
> +       unsigned int max_append = queue_max_zone_append_sectors(q) << 9;
> +
> +       iov_iter_truncate(iter, max_append);
> +
> +       return bio_iov_bvec_set(bio, iter);

OTOH if you copy the iter by value to a local one first and then
make sure the original iter is advanced it should work.  We don't
really need the iter advance for the original one, though.  Something like:

diff --git a/block/bio.c b/block/bio.c
index a1c4d2900c7a83..7d9e01580f2ab1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -949,7 +949,7 @@ void bio_release_pages(struct bio *bio, bool mark_dirty)
 }
 EXPORT_SYMBOL_GPL(bio_release_pages);
 
-static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
+static void __bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
 {
 	WARN_ON_ONCE(bio->bi_max_vecs);
 
@@ -959,7 +959,11 @@ static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
 	bio->bi_iter.bi_size = iter->count;
 	bio_set_flag(bio, BIO_NO_PAGE_REF);
 	bio_set_flag(bio, BIO_CLONED);
+}
 
+static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
+{
+	__bio_iov_bvec_set(bio, iter);
 	iov_iter_advance(iter, iter->count);
 	return 0;
 }
@@ -1019,6 +1023,17 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 	return 0;
 }
 
+static int bio_iov_bvec_set_append(struct bio *bio, struct iov_iter *iter)
+{
+	struct request_queue *q = bio->bi_bdev->bd_disk->queue;
+	struct iov_iter i = *iter;
+
+	iov_iter_truncate(&i, queue_max_zone_append_sectors(q) << 9);
+	__bio_iov_bvec_set(bio, &i);
+	iov_iter_advance(iter, i.count);
+	return 0;
+}
+
 static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
 {
 	unsigned short nr_pages = bio->bi_max_vecs - bio->bi_vcnt;
@@ -1094,8 +1109,8 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 	int ret = 0;
 
 	if (iov_iter_is_bvec(iter)) {
-		if (WARN_ON_ONCE(bio_op(bio) == REQ_OP_ZONE_APPEND))
-			return -EINVAL;
+		if (bio_op(bio) == REQ_OP_ZONE_APPEND)
+			return bio_iov_bvec_set_append(bio, iter);
 		return bio_iov_bvec_set(bio, iter);
 	}