On 24/03/2021 08:13, Johannes Thumshirn wrote:
> On 24/03/2021 08:11, Christoph Hellwig wrote:
>> On Wed, Mar 24, 2021 at 07:07:27AM +0000, Johannes Thumshirn wrote:
>>>>> 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) {
>>>>> + struct request_queue *q = bio->bi_bdev->bd_disk->queue;
>>>>> + unsigned int max_append =
>>>>> + queue_max_zone_append_sectors(q) << 9;
>>>>> +
>>>>> + if (WARN_ON_ONCE(iter->count > max_append))
>>>>> + return -EINVAL;
>>>>> + }
>>>>
>>>> That is not correct. bio_iov_iter_get_pages just fills the bio as far
>>>> as we can, and then returns 0 for the next call to continue. Basically
>>>> what you want here is a partial version of bio_iov_bvec_set.
>>>>
>>>
>>> Isn't that what I did? The above is checking if we have REQ_OP_ZONE_APPEND and
>>> then returns EINVAL if iter->count is bigger than queue_max_zone_append_sectors().
>>> If the check doesn't fail, its going to call bio_iov_bvec_set().
>>
>> And that is the problem. It should not fail, the payload is decoupled
>> from the max_append size.
>>
>> Doing the proper thing is not too hard as described above - make sure
>> the bi_iter points to only the chunk of iter passed in that fits, and
>> only advance the passed in iter by that amount.
>>
>
> Ah got it now,
> thanks
>
Stupid question, but wouldn't it be sufficient if I did (this can still be
simplified):
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);
+}
+
#define PAGE_PTRS_PER_BVEC (sizeof(struct bio_vec) / sizeof(struct page *))
/**
@@ -1094,8 +1104,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_append_bvec_set(bio, iter);
return bio_iov_bvec_set(bio, iter);
}
The above had a successful xfstests run and several fio --ioengine=splice runs.
