Messages in this thread | | | Date | Tue, 11 Aug 2020 17:23:10 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH] task_work: only grab task signal lock when needed |
| |
On 08/11, Jens Axboe wrote: > > --- 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 (!(READ_ONCE(task->jobctl) & JOBCTL_TASK_WORK) && > + lock_task_sighand(task, &flags)) {
Aaaaah, sorry Jens, now I think this is racy. So I am glad I didn't add this optimization into the initial version ;)
It is possible that JOBCTL_TASK_WORK is set but ->task_works == NULL. Say, task_work_add(TWA_SIGNAL) + task_work_cancel(), or the target task can call task_work_run() before it enters get_signal().
And in this case another task_work_add(tsk, TWA_SIGNAL) can actually race with get_signal() which does
current->jobctl &= ~JOBCTL_TASK_WORK; if (unlikely(current->task_works)) { spin_unlock_irq(&sighand->siglock); task_work_run();
nothing guarantees that get_signal() sees ->task_works != NULL. Probably this is what Jann meant.
We can probably add a barrier into get_signal() but I didn't sleep today, I'll try to think tomorrow.
Oleg.
| |