Re: Perf hotplug lockup in v4.9-rc8

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	jeremy.linton@arm.com, Boqun Feng <boqun.feng@gmail.com>
Subject: Re: Perf hotplug lockup in v4.9-rc8
Date: Thu, 22 Dec 2016 08:33:30 -0800	[thread overview]
Message-ID: <20161222163330.GT3924@linux.vnet.ibm.com> (raw)
In-Reply-To: <20161222140010.GY3174@twins.programming.kicks-ass.net>

On Thu, Dec 22, 2016 at 03:00:10PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 22, 2016 at 09:45:09AM +0100, Peter Zijlstra wrote:
> > On Mon, Dec 12, 2016 at 01:42:28PM +0100, Peter Zijlstra wrote:
> 
> > > > What are you trying to order here?
> > > 
> > > I suppose something like this:
> > > 
> > > 
> > > CPU0		CPU1		CPU2
> > > 
> > > 		(current == t)
> > > 
> > > t->perf_event_ctxp[] = ctx;
> > > smp_mb();
> > > cpu = task_cpu(t);
> > > 
> > > 		switch(t, n);
> > > 				migrate(t, 2);
> > > 				switch(p, t);
> > > 
> > > 				ctx = t->perf_event_ctxp[]; // must not be NULL
> > > 
> > 
> > So I think I can cast the above into a test like:
> > 
> >   W[x] = 1                W[y] = 1                R[z] = 1
> >   mb                      mb                      mb
> >   R[y] = 0                W[z] = 1                R[x] = 0
> > 
> > Where x is the perf_event_ctxp[], y is our task's cpu and z is our task
> > being placed on the rq of cpu2.
> > 
> > See also commit: 8643cda549ca ("sched/core, locking: Document
> > Program-Order guarantees"), Independent of which cpu initiates the
> > migration between CPU1 and CPU2 there is ordering between the CPUs.
> 
> I think that when we assume RCpc locks, the above CPU1 mb ends up being
> something like an smp_wmb() (ie. non transitive). CPU2 needs to do a
> context switch between observing the task on its runqueue and getting to
> switching in perf-events for the task, which keeps that a full mb.
> 
> Now, if only this model would have locks in ;-)

Yeah, we are slow.  ;-)

But you should be able to emulate them with xchg_acquire() and
smp_store_release().

							Thanx, Paul

> > This would then translate into something like:
> > 
> >   C C-peterz
> > 
> >   {
> >   }
> > 
> >   P0(int *x, int *y)
> >   {
> > 	  int r1;
> > 
> > 	  WRITE_ONCE(*x, 1);
> > 	  smp_mb();
> > 	  r1 = READ_ONCE(*y);
> >   }
> > 
> >   P1(int *y, int *z)
> >   {
> > 	  WRITE_ONCE(*y, 1);
> > 	  smp_mb();
> 
> And this modified to: smp_wmb()
> 
> > 	  WRITE_ONCE(*z, 1);
> >   }
> > 
> >   P2(int *x, int *z)
> >   {
> > 	  int r1;
> > 	  int r2;
> > 
> > 	  r1 = READ_ONCE(*z);
> > 	  smp_mb();
> > 	  r2 = READ_ONCE(*x);
> >   }
> > 
> >   exists
> >   (0:r1=0 /\ 2:r1=1 /\ 2:r2=0)
> 
> Still results in the same outcome.
> 
> If however we change P2's barrier into a smp_rmb() it does become
> possible, but as said above, there's a context switch in between which
> implies a full barrier so no worries.
> 
> Similar if I replace everything z with smp_store_release() and
> smp_load_acquire().
> 
> 
> Of course, its entirely possible the litmus test doesn't reflect
> reality, I still find it somewhat hard to write these things.
> 