Jens Axboe <axboe@xxxxxxxxx> writes:
> 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?
I'll fix it.
