| Message ID | b3e19eb1-18c4-8599-b68d-bf28673237d1@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] iov_iter: import single segments iovecs as ITER_UBUF | expand |
On Sat, Jun 18, 2022 at 08:08:08AM -0600, Jens Axboe wrote: > Using an ITER_UBUF is more efficient than an ITER_IOV, and for the single > segment case, there's no reason to use an ITER_IOV when an ITER_UBUF will > do. Experimental data collected shows that ~2/3rds of iovec imports are > single segments, from applications using readv/writev or recvmsg/sendmsg > that are iovec based. > > Explicitly check for nr_segs == 1 and import those as ubuf rather than > iovec based iterators. Hadn't we'd been through that before? There is infinibarf code that assumes ITER_IOVEC for what its ->write_iter() gets (and yes, that's the one that has ->write() with different semantics). And I wouldn't bet a dime on all ->sendmsg() and ->recvmsg() being flavour-agnostic either...
On Sat, Jun 18, 2022 at 04:19:24PM +0100, Al Viro wrote: > On Sat, Jun 18, 2022 at 08:08:08AM -0600, Jens Axboe wrote: > > Using an ITER_UBUF is more efficient than an ITER_IOV, and for the single > > segment case, there's no reason to use an ITER_IOV when an ITER_UBUF will > > do. Experimental data collected shows that ~2/3rds of iovec imports are > > single segments, from applications using readv/writev or recvmsg/sendmsg > > that are iovec based. > > > > Explicitly check for nr_segs == 1 and import those as ubuf rather than > > iovec based iterators. > > Hadn't we'd been through that before? There is infinibarf code that > assumes ITER_IOVEC for what its ->write_iter() gets (and yes, that's > the one that has ->write() with different semantics). > > And I wouldn't bet a dime on all ->sendmsg() and ->recvmsg() being > flavour-agnostic either... Incidentally, what will your patch do to one-segment readv(2) from e.g. /proc/self/status? Or anything else that has no ->read_iter, for that matter...
On 6/18/22 9:19 AM, Al Viro wrote: > On Sat, Jun 18, 2022 at 08:08:08AM -0600, Jens Axboe wrote: >> Using an ITER_UBUF is more efficient than an ITER_IOV, and for the single >> segment case, there's no reason to use an ITER_IOV when an ITER_UBUF will >> do. Experimental data collected shows that ~2/3rds of iovec imports are >> single segments, from applications using readv/writev or recvmsg/sendmsg >> that are iovec based. >> >> Explicitly check for nr_segs == 1 and import those as ubuf rather than >> iovec based iterators. > > Hadn't we'd been through that before? There is infinibarf code that > assumes ITER_IOVEC for what its ->write_iter() gets (and yes, that's > the one that has ->write() with different semantics). This is why it's an RFC... Would like to do something about that! > And I wouldn't bet a dime on all ->sendmsg() and ->recvmsg() being > flavour-agnostic either... FWIW, the liburing regressions tests do have sendmsg/recvmsg and at least basic stuff works. Haven't looked at all deeper on whether this is always the case.
On 6/18/22 9:32 AM, Al Viro wrote: > On Sat, Jun 18, 2022 at 04:19:24PM +0100, Al Viro wrote: >> On Sat, Jun 18, 2022 at 08:08:08AM -0600, Jens Axboe wrote: >>> Using an ITER_UBUF is more efficient than an ITER_IOV, and for the single >>> segment case, there's no reason to use an ITER_IOV when an ITER_UBUF will >>> do. Experimental data collected shows that ~2/3rds of iovec imports are >>> single segments, from applications using readv/writev or recvmsg/sendmsg >>> that are iovec based. >>> >>> Explicitly check for nr_segs == 1 and import those as ubuf rather than >>> iovec based iterators. >> >> Hadn't we'd been through that before? There is infinibarf code that >> assumes ITER_IOVEC for what its ->write_iter() gets (and yes, that's >> the one that has ->write() with different semantics). >> >> And I wouldn't bet a dime on all ->sendmsg() and ->recvmsg() being >> flavour-agnostic either... > > Incidentally, what will your patch do to one-segment readv(2) from > e.g. /proc/self/status? Or anything else that has no ->read_iter, for > that matter... Yes indeed, that won't work at all... I guess this has to be up to the caller, we can't stuff it this far down.
diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 6b2bf6f6f374..0973c622d3c0 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -1813,6 +1813,21 @@ ssize_t __import_iovec(int type, const struct iovec __user *uvec, return PTR_ERR(iov); } + /* + * Fast path - single segment import. Use UBUF for these, rather + * than setup an ITER_IOV. + */ + if (nr_segs == 1) { + ssize_t ret; + + total_len = iovp[0]->iov_len; + ret = import_ubuf(type, iovp[0]->iov_base, total_len, i); + *iovp = NULL; + if (unlikely(ret < 0)) + return ret; + return total_len; + } + /* * According to the Single Unix Specification we should return EINVAL if * an element length is < 0 when cast to ssize_t or if the total length
Using an ITER_UBUF is more efficient than an ITER_IOV, and for the single segment case, there's no reason to use an ITER_IOV when an ITER_UBUF will do. Experimental data collected shows that ~2/3rds of iovec imports are single segments, from applications using readv/writev or recvmsg/sendmsg that are iovec based. Explicitly check for nr_segs == 1 and import those as ubuf rather than iovec based iterators. Signed-off-by: Jens Axboe <axboe@kernel.dk> --- Lightly tested - boots and works just fine, and I ran this through the liburing test suite which does plenty of single segment readv/writev as well.