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

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Aug 11, 2020 at 08:45:16AM +0200, Oleg Nesterov wrote:
> On 08/11, Jann Horn 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 (!(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);
> > 
> > I think that should work in theory, but if you want to be able to do a
> > proper unlocked read of task->jobctl here, then I think you'd have to
> > use READ_ONCE() here
> 
> Agreed,
> 
> > and make all existing writes to ->jobctl use
> > WRITE_ONCE().
> 
> ->jobctl is always modified with ->siglock held, do we really need
> WRITE_ONCE() ?

In theory, yes. The compiler doesn't know about locks, it can tear
writes whenever it feels like it. In practise it doesn't happen much,
but...