Re: [PATCH 2/2] io_uring: use TWA_SIGNAL for task_work if the task isn't running

[Jens Axboe <axboe@kernel.dk>]

On 8/10/20 3:28 PM, Jens Axboe wrote:
> On 8/10/20 3:26 PM, Jann Horn wrote:
>> On Mon, Aug 10, 2020 at 11:12 PM Jens Axboe <axboe@kernel.dk> wrote:
>>> On 8/10/20 3:10 PM, Peter Zijlstra wrote:
>>>> On Mon, Aug 10, 2020 at 03:06:49PM -0600, Jens Axboe wrote:
>>>>
>>>>> should work as far as I can tell, but I don't even know if there's a
>>>>> reliable way to do task_in_kernel().
>>>>
>>>> Only on NOHZ_FULL, and tracking that is one of the things that makes it
>>>> so horribly expensive.
>>>
>>> Probably no other way than to bite the bullet and just use TWA_SIGNAL
>>> unconditionally...
>>
>> Why are you trying to avoid using TWA_SIGNAL? Is there a specific part
>> of handling it that's particularly slow?
> 
> Not particularly slow, but it's definitely heavier than TWA_RESUME. And
> as we're driving any pollable async IO through this, just trying to
> ensure it's as light as possible.
> 
> It's not a functional thing, just efficiency.

Ran some quick testing in a vm, which is worst case for this kind of
thing as any kind of mucking with interrupts is really slow. And the hit
is substantial. Though with the below, we're basically at parity again.
Just for discussion...


diff --git a/kernel/task_work.c b/kernel/task_work.c
index 5c0848ca1287..ea2c683c8563 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -42,7 +42,8 @@ task_work_add(struct task_struct *task, struct callback_head *work, int notify)
 		set_notify_resume(task);
 		break;
 	case TWA_SIGNAL:
-		if (lock_task_sighand(task, &flags)) {
+		if (!(task->jobctl & JOBCTL_TASK_WORK) &&
+		    lock_task_sighand(task, &flags)) {
 			task->jobctl |= JOBCTL_TASK_WORK;
 			signal_wake_up(task, 0);
 			unlock_task_sighand(task, &flags);

-- 
Jens Axboe