| Message ID | cover.1723039801.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | clodkid and abs mode CQ wait timeouts | expand |
On 8/7/24 8:18 AM, Pavel Begunkov wrote: > Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting > for completions, which makes the kernel to interpret the passed timespec > not as a relative time to wait but rather an absolute timeout. > > Patch 4 adds a way to set a clock id to use for CQ waiting. > > Tests: https://github.com/isilence/liburing.git abs-timeout Looks good to me - was going to ask about tests, but I see you have those already! Thanks.
On 8/12/24 12:13 PM, Jens Axboe wrote: > On 8/7/24 8:18 AM, Pavel Begunkov wrote: >> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >> for completions, which makes the kernel to interpret the passed timespec >> not as a relative time to wait but rather an absolute timeout. >> >> Patch 4 adds a way to set a clock id to use for CQ waiting. >> >> Tests: https://github.com/isilence/liburing.git abs-timeout > > Looks good to me - was going to ask about tests, but I see you have those > already! Thanks. Took a look at the test, also looks good to me. But we need the man pages updated, or nobody will ever know this thing exists.
On 8/12/24 19:30, Jens Axboe wrote: > On 8/12/24 12:13 PM, Jens Axboe wrote: >> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>> for completions, which makes the kernel to interpret the passed timespec >>> not as a relative time to wait but rather an absolute timeout. >>> >>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>> >>> Tests: https://github.com/isilence/liburing.git abs-timeout >> >> Looks good to me - was going to ask about tests, but I see you have those >> already! Thanks. > > Took a look at the test, also looks good to me. But we need the man > pages updated, or nobody will ever know this thing exists. If we go into that topic, people not so often read manuals to learn new features, a semi formal tutorial would be much more useful, I believe. Regardless, I can update mans before sending the tests, I was waiting if anyone have feedback / opinions on the api.
On 8/12/24 6:50 PM, Pavel Begunkov wrote: > On 8/12/24 19:30, Jens Axboe wrote: >> On 8/12/24 12:13 PM, Jens Axboe wrote: >>> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>>> for completions, which makes the kernel to interpret the passed timespec >>>> not as a relative time to wait but rather an absolute timeout. >>>> >>>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>>> >>>> Tests: https://github.com/isilence/liburing.git abs-timeout >>> >>> Looks good to me - was going to ask about tests, but I see you have those >>> already! Thanks. >> >> Took a look at the test, also looks good to me. But we need the man >> pages updated, or nobody will ever know this thing exists. > > If we go into that topic, people not so often read manuals > to learn new features, a semi formal tutorial would be much > more useful, I believe. > > Regardless, I can update mans before sending the tests, I was > waiting if anyone have feedback / opinions on the api. I regularly get people sending corrections or questions after having read man pages, so I'd have to disagree. In any case, if there's one spot that SHOULD have the documentation, it's the man pages. Definitely any addition should be added there too. I'd love for the man pages to have more section 7 additions, like one on fixed buffers and things like that, so that it would be THE spot to get to know about these features. Tutorials always useful (even if they tend often age poorly), but that should be an addition to the man pages, not instead of. On the GH wiki is where they can go, and I believe you have write access there too :-)
On 8/13/24 01:59, Jens Axboe wrote: > On 8/12/24 6:50 PM, Pavel Begunkov wrote: >> On 8/12/24 19:30, Jens Axboe wrote: >>> On 8/12/24 12:13 PM, Jens Axboe wrote: >>>> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>>>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>>>> for completions, which makes the kernel to interpret the passed timespec >>>>> not as a relative time to wait but rather an absolute timeout. >>>>> >>>>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>>>> >>>>> Tests: https://github.com/isilence/liburing.git abs-timeout >>>> >>>> Looks good to me - was going to ask about tests, but I see you have those >>>> already! Thanks. >>> >>> Took a look at the test, also looks good to me. But we need the man >>> pages updated, or nobody will ever know this thing exists. >> >> If we go into that topic, people not so often read manuals >> to learn new features, a semi formal tutorial would be much >> more useful, I believe. >> >> Regardless, I can update mans before sending the tests, I was >> waiting if anyone have feedback / opinions on the api. > > I regularly get people sending corrections or questions after having > read man pages, so I'd have to disagree. In any case, if there's one That doesn't necessarily mean they've learned about the feature from the man page. In my experience, people google a problem, find some clue like a name of the feature they need and then go to a manual (or other source) to learn more. Which is why I'm not saying that man pages don't have a purpose, on the contrary, but there are often more convenient ways of discovering in the long run. > spot that SHOULD have the documentation, it's the man pages. Definitely > any addition should be added there too. > > I'd love for the man pages to have more section 7 additions, like one on > fixed buffers and things like that, so that it would be THE spot to get > to know about these features. Tutorials always useful (even if they tend > often age poorly), but that should be an addition to the man pages, not "tutorials" could be different, they can be kept in the repo and up to date. bpftrace manual IMHO is a good example, it's more 'leisurely' readable, whereas manual pages need to be descriptive and hence verbose to cover all edge cases and conditions. https://github.com/bpftrace/bpftrace/blob/master/man/adoc/bpftrace.adoc > instead of. On the GH wiki is where they can go, and I believe you have > write access there too :-)
On 8/12/24 7:32 PM, Pavel Begunkov wrote: > On 8/13/24 01:59, Jens Axboe wrote: >> On 8/12/24 6:50 PM, Pavel Begunkov wrote: >>> On 8/12/24 19:30, Jens Axboe wrote: >>>> On 8/12/24 12:13 PM, Jens Axboe wrote: >>>>> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>>>>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>>>>> for completions, which makes the kernel to interpret the passed timespec >>>>>> not as a relative time to wait but rather an absolute timeout. >>>>>> >>>>>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>>>>> >>>>>> Tests: https://github.com/isilence/liburing.git abs-timeout >>>>> >>>>> Looks good to me - was going to ask about tests, but I see you have those >>>>> already! Thanks. >>>> >>>> Took a look at the test, also looks good to me. But we need the man >>>> pages updated, or nobody will ever know this thing exists. >>> >>> If we go into that topic, people not so often read manuals >>> to learn new features, a semi formal tutorial would be much >>> more useful, I believe. >>> >>> Regardless, I can update mans before sending the tests, I was >>> waiting if anyone have feedback / opinions on the api. >> >> I regularly get people sending corrections or questions after having >> read man pages, so I'd have to disagree. In any case, if there's one > > That doesn't necessarily mean they've learned about the feature from > the man page. In my experience, people google a problem, find some > clue like a name of the feature they need and then go to a manual > (or other source) to learn more. > > Which is why I'm not saying that man pages don't have a purpose, on > the contrary, but there are often more convenient ways of discovering > in the long run. In my experience, you google if you have very little clue what you're doing, to hopefully learn. And you use a man page, if whatever API you're using has good man pages, if you're just curius about a specific function. There's definitely a place for both. None of that changes the fact that the liburing man pages should _always_ document all of the API. >> spot that SHOULD have the documentation, it's the man pages. Definitely >> any addition should be added there too. >> >> I'd love for the man pages to have more section 7 additions, like one on >> fixed buffers and things like that, so that it would be THE spot to get >> to know about these features. Tutorials always useful (even if they tend >> often age poorly), but that should be an addition to the man pages, not > > "tutorials" could be different, they can be kept in the repo and > up to date. bpftrace manual IMHO is a good example, it's more > 'leisurely' readable, whereas manual pages need to be descriptive > and hence verbose to cover all edge cases and conditions. > > https://github.com/bpftrace/bpftrace/blob/master/man/adoc/bpftrace.adoc Yeah that'd be fine too, the closer to the repo, the better. And maybe that can replace section 7 man pages, we are sorely missing some descriptions of idiomatic usage of various features. That kind of thing is hard to learn in a man page.
On 8/13/24 03:09, Jens Axboe wrote: > On 8/12/24 7:32 PM, Pavel Begunkov wrote: >> On 8/13/24 01:59, Jens Axboe wrote: >>> On 8/12/24 6:50 PM, Pavel Begunkov wrote: >>>> On 8/12/24 19:30, Jens Axboe wrote: >>>>> On 8/12/24 12:13 PM, Jens Axboe wrote: >>>>>> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>>>>>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>>>>>> for completions, which makes the kernel to interpret the passed timespec >>>>>>> not as a relative time to wait but rather an absolute timeout. >>>>>>> >>>>>>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>>>>>> >>>>>>> Tests: https://github.com/isilence/liburing.git abs-timeout >>>>>> >>>>>> Looks good to me - was going to ask about tests, but I see you have those >>>>>> already! Thanks. >>>>> >>>>> Took a look at the test, also looks good to me. But we need the man >>>>> pages updated, or nobody will ever know this thing exists. >>>> >>>> If we go into that topic, people not so often read manuals >>>> to learn new features, a semi formal tutorial would be much >>>> more useful, I believe. >>>> >>>> Regardless, I can update mans before sending the tests, I was >>>> waiting if anyone have feedback / opinions on the api. >>> >>> I regularly get people sending corrections or questions after having >>> read man pages, so I'd have to disagree. In any case, if there's one >> >> That doesn't necessarily mean they've learned about the feature from >> the man page. In my experience, people google a problem, find some >> clue like a name of the feature they need and then go to a manual >> (or other source) to learn more. >> >> Which is why I'm not saying that man pages don't have a purpose, on >> the contrary, but there are often more convenient ways of discovering >> in the long run. > > In my experience, you google if you have very little clue what you're > doing, to hopefully learn. And you use a man page, if whatever API you're > using has good man pages, if you're just curius about a specific > function. There's definitely a place for both. > > None of that changes the fact that the liburing man pages should > _always_ document all of the API. Nobody said the opposite, but I don't buy that man pages or lack of thereof somehow mean "nobody will ever know this thing exists".
On Aug 12, 2024, at 8:38 PM, Pavel Begunkov <asml.silence@gmail.com> wrote: > > On 8/13/24 03:09, Jens Axboe wrote: >>> On 8/12/24 7:32 PM, Pavel Begunkov wrote: >>> On 8/13/24 01:59, Jens Axboe wrote: >>>> On 8/12/24 6:50 PM, Pavel Begunkov wrote: >>>>> On 8/12/24 19:30, Jens Axboe wrote: >>>>>> On 8/12/24 12:13 PM, Jens Axboe wrote: >>>>>>> On 8/7/24 8:18 AM, Pavel Begunkov wrote: >>>>>>>> Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting >>>>>>>> for completions, which makes the kernel to interpret the passed timespec >>>>>>>> not as a relative time to wait but rather an absolute timeout. >>>>>>>> >>>>>>>> Patch 4 adds a way to set a clock id to use for CQ waiting. >>>>>>>> >>>>>>>> Tests: https://github.com/isilence/liburing.git abs-timeout >>>>>>> >>>>>>> Looks good to me - was going to ask about tests, but I see you have those >>>>>>> already! Thanks. >>>>>> >>>>>> Took a look at the test, also looks good to me. But we need the man >>>>>> pages updated, or nobody will ever know this thing exists. >>>>> >>>>> If we go into that topic, people not so often read manuals >>>>> to learn new features, a semi formal tutorial would be much >>>>> more useful, I believe. >>>>> >>>>> Regardless, I can update mans before sending the tests, I was >>>>> waiting if anyone have feedback / opinions on the api. >>>> >>>> I regularly get people sending corrections or questions after having >>>> read man pages, so I'd have to disagree. In any case, if there's one >>> >>> That doesn't necessarily mean they've learned about the feature from >>> the man page. In my experience, people google a problem, find some >>> clue like a name of the feature they need and then go to a manual >>> (or other source) to learn more. >>> >>> Which is why I'm not saying that man pages don't have a purpose, on >>> the contrary, but there are often more convenient ways of discovering >>> in the long run. >> In my experience, you google if you have very little clue what you're >> doing, to hopefully learn. And you use a man page, if whatever API you're >> using has good man pages, if you're just curius about a specific >> function. There's definitely a place for both. >> None of that changes the fact that the liburing man pages should >> _always_ document all of the API. > > Nobody said the opposite, but I don't buy that man pages or lack > of thereof somehow mean "nobody will ever know this thing exists". I don’t know why we’re still arguing about this, when the point is that any addition to liburing should come with a man page update. That part isn’t up for debate, and honestly the only thing that matters here. Would be it great to have additional documentation like a tutorial? Certainly. But for a flag that adds absolute timing for waiting rather than relative, just documenting is probably all that’s needed. It’s not like it needs a ton of how to or documentation. If it only exists in a header somewhere, it’s. It going to be easy to discover. — Jens Axboe
Patch 3 allows the user to pass IORING_ENTER_ABS_TIMER while waiting for completions, which makes the kernel to interpret the passed timespec not as a relative time to wait but rather an absolute timeout. Patch 4 adds a way to set a clock id to use for CQ waiting. Tests: https://github.com/isilence/liburing.git abs-timeout v2: Add clock id registration Adjustment using iowq->timeout in Patch 2 Pavel Begunkov (4): io_uring/napi: refactor __io_napi_busy_loop() io_uring/napi: postpone napi timeout adjustment io_uring: add absolute mode wait timeouts io_uring: user registered clockid for wait timeouts include/linux/io_uring_types.h | 3 +++ include/uapi/linux/io_uring.h | 8 ++++++++ io_uring/io_uring.c | 22 ++++++++++++--------- io_uring/io_uring.h | 8 ++++++++ io_uring/napi.c | 35 +++++++++++----------------------- io_uring/napi.h | 16 ---------------- io_uring/register.c | 31 ++++++++++++++++++++++++++++++ 7 files changed, 74 insertions(+), 49 deletions(-)