Re: napi_busy_poll

[Olivier Langlois <olivier@trillion01.com>]

On Mon, 2022-02-21 at 13:03 +0800, Hao Xu wrote:
> > 
> > but I think that there is a possible race condition where the
> > napi_list
> > could be used from io_cqring_wait() while another thread modify the
> > list. This is NOT done in my testing scenario but definitely
> > something
> > that could happen somewhere in the real world...
> 
> Will there be any issue if we do the access with
> list_for_each_entry_safe? I think it is safe enough.

Hi Hao,

If napi_busy_poll is exclusively done from the sqpoll thread, all is
good because all the napi_list manipulations are performed from the
sqpoll thread.

The issue is if we want to offer napi_busy_poll for a task calling
io_uring_enter(). If the busy_poll is performed from io_cqring_wait()
as I propose in my patch, the napi_list could be updated by a different
thread calling io_uring_enter() to submit other requests.

This is an issue that v2 is addressing. This makes the code uglier. The
strategy being to splice the context napi_list into a local list in
io_cqring_wait() and assume that the most likely outcome when the
busy_poll will be over the only thing that will be needed is to move
back the local list into the context. If in the meantime, the context
napi_list has been updated, the lists are going to be merged. This
appears to be the approach minimizing the amount of memory allocations.

Creating a benchmark program took more time than I originally expected.
I am going to run it and if gains from napi_polling from
io_cqring_wait() aren't that good... maybe ditching napi_busy_poll()
support from io_cqring_wait() and that way, locking the lock before
adding napi ids will not be required anymore...

Here is what will be added in v2:
- Evaluate list_empty(&ctx->napi_list) outside io_napi_busy_loop() to
keep __io_sq_thread() execution as fast as possible
- In io_cqring_wait(), move up the sig block to avoid needless
computation if the block exits the function
- In io_cqring_wait(), protect ctx->napi_list from race condition by
splicing it into a local list
- In io_cqring_wait(), allow busy polling when uts is missing
- Fix kernel test robot issues