On 5/23/22 8:47 AM, Al Viro wrote: > On Mon, May 23, 2022 at 08:34:44AM -0600, Jens Axboe wrote: >> On 5/23/22 08:22, Al Viro wrote: >>> On Sun, May 22, 2022 at 08:43:26PM -0600, Jens Axboe wrote: >>> >>>> Branch here: >>>> >>>> https://git.kernel.dk/cgit/linux-block/log/?h=iov-iter >>>> >>>> First 5 are generic ones, and some of them should just be folded with >>>> your changes. >>>> >>>> Last 2 are just converting io_uring to use it where appropriate. >>>> >>>> We can also use it for vectored readv/writev and recvmsg/sendmsg with >>>> one segment. The latter is mostly single segment in the real world >>>> anyway, former probably too. Though not sure it's worth it when we're >>>> copying a single iovec first anyway? Something to test... >>> >>> Not a good idea. Don't assume that all users of iov_iter are well-behaving; >>> not everything is flavour-agnostic. If nothing else, you'll break the hell >>> out of infinibarf - both qib and hfi check that ->write_iter() gets >>> IOV_ITER target and fail otherwise. >> >> OK, I'll check up on that. >> >>> BTW, #work.iov_iter is going to be rebased and reordered; if nothing else, >>> a bunch of places like >>> dio->should_dirty = iter_is_iovec(iter) && iov_iter_rw(iter) == READ; >>> need to be dealt with before we switch new_sync_read() and new_sync_write() >>> to ITER_UBUF. >> >> I already made an attempt at that, see the git branch I sent in the last email. > > There's several more, AFAICS (cifs, ceph, fuse, gfs2)... The check in > /dev/fuse turned out to be fine - it's only using primitives, so we > can pass ITER_UBUF ones there. mm/shmem.c check... similar, but I > really wonder if x86 clear_user() really sucks worse than > copy_to_user() from zero page... Yep, not surprised if it isn't complete, I just tackled the ones I found. I do like the idea of having a generic check for that rather than implicit knowledge about which iter types may contain user memory. I haven't looked at clear_user() vs copy_to_user() from the zero page. But should be trivial to benchmark and profile. I'll try and do that when I find some time. > FWIW, I'd added bool ->user_backed next to ->data_source, with > user_backed_iter() as an inline predicate checking it. Seems to get > slightly better iov_iter.c code generation that way... OK that sounds fine too, I pondered doing something similar rather than have the helper since there is an existing hole in there anyway. > Current branch pushed to #new.iov_iter (at the moment; will rename > back to work.iov_iter once it gets more or less stable). Sounds good, I'll see what I need to rebase. -- Jens Axboe
