On 9/19/23 8:45 AM, Christian Brauner wrote: > On Tue, Sep 12, 2023 at 11:06:39AM -0600, Jens Axboe wrote: >> On 9/9/23 9:11 AM, Jens Axboe wrote: >>> Hi, >>> >>> This adds support for IORING_OP_WAITID, which is an async variant of >>> the waitid(2) syscall. Rather than have a parent need to block waiting >>> on a child task state change, it can now simply get an async notication >>> when the requested state change has occured. >>> >>> Patches 1..4 are purely prep patches, and should not have functional >>> changes. They split out parts of do_wait() into __do_wait(), so that >>> the prepare-to-wait and sleep parts are contained within do_wait(). >>> >>> Patch 5 adds io_uring support. >>> >>> I wrote a few basic tests for this, which can be found in the >>> 'waitid' branch of liburing: >>> >>> https://git.kernel.dk/cgit/liburing/log/?h=waitid >>> >>> Also spun a custom kernel for someone to test it, and no issues reported >>> so far. >> >> Forget to mention that I also ran all the ltp testcases for any wait* >> syscall test, and everything still passes just fine. > > I think the struct that this ends up exposing to io_uring is pretty ugly > and it would warrant a larger cleanup. I wouldn't be surprised if you > get some people complain about this. > > Other than that I don't have any complaints about the series. io_uring only really needs child_wait and wo_pid on the wait_opts side, for waitid_info it needs all of it. I'm assuming your worry is about the former rather than the latter. I think we could only make this smaller if we had a separate entry point for io_uring, which would then make the code reuse a lot smaller. Right now we just have __do_wait() abstracted out, and if we added a third struct that has child_wait/wo_pid and exposed just that, we could not share this infrastructure. So as far as I can tell, there's no way to make the sharing less than it is, at least not without adding cost of more code and less reuse. Shrug? -- Jens Axboe
