All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	"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 11:48:57 -0500	[thread overview]
Message-ID: <20100201164856.GA3486@Krystal> (raw)
In-Reply-To: <alpine.LFD.2.00.1002010816030.4206@localhost.localdomain>

* Linus Torvalds (torvalds@linux-foundation.org) wrote:
> 
> 
> On Mon, 1 Feb 2010, Mathieu Desnoyers wrote:
> > 
> > However, this does not deal with mm_cpumask update, and we cannot use
> > the per-cpu rq lock, as it's a process-wide data structure updated with
> > clear_bit/set_bit in switch_mm(). So at the very least, we would have to
> > add memory barriers in switch_mm() on some architectures to deal with
> > this.
> 
> I'd much rather have a "switch_mm()" is a guaranteed memory barrier logic, 
> because quite frankly, I don't see how it ever couldn't be one anyway. It 
> fundamentally needs to do at least a TLB context switch (which may be just 
> switching an ASI around, not flushing the whole TLB, of course), and I bet 
> that for 99% of all architectures, that is already pretty much guaranteed 
> to be equivalent to a memory barrier.
> 
> It certainly is for x86. "mov to cr0" is serializing (setting any control 
> register except cr8 is serializing). And I strongly suspect other 
> architectures will be too.

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.

> 
> Btw, one reason to strongly prefer "switch_mm()" over any random context 
> switch is that at least it won't affect inter-thread (kernel or user-land) 
> switching, including switching to/from the idle thread.
> 
> So I'd be _much_ more open to a "let's guarantee that 'switch_mm()' always 
> implies a memory barrier" model than to playing clever games with 
> spinlocks.

If we really want to make this patch less intrusive, we can consider
iterating on each online cpu in sys_membarrier() rather than on the
mm_cpumask. But it comes at the cost of useless cache-line bouncing on
large machines with few threads running in the process, as we would grab
the rq locks one by one for all cpus.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2010-02-01 16:49 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 [this message]
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
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=20100201164856.GA3486@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --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=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 \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.