public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,
	akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Nicholas Miell <nmiell@comcast.net>,
	laijs@cn.fujitsu.com, dipankar@in.ibm.com, josh@joshtriplett.org,
	dvhltc@us.ibm.com, niv@us.ibm.com, tglx@linutronix.de,
	peterz@infradead.org, Valdis.Kletnieks@vt.edu,
	dhowells@redhat.com
Subject: Re: [patch 2/3] scheduler: add full memory barriers upon task switch at runqueue lock/unlock
Date: Mon, 1 Feb 2010 09:34:18 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1002010926410.4206@localhost.localdomain> (raw)
In-Reply-To: <1265044392.29013.61.camel@gandalf.stny.rr.com>



On Mon, 1 Feb 2010, Steven Rostedt wrote:

> On Mon, 2010-02-01 at 11:48 -0500, Mathieu Desnoyers wrote:
> 
> > What we have to be careful about here is that it's not enough to just
> > rely on switch_mm() containing a memory barrier. What we really need to
> > enforce is that switch_mm() issues memory barriers both _before_ and
> > _after_ mm_cpumask modification. The "after" part is usually dealt with
> > by the TLB context switch, but the "before" part usually isn't.
> 
> Then we add a smp_mb__before_clear_bit() in the switch_mm() on all archs
> that do not have clear_bit imply a smp_mb().

No. I want to get an _explanation_ first.

And no, "I want to do user-level RCU and I'm looking at mm_cpumask" is not 
an explanation.

In order to explain memory ordering rules, you need to actually show the 
actual accesses on the different cpu's and the ordering requirements 
between them, and explain them. IOW, you need to show

	thread1			thread2
	-------			-------

	some-particular-memop
				mm_cpumask access
	memory barrier
				memory barrier
	mm_cpumask access
				some-particular-memop	
	memory barrier

	some-other-particular-memop

and explain exactly why the memory barriers are needed.

In particular, now I've heard the INSANE statement that we need

  "memory barriers both _before_ and _after_ mm_cpumask modification."

and people need to realize that such statements are totally worthless. 

The fact is, a memory barrier with regards to a single location 
modification makes no sense. Not before, not after. Putting a barrier 
around a single access (even if that "single" access is then a 
read-modify-read one) is a meaningless operation - it has _zero_ semantic 
information.

Memory barriers need to be _between_ two operations, and even then they 
never make sense on their own - you need to have another actor that also 
has a memory barrier, and that you are ordering things with regards to.

Saying that they are "around" one operation is crazy talk. It's a 
nonsensical statement. It shows a total lack of understanding of what a 
barrier is about. You can't put a barrier 'around' anything at all.

So before we go any further, I want to see the graph of barriers AND THE 
THINGS THEY ARE BARRIERS BETWEEN. On both sides. I want to know that you 
guys even know _what_ you are protecting against, and I want it documented 
so that when people say "this would solve it", they can show that yes, you 
actually need barriers at both points.

			Linus

  reply	other threads:[~2010-02-01 17:35 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-31 20:52 [patch 0/3] introduce sys_membarrier(): process-wide memory barrier (v8) Mathieu Desnoyers
2010-01-31 20:52 ` [patch 1/3] Create spin lock/spin unlock with distinct memory barrier Mathieu Desnoyers
2010-02-01  7:25   ` Nick Piggin
2010-02-01 14:08     ` Mathieu Desnoyers
2010-02-01  7:28   ` Nick Piggin
2010-02-01 14:10     ` Mathieu Desnoyers
2010-02-01 15:22   ` Linus Torvalds
2010-02-01 15:41     ` Steven Rostedt
2010-01-31 20:52 ` [patch 2/3] scheduler: add full memory barriers upon task switch at runqueue lock/unlock Mathieu Desnoyers
2010-02-01  7:33   ` Nick Piggin
2010-02-01  9:42     ` Peter Zijlstra
2010-02-01 10:11       ` Nick Piggin
2010-02-01 10:36         ` Peter Zijlstra
2010-02-01 10:49           ` Nick Piggin
2010-02-01 14:47             ` Mathieu Desnoyers
2010-02-01 14:58               ` Nick Piggin
2010-02-01 15:23                 ` Steven Rostedt
2010-02-01 15:44                   ` Steven Rostedt
2010-02-01 16:00                   ` Mike Galbraith
2010-02-01 15:27   ` Linus Torvalds
2010-02-01 16:09     ` Mathieu Desnoyers
2010-02-01 16:23       ` Linus Torvalds
2010-02-01 16:48         ` Mathieu Desnoyers
2010-02-01 16:56           ` Linus Torvalds
2010-02-01 17:45             ` Mathieu Desnoyers
2010-02-01 18:00               ` Steven Rostedt
2010-02-01 18:36               ` Linus Torvalds
2010-02-01 19:56                 ` Mathieu Desnoyers
2010-02-01 20:42                   ` Linus Torvalds
2010-02-01 22:42                     ` Mathieu Desnoyers
2010-02-01 20:33                 ` Steven Rostedt
2010-02-01 20:52                   ` Linus Torvalds
2010-02-01 22:39                     ` Steven Rostedt
2010-02-01 23:09                       ` Steven Rostedt
2010-02-01 17:13           ` Steven Rostedt
2010-02-01 17:34             ` Linus Torvalds [this message]
2010-02-01 16:24       ` Steven Rostedt
2010-02-01 16:29         ` Peter Zijlstra
2010-02-01 16:46           ` Steven Rostedt
2010-02-01 16:11     ` Steven Rostedt
2010-01-31 20:52 ` [patch 3/3] introduce sys_membarrier(): process-wide memory barrier (v8) Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.00.1002010926410.4206@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=dvhltc@us.ibm.com \
    --cc=josh@joshtriplett.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=niv@us.ibm.com \
    --cc=nmiell@comcast.net \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox