On Thu, Mar 03, 2022 at 01:41:50PM -0700, Jens Axboe wrote: > On 3/3/22 10:18 AM, Jens Axboe wrote: > > On 3/3/22 9:31 AM, Jens Axboe wrote: > >> On 3/3/22 7:40 AM, Jens Axboe wrote: > >>> On 3/3/22 7:36 AM, Jens Axboe wrote: > >>>> The only potential oddity here is that the fd passed back is not a > >>>> legitimate fd. io_uring does support poll(2) on its file descriptor, so > >>>> that could cause some confusion even if I don't think anyone actually > >>>> does poll(2) on io_uring. > >>> > >>> Side note - the only implication here is that we then likely can't make > >>> the optimized behavior the default, it has to be an IORING_SETUP_REG > >>> flag which tells us that the application is aware of this limitation. > >>> Though I guess close(2) might mess with that too... Hmm. > >> > >> Not sure I can find a good approach for that. Tried out your patch and > >> made some fixes: > >> > >> - Missing free on final tctx free > >> - Rename registered_files to registered_rings > >> - Fix off-by-ones in checking max registration count > >> - Use kcalloc > >> - Rename ENTER_FIXED_FILE -> ENTER_REGISTERED_RING > >> - Don't pass in tctx to io_uring_unreg_ringfd() > >> - Get rid of forward declaration for adding tctx node > >> - Get rid of extra file pointer in io_uring_enter() > >> - Fix deadlock in io_ringfd_register() > >> - Use io_uring_rsrc_update rather than add a new struct type > > > > - Allow multiple register/unregister instead of enforcing just 1 at the > > time > > - Check for it really being a ring fd when registering > > > > For different batch counts, nice improvements are seen. Roughly: > > > > Batch==1 15% faster > > Batch==2 13% faster > > Batch==4 11% faster > > > > This is just in microbenchmarks where the fdget/fdput play a bigger > > factor, but it will certainly help with real world situations where > > batching is more limited than in benchmarks. > Certainly seems worthwhile. > In trying this out in applications, I think the better registration API > is to allow io_uring to pick the offset. The application doesn't care, > it's just a magic integer there. And if we allow io_uring to pick it, > then that makes things a lot easier to deal with. > > For registration, pass in an array of io_uring_rsrc_update structs, just > filling in the ring_fd in the data field. Return value is number of ring > fds registered, and up->offset now contains the chosen offset for each > of them. > > Unregister is the same struct, but just with offset filled in. > > For applications using io_uring, which is all of them as far as I'm > aware, we can also easily hide this. This means we can get the optimized > behavior by default. > Did you mean s/using io_uring/using liburing/ here? Regards, Vito Caputo
