On 10/25/21 08:33, Christoph Hellwig wrote:
On Sat, Oct 23, 2021 at 05:21:34PM +0100, Pavel Begunkov wrote:
--- a/block/fops.c
+++ b/block/fops.c
@@ -352,11 +352,21 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
bio->bi_end_io = blkdev_bio_end_io_async;
bio->bi_ioprio = iocb->ki_ioprio;
- ret = bio_iov_iter_get_pages(bio, iter);
- if (unlikely(ret)) {
- bio->bi_status = BLK_STS_IOERR;
- bio_endio(bio);
- return ret;
+ if (!iov_iter_is_bvec(iter)) {
+ ret = bio_iov_iter_get_pages(bio, iter);
+ if (unlikely(ret)) {
+ bio->bi_status = BLK_STS_IOERR;
+ bio_endio(bio);
+ return ret;
+ }
Nit: I generally find it much nicer to read if simple if statements
don't use pointless negations.
+ } else {
+ /*
+ * Users don't rely on the iterator being in any particular
+ * state for async I/O returning -EIOCBQUEUED, hence we can
+ * avoid expensive iov_iter_advance(). Bypass
+ * bio_iov_iter_get_pages() and set the bvec directly.
+ */
+ bio_iov_bvec_set(bio, iter);
So if this optimization is so useful, please also do it for
non-bvec iov_iters, which is what 99% of the applications actually
use.
It's an async path, so mainly io_uring or aio, I don't think there
is much profit in doing that for iov, especially behind iov -> bvec
translation with page referencing.
Could've been done nonetheless, but what I think about looks too
ugly because of the loop inside of bio_iov_iter_get_pages(). Don't
think the sketch below is viable, any better ideas?
ssize_t __bio_iov_iter_get_pages() {
...
/* not advancing */
return size;
}
do {
if (bio_op(bio) == REQ_OP_ZONE_APPEND)
ret = __bio_iov_append_get_pages(bio, iter);
else
ret = __bio_iov_iter_get_pages(bio, iter);
if (ret < 0)
break;
iov_iter_advance(ret);
} while (iov_iter_count(iter) && !bio_full(bio, 0));
and copy paste into fops.c
do {
if (bio_op(bio) == REQ_OP_ZONE_APPEND)
ret = __bio_iov_append_get_pages(bio, iter);
else
ret = __bio_iov_iter_get_pages(bio, iter);
if (ret < 0 || iov_iter_count(iter) == ret || bio_full(bio, 0))
break;
iov_iter_advance(ret);
} while (1);
--
Pavel Begunkov
