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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 16 Jan 2017 15:50:40 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
        rostedt <rostedt@...dmis.org>,
        Nicholas Miell <nmiell@...cast.net>,
        Ingo Molnar <mingo@...hat.com>,
        One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
        Lai Jiangshan <laijs@...fujitsu.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        David Howells <dhowells@...hat.com>,
        bobby prani <bobby.prani@...il.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Shuah Khan <shuahkh@....samsung.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC PATCH] membarrier: handle nohz_full with expedited thread registration

On Mon, Jan 16, 2017 at 2:56 PM, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> 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