[PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Pavel Begunkov]

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(-)

-- 
2.45.2

[PATCH v2 1/4] io_uring/napi: refactor __io_napi_busy_loop()

[Pavel Begunkov]

we don't need to set ->napi_prefer_busy_poll if we're not going to poll,
do the checks first and all polling preparation after.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/napi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/io_uring/napi.c b/io_uring/napi.c
index 4fd6bb331e1e..a670f49e30ef 100644
--- a/io_uring/napi.c
+++ b/io_uring/napi.c
@@ -301,10 +301,11 @@ void __io_napi_adjust_timeout(struct io_ring_ctx *ctx, struct io_wait_queue *iow
  */
 void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq)
 {
-	iowq->napi_prefer_busy_poll = READ_ONCE(ctx->napi_prefer_busy_poll);
+	if ((ctx->flags & IORING_SETUP_SQPOLL) || !ctx->napi_enabled)
+		return;
 
-	if (!(ctx->flags & IORING_SETUP_SQPOLL) && ctx->napi_enabled)
-		io_napi_blocking_busy_loop(ctx, iowq);
+	iowq->napi_prefer_busy_poll = READ_ONCE(ctx->napi_prefer_busy_poll);
+	io_napi_blocking_busy_loop(ctx, iowq);
 }
 
 /*
-- 
2.45.2

[PATCH v2 2/4] io_uring/napi: postpone napi timeout adjustment

[Pavel Begunkov]

Remove io_napi_adjust_timeout() and move the adjustments out of the
common path into __io_napi_busy_loop(). Now the limit it's calculated
based on struct io_wait_queue::timeout, for which we query current time
another time. The overhead shouldn't be a problem, it's a polling path,
however that can be optimised later by additionally saving the delta
time value in io_cqring_wait().

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c |  1 -
 io_uring/napi.c     | 28 +++++++---------------------
 io_uring/napi.h     | 16 ----------------
 3 files changed, 7 insertions(+), 38 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3942db160f18..9ec07f76ad19 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2423,7 +2423,6 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
 
 		dt = timespec64_to_ktime(ts);
 		iowq.timeout = ktime_add(dt, ktime_get());
-		io_napi_adjust_timeout(ctx, &iowq, dt);
 	}
 
 	if (sig) {
diff --git a/io_uring/napi.c b/io_uring/napi.c
index a670f49e30ef..d39ae20a3db8 100644
--- a/io_uring/napi.c
+++ b/io_uring/napi.c
@@ -271,27 +271,6 @@ int io_unregister_napi(struct io_ring_ctx *ctx, void __user *arg)
 	return 0;
 }
 
-/*
- * __io_napi_adjust_timeout() - adjust busy loop timeout
- * @ctx: pointer to io-uring context structure
- * @iowq: pointer to io wait queue
- * @ts: pointer to timespec or NULL
- *
- * Adjust the busy loop timeout according to timespec and busy poll timeout.
- * If the specified NAPI timeout is bigger than the wait timeout, then adjust
- * the NAPI timeout accordingly.
- */
-void __io_napi_adjust_timeout(struct io_ring_ctx *ctx, struct io_wait_queue *iowq,
-			      ktime_t to_wait)
-{
-	ktime_t poll_dt = READ_ONCE(ctx->napi_busy_poll_dt);
-
-	if (to_wait)
-		poll_dt = min(poll_dt, to_wait);
-
-	iowq->napi_busy_poll_dt = poll_dt;
-}
-
 /*
  * __io_napi_busy_loop() - execute busy poll loop
  * @ctx: pointer to io-uring context structure
@@ -304,6 +283,13 @@ void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq)
 	if ((ctx->flags & IORING_SETUP_SQPOLL) || !ctx->napi_enabled)
 		return;
 
+	iowq->napi_busy_poll_dt = READ_ONCE(ctx->napi_busy_poll_dt);
+	if (iowq->timeout != KTIME_MAX) {
+		ktime_t dt = ktime_sub(iowq->timeout, ktime_get());
+
+		iowq->napi_busy_poll_dt = min_t(u64, iowq->napi_busy_poll_dt, dt);
+	}
+
 	iowq->napi_prefer_busy_poll = READ_ONCE(ctx->napi_prefer_busy_poll);
 	io_napi_blocking_busy_loop(ctx, iowq);
 }
diff --git a/io_uring/napi.h b/io_uring/napi.h
index 88f1c21d5548..87e30b4f8d9e 100644
--- a/io_uring/napi.h
+++ b/io_uring/napi.h
@@ -17,8 +17,6 @@ int io_unregister_napi(struct io_ring_ctx *ctx, void __user *arg);
 
 void __io_napi_add(struct io_ring_ctx *ctx, struct socket *sock);
 
-void __io_napi_adjust_timeout(struct io_ring_ctx *ctx,
-		struct io_wait_queue *iowq, ktime_t to_wait);
 void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq);
 int io_napi_sqpoll_busy_poll(struct io_ring_ctx *ctx);
 
@@ -27,15 +25,6 @@ static inline bool io_napi(struct io_ring_ctx *ctx)
 	return !list_empty(&ctx->napi_list);
 }
 
-static inline void io_napi_adjust_timeout(struct io_ring_ctx *ctx,
-					  struct io_wait_queue *iowq,
-					  ktime_t to_wait)
-{
-	if (!io_napi(ctx))
-		return;
-	__io_napi_adjust_timeout(ctx, iowq, to_wait);
-}
-
 static inline void io_napi_busy_loop(struct io_ring_ctx *ctx,
 				     struct io_wait_queue *iowq)
 {
@@ -86,11 +75,6 @@ static inline bool io_napi(struct io_ring_ctx *ctx)
 static inline void io_napi_add(struct io_kiocb *req)
 {
 }
-static inline void io_napi_adjust_timeout(struct io_ring_ctx *ctx,
-					  struct io_wait_queue *iowq,
-					  ktime_t to_wait)
-{
-}
 static inline void io_napi_busy_loop(struct io_ring_ctx *ctx,
 				     struct io_wait_queue *iowq)
 {
-- 
2.45.2

[PATCH v2 3/4] io_uring: add absolute mode wait timeouts

[Pavel Begunkov]

In addition to current relative timeouts for the waiting loop, where the
timespec argument specifies the maximum time it can wait for, add
support for the absolute mode, with the value carrying a CLOCK_MONOTONIC
absolute time until which we should return control back to the user.

Suggested-by: Lewis Baker <lewissbaker@gmail.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/uapi/linux/io_uring.h |  1 +
 io_uring/io_uring.c           | 15 ++++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 2aaf7ee256ac..afc901502804 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -507,6 +507,7 @@ struct io_cqring_offsets {
 #define IORING_ENTER_SQ_WAIT		(1U << 2)
 #define IORING_ENTER_EXT_ARG		(1U << 3)
 #define IORING_ENTER_REGISTERED_RING	(1U << 4)
+#define IORING_ENTER_ABS_TIMER		(1U << 5)
 
 /*
  * Passed in for io_uring_setup(2). Copied back with updated info on success
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 9ec07f76ad19..5282f9887440 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2387,7 +2387,7 @@ static inline int io_cqring_wait_schedule(struct io_ring_ctx *ctx,
  * Wait until events become available, if we don't already have some. The
  * application must reap them itself, as they reside on the shared cq ring.
  */
-static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
+static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, u32 flags,
 			  const sigset_t __user *sig, size_t sigsz,
 			  struct __kernel_timespec __user *uts)
 {
@@ -2416,13 +2416,13 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
 
 	if (uts) {
 		struct timespec64 ts;
-		ktime_t dt;
 
 		if (get_timespec64(&ts, uts))
 			return -EFAULT;
 
-		dt = timespec64_to_ktime(ts);
-		iowq.timeout = ktime_add(dt, ktime_get());
+		iowq.timeout = timespec64_to_ktime(ts);
+		if (!(flags & IORING_ENTER_ABS_TIMER))
+			iowq.timeout = ktime_add(iowq.timeout, ktime_get());
 	}
 
 	if (sig) {
@@ -3153,7 +3153,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 
 	if (unlikely(flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP |
 			       IORING_ENTER_SQ_WAIT | IORING_ENTER_EXT_ARG |
-			       IORING_ENTER_REGISTERED_RING)))
+			       IORING_ENTER_REGISTERED_RING |
+			       IORING_ENTER_ABS_TIMER)))
 		return -EINVAL;
 
 	/*
@@ -3251,8 +3252,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 			if (likely(!ret2)) {
 				min_complete = min(min_complete,
 						   ctx->cq_entries);
-				ret2 = io_cqring_wait(ctx, min_complete, sig,
-						      argsz, ts);
+				ret2 = io_cqring_wait(ctx, min_complete, flags,
+						      sig, argsz, ts);
 			}
 		}
 
-- 
2.45.2

[PATCH v2 4/4] io_uring: user registered clockid for wait timeouts

[Pavel Begunkov]

Add a new registration opcode IORING_REGISTER_CLOCK, which allows the
user to select which clock id it wants to use with CQ waiting timeouts.
It only allows a subset of all posix clocks and currently supports
CLOCK_MONOTONIC and CLOCK_BOOTTIME.

Suggested-by: Lewis Baker <lewissbaker@gmail.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring_types.h |  3 +++
 include/uapi/linux/io_uring.h  |  7 +++++++
 io_uring/io_uring.c            |  8 ++++++--
 io_uring/io_uring.h            |  8 ++++++++
 io_uring/napi.c                |  2 +-
 io_uring/register.c            | 31 +++++++++++++++++++++++++++++++
 6 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 3315005df117..4b9ba523978d 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -239,6 +239,9 @@ struct io_ring_ctx {
 		struct io_rings		*rings;
 		struct percpu_ref	refs;
 
+		clockid_t		clockid;
+		enum tk_offsets		clock_offset;
+
 		enum task_work_notify_mode	notify_method;
 		unsigned			sq_thread_idle;
 	} ____cacheline_aligned_in_smp;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index afc901502804..48c440edf674 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -596,6 +596,8 @@ enum io_uring_register_op {
 	IORING_REGISTER_NAPI			= 27,
 	IORING_UNREGISTER_NAPI			= 28,
 
+	IORING_REGISTER_CLOCK			= 29,
+
 	/* this goes last */
 	IORING_REGISTER_LAST,
 
@@ -676,6 +678,11 @@ struct io_uring_restriction {
 	__u32 resv2[3];
 };
 
+struct io_uring_clock_register {
+	__u32	clockid;
+	__u32	__resv[3];
+};
+
 struct io_uring_buf {
 	__u64	addr;
 	__u32	len;
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 5282f9887440..20229e72b65c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2377,7 +2377,8 @@ static inline int io_cqring_wait_schedule(struct io_ring_ctx *ctx,
 	ret = 0;
 	if (iowq->timeout == KTIME_MAX)
 		schedule();
-	else if (!schedule_hrtimeout(&iowq->timeout, HRTIMER_MODE_ABS))
+	else if (!schedule_hrtimeout_range_clock(&iowq->timeout, 0,
+						 HRTIMER_MODE_ABS, ctx->clockid))
 		ret = -ETIME;
 	current->in_iowait = 0;
 	return ret;
@@ -2422,7 +2423,7 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, u32 flags,
 
 		iowq.timeout = timespec64_to_ktime(ts);
 		if (!(flags & IORING_ENTER_ABS_TIMER))
-			iowq.timeout = ktime_add(iowq.timeout, ktime_get());
+			iowq.timeout = ktime_add(iowq.timeout, io_get_time(ctx));
 	}
 
 	if (sig) {
@@ -3424,6 +3425,9 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
 	if (!ctx)
 		return -ENOMEM;
 
+	ctx->clockid = CLOCK_MONOTONIC;
+	ctx->clock_offset = 0;
+
 	if ((ctx->flags & IORING_SETUP_DEFER_TASKRUN) &&
 	    !(ctx->flags & IORING_SETUP_IOPOLL) &&
 	    !(ctx->flags & IORING_SETUP_SQPOLL))
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index c2acf6180845..9935819f12b7 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -437,6 +437,14 @@ static inline bool io_file_can_poll(struct io_kiocb *req)
 	return false;
 }
 
+static inline ktime_t io_get_time(struct io_ring_ctx *ctx)
+{
+	if (ctx->clockid == CLOCK_MONOTONIC)
+		return ktime_get();
+
+	return ktime_get_with_offset(ctx->clock_offset);
+}
+
 enum {
 	IO_CHECK_CQ_OVERFLOW_BIT,
 	IO_CHECK_CQ_DROPPED_BIT,
diff --git a/io_uring/napi.c b/io_uring/napi.c
index d39ae20a3db8..1c3e7562e90b 100644
--- a/io_uring/napi.c
+++ b/io_uring/napi.c
@@ -285,7 +285,7 @@ void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq)
 
 	iowq->napi_busy_poll_dt = READ_ONCE(ctx->napi_busy_poll_dt);
 	if (iowq->timeout != KTIME_MAX) {
-		ktime_t dt = ktime_sub(iowq->timeout, ktime_get());
+		ktime_t dt = ktime_sub(iowq->timeout, io_get_time(ctx));
 
 		iowq->napi_busy_poll_dt = min_t(u64, iowq->napi_busy_poll_dt, dt);
 	}
diff --git a/io_uring/register.c b/io_uring/register.c
index e3c20be5a198..57cb85c42526 100644
--- a/io_uring/register.c
+++ b/io_uring/register.c
@@ -335,6 +335,31 @@ static __cold int io_register_iowq_max_workers(struct io_ring_ctx *ctx,
 	return ret;
 }
 
+static int io_register_clock(struct io_ring_ctx *ctx,
+			     struct io_uring_clock_register __user *arg)
+{
+	struct io_uring_clock_register reg;
+
+	if (copy_from_user(&reg, arg, sizeof(reg)))
+		return -EFAULT;
+	if (memchr_inv(&reg.__resv, 0, sizeof(reg.__resv)))
+		return -EINVAL;
+
+	switch (reg.clockid) {
+	case CLOCK_MONOTONIC:
+		ctx->clock_offset = 0;
+		break;
+	case CLOCK_BOOTTIME:
+		ctx->clock_offset = TK_OFFS_BOOT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ctx->clockid = reg.clockid;
+	return 0;
+}
+
 static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 			       void __user *arg, unsigned nr_args)
 	__releases(ctx->uring_lock)
@@ -511,6 +536,12 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 			break;
 		ret = io_unregister_napi(ctx, arg);
 		break;
+	case IORING_REGISTER_CLOCK:
+		ret = -EINVAL;
+		if (!arg || nr_args)
+			break;
+		ret = io_register_clock(ctx, arg);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
-- 
2.45.2

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Jens Axboe]

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.

-- 
Jens Axboe

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Jens Axboe]

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.

-- 
Jens Axboe

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Pavel Begunkov]

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.

-- 
Pavel Begunkov

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Jens Axboe]

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 :-)

-- 
Jens Axboe

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Pavel Begunkov]

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 :-)

-- 
Pavel Begunkov

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Jens Axboe]

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.

-- 
Jens Axboe

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Pavel Begunkov]

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".

-- 
Pavel Begunkov

Re: [PATCH v2 0/4] clodkid and abs mode CQ wait timeouts

[Jens Axboe]

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