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
