[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190904160953.GU2332@hirez.programming.kicks-ass.net>
Date: Wed, 4 Sep 2019 18:09:53 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
paulmck <paulmck@...ux.ibm.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
Oleg Nesterov <oleg@...hat.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
"Russell King, ARM Linux" <linux@...linux.org.uk>,
Chris Metcalf <cmetcalf@...hip.com>,
Chris 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 Wed, Sep 04, 2019 at 11:19:00AM -0400, Mathieu Desnoyers wrote:
> ----- On Sep 3, 2019, at 4:36 PM, Linus Torvalds torvalds@...ux-foundation.org wrote:
> > 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.
>
> One issue here is that membarrier iterates over all runqueues without
> grabbing any runqueue lock. If we copy that state from mm to rq on
> sched switch prepare, we would need to ensure we have the proper
> memory barriers between:
>
> prior user-space memory accesses / setting the runqueue membarrier state
>
> and
>
> setting the runqueue membarrier state / following user-space memory accesses
>
> Copying the membarrier state into the task struct leverages the fact that
> we have documented and guaranteed those barriers around the rq->curr update
> in the scheduler.
Should be the same as the barriers we already rely on for rq->curr, no?
That is, if we put this before switch_mm() then we have
smp_mb__after_spinlock() and switch_mm() itself.
Also, if we place mm->membarrier_state in the same cacheline as mm->pgd
(which switch_mm() is bound to load) then we should be fine, I think.
Powered by blists - more mailing lists