linux-kernel - Re: [RFC PATCH 1/2] Fix: sched/membarrier: p->mm->membarrier

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]

Message-ID: <CAHk-=whfYb5RnJGqDV3W3093XGwOwePV-SxixaWcWM6hmidArg@mail.gmail.com>
Date:   Tue, 3 Sep 2019 13:36:43 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Chris Metcalf <cmetcalf@...hip.com>,
        Christoph Lameter <cl@...ux.com>,
        Kirill Tkhai <tkhai@...dex.ru>, Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFC PATCH 1/2] Fix: sched/membarrier: p->mm->membarrier_state
 racy load

On Tue, Sep 3, 2019 at 1:25 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> Why can't we frob this state into a line/word we already have to
> unconditionally touch, like the thread_info::flags word for example.

I agree, but we don't have any easily used flags left, I think.

But yes, it would be better to not have membarrier always dirty
another cacheline in the scheduler. So instead of

        atomic_set(&t->membarrier_state,
                   atomic_read(&t->mm->membarrier_state));

it migth be better to do something like

        if (mm->membarrier_state)
                atomic_or(&t->membarrier_state, mm->membarrier_state);

or something along those lines - I think we've already brought in the
'mm' struct into the cache anyway, and we'd not do the write (and
dirty the destination cacheline) for the common case of no membarrier
usage.

But yes, it would be better still if we can re-use some already dirty
cache state.

I wonder if the easiest model might be to just use a percpu variable
instead for the membarrier stuff? It's not like it has to be in
'struct task_struct' at all, I think. We only care about the current
runqueues, and those are percpu anyway.

             Linus