Re: [PATCH 2/2] [PATCH] sched: Add smp_rmb() in task rq locking cycles

[Paul E. McKenney]

On Tue, Feb 17, 2015 at 02:05:23PM +0100, Peter Zijlstra wrote:
> On Tue, Feb 17, 2015 at 01:12:58PM +0100, Peter Zijlstra wrote:
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -341,6 +341,22 @@ static struct rq *task_rq_lock(struct ta
> >  		raw_spin_lock_irqsave(&p->pi_lock, *flags);
> >  		rq = task_rq(p);
> >  		raw_spin_lock(&rq->lock);
> > +		/*
> > +		 *	move_queued_task()		task_rq_lock()
> > +		 *
> > +		 *	ACQUIRE (rq->lock)
> > +		 *	[S] ->on_rq = MIGRATING		[L] rq = task_rq()
> > +		 *	WMB (__set_task_cpu())		ACQUIRE (rq->lock);
> > +		 *	[S] ->cpu = new_cpu		[L] task_rq()
> > +		 *					[L] ->on_rq
> > +		 *	RELEASE (rq->lock)
> > +		 *
> > +		 * If we observe the old cpu in task_rq_lock, the acquire of
> > +		 * the old rq->lock will fully serialize against the stores.
> > +		 *
> > +		 * If we observe the new cpu in task_rq_lock, the acquire will
> > +		 * pair with the WMB to ensure we must then also see migrating.
> > +		 */
> >  		if (likely(rq == task_rq(p) && !task_on_rq_migrating(p)))
> >  			return rq;
> >  		raw_spin_unlock(&rq->lock);
> 
> Hey Paul, remember this: https://lkml.org/lkml/2014/7/16/310

I do now.  ;-)

> I just used a creative one :-)

The scenario above?

> BTW, should we attempt to include that table in memory-barriers.txt like
> Mathieu said? As a cheat sheet with references to longer explanations
> for the 'interesting' ones?
> 
> FWIW, we should probably update that table to include control
> dependencies too; we didn't (formally) have those back then I think.
> 
> The blob under SMP BARRIER PAIRING does not mention pairing with control
> dependencies; and I'm rather sure I've done so.

Yep, they should pair as well, though the pairing is limited.
No transitivity, of course.

So the straightforward approach requires eighteen bits per cell, though
some of them are a bit, ummm, "unusual".  Sixteen of these are given by
Scenarios 0-15 in http://lwn.net/Articles/573436/, with the barrier on
the side corresponding to the first column and the barrier on the top
corresponding to the second column.  The seventeenth bit says whether
you get transitivity chaining after the top access, assuming that it
happens later.  The eighteenth bit says whether you get transitivity
chaining before the side access, assuming that it happens earlier.

The following is a rough first guess, filling in only the diagonal.
Some of the entries are no doubt wrong, and getting them right requires
something like 7*7*18 test cases, which will take some time.  So, is
something like this really helpful?


      |   mb  |  wmb  |  rmb  |  rbd  |  acq  |  rel  |  ctl  |
 -----+-------+-------+-------+-------+-------+-------+-------+
   mb | 3ffff |   X   |   X   |   X   |   X   |   X   |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  wmb |   X   | 01000 |   X   |   X   |   X   |   X   |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  rmb |   X   |   X   | 00000 |   X   |   X   |   X   |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  rbd |   X   |   X   |   X   | 00000 |   X   |   X   |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  acq |   X   |   X   |   X   |   X   | 00020 |   X   |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  rel |   X   |   X   |   X   |   X   |   X   | 0cc00 |   X   +
 -----+-------+-------+-------+-------+-------+-------+-------+
  ctl |   X   |   X   |   X   |   X   |   X   |   X   | 00020 +
 -----+-------+-------+-------+-------+-------+-------+-------+

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/