Re: [RFC PATCH] membarrier: handle nohz_full with expedited thread registration

[Linus Torvalds]

On Mon, Jan 16, 2017 at 2:56 PM, Mathieu Desnoyers
<mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>
> So for both register and unregister functions, as well as the use in
> membarrier_nohz_full_expedited(), disabling interrupts around the rq
> lock should fix this. But perhaps it would be wiser trying not to use the
> rq lock at all.

Definitely.


>> (d) when it doesn't even make any sense in the first place for a
>> per-thread value that is never modified by any other threads!
>
> The variable "membarrier_expedited" is indeed only modified by the
> current thread, but it is read by other threads calling
> membarrier_nohz_full_expedited().

Why not just make the write be a "smp_store_release()", and the read
be a "smp_load_acquire()". That guarantees a certain amount of
ordering. The only amount that I suspect makes sense, in fact.

But it's not clear what the problem is, so..

>> Oh, and the clone semantics make no sense either.
>
> Currently, this patch clears the state on exec and when forking a new thread,
> but keeps the thread state when forking a new process, which AFAIU is
> in line with current practices. But perhaps not, what I am missing ?

I'm not seeing how a regular fork() could possibly ever make sense to
have the membarrier state in the newly forked process. Not that
"fork()" is really well-defined for within a single thread anyway (it
actually is as far as Linux is concerned, but not in POSIX, afaik).

So if there is no major reason for it, I would strongly suggest that
_if_ all this makes sense in the first place, the membarrier thing
should just be cleared unconditionally both for exec and for
clone/fork.

              Linus