On 6/18/21 9:33 AM, Pavel Begunkov wrote:
> On 6/18/21 4:23 PM, Jens Axboe wrote:
>> On 6/17/21 11:14 AM, Pavel Begunkov wrote:
>>> If task_state is cleared, io_req_task_work_add() will go the slow path
>>> adding a task_work, setting the task_state, waking up the task and so
>>> on. Not to mention it's expensive. tctx_task_work() first clears the
>>> state and then executes all the work items queued, so if any of them
>>> resubmits or adds new task_work items, it would unnecessarily go through
>>> the slow path of io_req_task_work_add().
>>>
>>> Let's clear the ->task_state at the end. We still have to check
>>> ->task_list for emptiness afterward to synchronise with
>>> io_req_task_work_add(), do that, and set the state back if we're going
>>> to retry, because clearing not-ours task_state on the next iteration
>>> would be buggy.
>>
>> Are we not re-introducing the problem fixed by 1d5f360dd1a3c by swapping
>> these around?
>
> if (wq_list_empty(&tctx->task_list)) {
> clear_bit(0, &tctx->task_state);
> if (wq_list_empty(&tctx->task_list))
> break;
> ... // goto repeat
> }
Yeah ok, that should do it.
I've applied the series, thanks.
--
Jens Axboe
