Re: [PATCH v10 2/5] io-uring: add napi busy poll support

[Jens Axboe <axboe@kernel.dk>]

On 4/26/23 7:41?PM, Jens Axboe wrote:
>> +static void io_napi_multi_busy_loop(struct list_head *napi_list,
>> +		struct io_wait_queue *iowq)
>> +{
>> +	unsigned long start_time = busy_loop_current_time();
>> +
>> +	do {
>> +		if (list_is_singular(napi_list))
>> +			break;
>> +		if (!__io_napi_busy_loop(napi_list, iowq->napi_prefer_busy_poll))
>> +			break;
>> +	} while (!io_napi_busy_loop_should_end(iowq, start_time));
>> +}
> 
> Do we need to check for an empty list here?
> 
>> +static void io_napi_blocking_busy_loop(struct list_head *napi_list,
>> +		struct io_wait_queue *iowq)
>> +{
>> +	if (!list_is_singular(napi_list))
>> +		io_napi_multi_busy_loop(napi_list, iowq);
>> +
>> +	if (list_is_singular(napi_list)) {
>> +		struct io_napi_ht_entry *ne;
>> +
>> +		ne = list_first_entry(napi_list, struct io_napi_ht_entry, list);
>> +		napi_busy_loop(ne->napi_id, io_napi_busy_loop_should_end, iowq,
>> +			iowq->napi_prefer_busy_poll, BUSY_POLL_BUDGET);
>> +	}
>> +}
> 
> Presumably io_napi_multi_busy_loop() can change the state of the list,
> which is why we have if (cond) and then if (!cond) here? Would probably
> warrant a comment as it looks a bit confusing.

Doesn't look like that's the case? We just call into
io_napi_multi_busy_loop() -> napi_busy_loop() which doesn't touch it. So
the state should be the same?

We also check if the list isn't singular before we call it, and then
io_napi_multi_busy_loop() breaks out of the loop if it is. And we know
it's not singular when calling, and I don't see what changes it.

Unless I'm missing something, which is quite possible, this looks overly
convoluted and has extra pointless checks?

-- 
Jens Axboe