Conversation

Notices

1. Embed this notice
   Jens Axboe (axboe@fosstodon.org)'s status on Saturday, 25-Mar-2023 01:26:32 JST Jens Axboe

   One of my pet peeves - people using vectored IO operations with single segments. At least now we'll make it a bit better, though still wasting time on copying in a single iovec.

   https://lore.kernel.org/io-uring/43cb1fb7-b30b-8df1-bba6-e50797d680c6@kernel.dk/

   • Embed this notice
     Linus Torvalds (torvalds@social.kernel.org)'s status on Saturday, 25-Mar-2023 01:26:30 JST Linus Torvalds

     in reply to

     @axboe shouldn’t we do that for import_single_range() and for import_iovec() too, and turn those into ITER_UBUF rather than ITER_IOVEC?

     Maybe I’m missing something.

   • Embed this notice
     Jens Axboe (axboe@fosstodon.org)'s status on Saturday, 25-Mar-2023 01:46:05 JST Jens Axboe

     in reply to

     • Linus Torvalds

     @torvalds My kvm boots fine with:

     diff --git a/lib/iov_iter.c b/lib/iov_iter.c
     index 274014e4eafe..06e46530cb3d 100644
     --- a/lib/iov_iter.c
     +++ b/lib/iov_iter.c
     @@ -1868,7 +1868,7 @@ int import_single_range(int rw, void __user *buf, size_t len,
     
     iov->iov_base = buf;
     iov->iov_len = len;
     - iov_iter_init(i, rw, iov, 1, len);
     + iov_iter_ubuf(i, rw, buf, len);
     return 0;
     }
     EXPORT_SYMBOL(import_single_range);

     Let me try the __import_iovec() as well...

   • Embed this notice
     Linus Torvalds (torvalds@social.kernel.org)'s status on Saturday, 25-Mar-2023 01:46:05 JST Linus Torvalds

     in reply to

     @axboe you should get rid of the two preceding lines as welll (the iov_base/iov_len initialization). They are now wrong, and the iov_iter_ubuf() will overwrite them anyway. The compiler will presumably not generate code for them as it notices, but still..

   • Embed this notice
     Jens Axboe (axboe@fosstodon.org)'s status on Saturday, 25-Mar-2023 01:46:06 JST Jens Axboe

     in reply to

     • Linus Torvalds

     @torvalds Yep, we certainly could do that. I did that in io_uring, I'd be weary of doing it generically without auditing all the callers. Some callers assume you'll get an ITER_IOVEC out of those helpers, would make me nervous to do it wholesale inside the functions.

     For liburing specifically, I'd want to add a patch that just turns io_uring_prep_{readv,writev} into read/write instead. Because the iovec copy overhead is noticeable too, and the ITER_UBUF conversion only saves iterator overhead.