On 08/11, Peter Zijlstra wrote:
>
> On Tue, Aug 11, 2020 at 09:14:02AM +0200, Oleg Nesterov wrote:
> > On 08/11, Peter Zijlstra wrote:
> > >
> > > On Tue, Aug 11, 2020 at 08:45:16AM +0200, Oleg Nesterov wrote:
> > > >
> > > > ->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.
> >
> > Yes, but why does this matter? Could you spell please?
>
> Ah, well, that I don't konw. Why do we need the READ_ONCE() ?
>
> It does:
>
> > + if (!(task->jobctl & JOBCTL_TASK_WORK) &&
> > + lock_task_sighand(task, &flags)) {
>
> and the lock_task_sighand() implies barrier(), so I thought the reason
> for the READ_ONCE() was load-tearing, and then we need WRITE_ONCE() to
> avoid store-tearing.
I don't think we really need READ_ONCE() for correctness, compiler can't
reorder this LOAD with cmpxchg() above, and I think we don't care about
load-tearing.
But I guess we need READ_ONCE() or data_race() to shut kcsan up.
Oleg.
