[PATCH] io_uring: add napi busy settings to the fdinfo output

[PATCH] io_uring: add napi busy settings to the fdinfo output

[Olivier Langlois]

this info may be useful when attempting to debug a problem
involving a ring using the feature.

Here is an example of the output:
ip-172-31-39-89 /proc/772/fdinfo # cat 14
pos:	0
flags:	02000002
mnt_id:	16
ino:	10243
SqMask:	0xff
SqHead:	633
SqTail:	633
CachedSqHead:	633
CqMask:	0x3fff
CqHead:	430250
CqTail:	430250
CachedCqTail:	430250
SQEs:	0
CQEs:	0
SqThread:	885
SqThreadCpu:	0
SqTotalTime:	52793826
SqWorkTime:	3590465
UserFiles:	0
UserBufs:	0
PollList:
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=6, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=6, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
  op=10, task_works=0
CqOverflowList:
NAPI:	enabled
napi_busy_poll_to:	1
napi_prefer_busy_poll:	true

Signed-off-by: Olivier Langlois <olivier@trillion01.com>
---
 io_uring/fdinfo.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
index b1e0e0d85349..3ba42e136a40 100644
--- a/io_uring/fdinfo.c
+++ b/io_uring/fdinfo.c
@@ -221,7 +221,18 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
 			   cqe->user_data, cqe->res, cqe->flags);
 
 	}
-
+#ifdef CONFIG_NET_RX_BUSY_POLL
+	if (ctx->napi_enabled) {
+		seq_puts(m, "NAPI:\tenabled\n");
+		seq_printf(m, "napi_busy_poll_dt:\t%llu\n", ctx->napi_busy_poll_dt);
+		if (ctx->napi_prefer_busy_poll)
+			seq_puts(m, "napi_prefer_busy_poll:\ttrue\n");
+		else
+			seq_puts(m, "napi_prefer_busy_poll:\tfalse\n");
+	} else {
+		seq_puts(m, "NAPI:\tdisabled\n");
+	}
+#endif
 	spin_unlock(&ctx->completion_lock);
 }
 #endif
-- 
2.45.2

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Pavel Begunkov]

On 7/29/24 23:38, Olivier Langlois wrote:
> this info may be useful when attempting to debug a problem
> involving a ring using the feature.

Apart from a comment below,

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

Maybe, Jens would be willing to move the block after the spin_unlock
while applying.


> Signed-off-by: Olivier Langlois <olivier@trillion01.com>
> ---
>   io_uring/fdinfo.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> index b1e0e0d85349..3ba42e136a40 100644
> --- a/io_uring/fdinfo.c
> +++ b/io_uring/fdinfo.c
> @@ -221,7 +221,18 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
>   			   cqe->user_data, cqe->res, cqe->flags);
>   
>   	}
> -
> +#ifdef CONFIG_NET_RX_BUSY_POLL
> +	if (ctx->napi_enabled) {
> +		seq_puts(m, "NAPI:\tenabled\n");
> +		seq_printf(m, "napi_busy_poll_dt:\t%llu\n", ctx->napi_busy_poll_dt);
> +		if (ctx->napi_prefer_busy_poll)
> +			seq_puts(m, "napi_prefer_busy_poll:\ttrue\n");
> +		else
> +			seq_puts(m, "napi_prefer_busy_poll:\tfalse\n");
> +	} else {
> +		seq_puts(m, "NAPI:\tdisabled\n");
> +	}
> +#endif
>   	spin_unlock(&ctx->completion_lock);

That doesn't need to be under completion_lock, it should move outside
of the spin section.


>   }
>   #endif

-- 
Pavel Begunkov

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Jens Axboe]

On Mon, 29 Jul 2024 18:38:33 -0400, Olivier Langlois wrote:
> this info may be useful when attempting to debug a problem
> involving a ring using the feature.
> 
> Here is an example of the output:
> ip-172-31-39-89 /proc/772/fdinfo # cat 14
> pos:	0
> flags:	02000002
> mnt_id:	16
> ino:	10243
> SqMask:	0xff
> SqHead:	633
> SqTail:	633
> CachedSqHead:	633
> CqMask:	0x3fff
> CqHead:	430250
> CqTail:	430250
> CachedCqTail:	430250
> SQEs:	0
> CQEs:	0
> SqThread:	885
> SqThreadCpu:	0
> SqTotalTime:	52793826
> SqWorkTime:	3590465
> UserFiles:	0
> UserBufs:	0
> PollList:
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=6, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=6, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
>   op=10, task_works=0
> CqOverflowList:
> NAPI:	enabled
> napi_busy_poll_to:	1
> napi_prefer_busy_poll:	true
> 
> [...]

Applied, thanks!

[1/1] io_uring: add napi busy settings to the fdinfo output
      commit: 0c87670003aabfdf8772e8ee19d8794adab9a7e7

Best regards,
-- 
Jens Axboe

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Olivier Langlois]

Thank you Pavel for your review!

Since I have no indication if Jens did see your comment before applying
my patch, I will prepare another one with your comment addressed.

just ignore it if it is not needed anymore.

On Tue, 2024-07-30 at 12:05 +0100, Pavel Begunkov wrote:
> On 7/29/24 23:38, Olivier Langlois wrote:
> > this info may be useful when attempting to debug a problem
> > involving a ring using the feature.
> 
> Apart from a comment below,
> 
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> Maybe, Jens would be willing to move the block after the spin_unlock
> while applying.
> 
> 
> > Signed-off-by: Olivier Langlois <olivier@trillion01.com>
> > ---
> >   io_uring/fdinfo.c | 13 ++++++++++++-
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> > index b1e0e0d85349..3ba42e136a40 100644
> > --- a/io_uring/fdinfo.c
> > +++ b/io_uring/fdinfo.c
> > @@ -221,7 +221,18 @@ __cold void io_uring_show_fdinfo(struct
> > seq_file *m, struct file *file)
> >   			   cqe->user_data, cqe->res, cqe->flags);
> >   
> >   	}
> > -
> > +#ifdef CONFIG_NET_RX_BUSY_POLL
> > +	if (ctx->napi_enabled) {
> > +		seq_puts(m, "NAPI:\tenabled\n");
> > +		seq_printf(m, "napi_busy_poll_dt:\t%llu\n", ctx-
> > >napi_busy_poll_dt);
> > +		if (ctx->napi_prefer_busy_poll)
> > +			seq_puts(m,
> > "napi_prefer_busy_poll:\ttrue\n");
> > +		else
> > +			seq_puts(m,
> > "napi_prefer_busy_poll:\tfalse\n");
> > +	} else {
> > +		seq_puts(m, "NAPI:\tdisabled\n");
> > +	}
> > +#endif
> >   	spin_unlock(&ctx->completion_lock);
> 
> That doesn't need to be under completion_lock, it should move outside
> of the spin section.
> 
> 
> >   }
> >   #endif
> 

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Pavel Begunkov]

On 7/30/24 15:04, Olivier Langlois wrote:
> Thank you Pavel for your review!
> 
> Since I have no indication if Jens did see your comment before applying
> my patch, I will prepare another one with your comment addressed.

Jens saw it

https://git.kernel.dk/cgit/linux-block/commit/?h=for-6.12/io_uring

-- 
Pavel Begunkov

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Jens Axboe]

On 7/30/24 8:08 AM, Pavel Begunkov wrote:
> On 7/30/24 15:04, Olivier Langlois wrote:
>> Thank you Pavel for your review!
>>
>> Since I have no indication if Jens did see your comment before applying
>> my patch, I will prepare another one with your comment addressed.
> 
> Jens saw it
> 
> https://git.kernel.dk/cgit/linux-block/commit/?h=for-6.12/io_uring

Yep, I shuffled it below the lock. I queued this one for 6.12 as it's
not really 6.11 material, and the other two for 6.11. I did a bit of
commit message editing for all of them.

-- 
Jens Axboe

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Pavel Begunkov]

On 7/29/24 23:38, Olivier Langlois wrote:
> this info may be useful when attempting to debug a problem
> involving a ring using the feature.

While on the topic of busy polling, there is a function
io_napi_adjust_timeout(), it ensures that we don't busy poll for longer
than the passed wait timeout.

Do you use it? I have some doubts in regards to its usefulness, and
would prefer to try get rid of it if there are no users since it's a
hustle.


> CqOverflowList:
> NAPI:	enabled
> napi_busy_poll_to:	1
> napi_prefer_busy_poll:	true
> 
> Signed-off-by: Olivier Langlois <olivier@trillion01.com>
> ---
>   io_uring/fdinfo.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> index b1e0e0d85349..3ba42e136a40 100644
> --- a/io_uring/fdinfo.c
> +++ b/io_uring/fdinfo.c
> @@ -221,7 +221,18 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *file)
>   			   cqe->user_data, cqe->res, cqe->flags);
>   
>   	}
> -
> +#ifdef CONFIG_NET_RX_BUSY_POLL
> +	if (ctx->napi_enabled) {
> +		seq_puts(m, "NAPI:\tenabled\n");
> +		seq_printf(m, "napi_busy_poll_dt:\t%llu\n", ctx->napi_busy_poll_dt);
> +		if (ctx->napi_prefer_busy_poll)
> +			seq_puts(m, "napi_prefer_busy_poll:\ttrue\n");
> +		else
> +			seq_puts(m, "napi_prefer_busy_poll:\tfalse\n");
> +	} else {
> +		seq_puts(m, "NAPI:\tdisabled\n");
> +	}
> +#endif
>   	spin_unlock(&ctx->completion_lock);
>   }
>   #endif

-- 
Pavel Begunkov

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Pavel Begunkov]

On 7/30/24 16:33, Pavel Begunkov wrote:
> While on the topic of busy polling, there is a function
> io_napi_adjust_timeout(), it ensures that we don't busy poll for longer
> than the passed wait timeout.
> 
> Do you use it? I have some doubts in regards to its usefulness, and
> would prefer to try get rid of it if there are no users since it's a
> hustle.

And I need to ask Lewis (CC'ed) the same question (see above).
Looking at

commit 415ce0ea55c5a3afea501a773e002be9ed7149f5
Author: Jens Axboe <axboe@kernel.dk>
Date:   Mon Jun 3 13:56:53 2024 -0600

     io_uring/napi: fix timeout calculation

https://git.kernel.dk/cgit/linux-block/commit/?id=415ce0ea55c5a3afea501a773e002be9ed7149f5


It sounds like you might be using it.

-- 
Pavel Begunkov

Re: [PATCH] io_uring: add napi busy settings to the fdinfo output

[Olivier Langlois]

On Tue, 2024-07-30 at 16:33 +0100, Pavel Begunkov wrote:
> On 7/29/24 23:38, Olivier Langlois wrote:
> > this info may be useful when attempting to debug a problem
> > involving a ring using the feature.
> 
> While on the topic of busy polling, there is a function
> io_napi_adjust_timeout(), it ensures that we don't busy poll for
> longer
> than the passed wait timeout.
> 
> Do you use it? I have some doubts in regards to its usefulness, and
> would prefer to try get rid of it if there are no users since it's a
> hustle.
> 
I am not because I use NAPI busy poll exclusively from SQPOLL thread
which in this context all that matters is that the feature is enabled
or not.

For users doing the polling through the io_uring syscall if the
precision of the timeout is important for them, it might serve that
purpose.