All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Mike Frysinger <vapier@gentoo.org>,
	uclinux-dist-devel@blackfin.uclinux.org,
	linux-pm@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [linux-pm] [uclinux-dist-devel] freezer: should barriers be smp?
Date: Fri, 15 Apr 2011 00:34:21 +0200	[thread overview]
Message-ID: <201104150034.21937.rjw@sisk.pl> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1104141040060.2487-100000@iolanthe.rowland.org>

On Thursday, April 14, 2011, Alan Stern wrote:
> On Thu, 14 Apr 2011, Rafael J. Wysocki wrote:
> 
> > On Thursday, April 14, 2011, Alan Stern wrote:
> > > On Wed, 13 Apr 2011, Rafael J. Wysocki wrote:
> > > 
> > > > The above means that smp_*mb() are defined as *mb() if CONFIG_SMP is set,
> > > > which basically means that *mb() are more restrictive than the corresponding
> > > > smp_*mb().  More precisely, they also cover the cases in which the CPU
> > > > reorders instructions on uniprocessor, which we definitely want to cover.
> > > > 
> > > > IOW, your patch would break things on uniprocessor where the CPU reorders
> > > > instructions.
> > > 
> > > How could anything break on a UP system?  CPUs don't reorder 
> > > instructions that drastically.  For example, no CPU will ever violate
> > > this assertion:
> > > 
> > > 	x = 0;
> > > 	y = x;
> > > 	x = 1;
> > > 	assert(y == 0);
> > > 
> > > even if it does reorder the second and third statements internally.  
> > > This is guaranteed by the C language specification.
> > 
> > Well, you conveniently removed the patch from your reply. :-)
> 
> All the patch does is replace an instance of wmb() with smp_wmb() and 
> an instance of rmb() with smp_rmb().
> 
> > For example, there's no reason why the CPU cannot reorder things so that
> > the "if (frozen(p))" is (speculatively) done before the "if (!freezing(p))"
> > if there's only a compiler barrier between them.
> 
> That's true.  On an SMP system, smp_wmb() is identical to wmb(), so
> there will be a true memory barrier when it is needed.  On a UP system,
> reordering the instructions in this way will not change the final
> result -- in particular, it won't break anything.
> 
> In your example, the two tests look at different flags in *p.  
> Speculative reordering of the tests won't make any difference unless
> one of the flags gets changed in between.  On a UP system, the only way
> the flag can be changed is for the CPU to change it, in which case
> the CPU would obviously know that the speculative result had to be
> invalidated.

Note, however, that preemption may happen basically at any time, so the
task that executes the two "if" statements can be preempted after it has
loaded p->flags into a register and before it checks the TIF_FREEZE (if
they are reordered).  In that case the p->flags (in memory) may be
changed by another task in the meantime.

> > > > > Documentation/memory-barriers.txt:
> > > > > SMP memory barriers are reduced to compiler barriers on uniprocessor compiled
> > > > > systems because it is assumed that a CPU will appear to be self-consistent,
> > > > > and will order overlapping accesses correctly with respect to itself.
> > > > 
> > > > Exactly, which is not guaranteed in general (e.g. on Alpha).  That is, some
> > > > CPUs can reorder instructions in such a way that a compiler barrier is not
> > > > sufficient to prevent breakage.
> > > 
> > > I don't think this is right.  You _can_ assume that Alphas appear to be
> > > self-consistent.  If they didn't, you wouldn't be able to use them at
> > > all.
> > 
> > I'm quite convinced that the statement "some CPUs can reorder instructions in
> > such a way that a compiler barrier is not sufficient to prevent breakage" is
> > correct.
> 
> No.  The correct statement is "Some CPUs can reorder instructions in 
> such a way that a compiler barrier is not sufficient to prevent 
> breakage on SMP systems."

That's if preemption is not taken into account.

> Just for kicks...  Which was added to the kernel first: SMP support or 
> memory barriers?  I don't know the answer; it would take a fair amount 
> of digging to find out.

I have no idea. :-)

Rafael

  reply	other threads:[~2011-04-14 22:34 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-13  6:14 freezer: should barriers be smp ? Mike Frysinger
2011-04-13 20:58 ` Rafael J. Wysocki
2011-04-13 20:58 ` Rafael J. Wysocki
2011-04-13 21:02   ` Mike Frysinger
2011-04-13 21:05     ` Pavel Machek
2011-04-13 21:05     ` Pavel Machek
2011-04-13 21:11       ` [uclinux-dist-devel] " Mike Frysinger
2011-04-13 21:53         ` Rafael J. Wysocki
2011-04-13 21:53         ` Rafael J. Wysocki
2011-04-13 22:11           ` Alan Stern
2011-04-13 22:11           ` [linux-pm] " Alan Stern
2011-04-13 22:34             ` [uclinux-dist-devel] freezer: should barriers be smp? Rafael J. Wysocki
2011-04-13 22:34             ` [linux-pm] " Rafael J. Wysocki
2011-04-14 14:55               ` Alan Stern
2011-04-14 14:55               ` [linux-pm] " Alan Stern
2011-04-14 22:34                 ` Rafael J. Wysocki [this message]
2011-04-15 14:32                   ` Alan Stern
2011-04-15 14:32                   ` Alan Stern
2011-04-14 22:34                 ` Rafael J. Wysocki
2011-04-13 22:22           ` [uclinux-dist-devel] freezer: should barriers be smp ? Mike Frysinger
2011-04-13 22:22           ` Mike Frysinger
2011-04-13 22:49             ` Rafael J. Wysocki
2011-04-13 22:49             ` Rafael J. Wysocki
2011-04-13 22:53               ` Rafael J. Wysocki
2011-04-13 22:53               ` Rafael J. Wysocki
2011-04-13 22:57               ` Mike Frysinger
2011-04-13 23:12                 ` Rafael J. Wysocki
2011-04-13 23:12                 ` Rafael J. Wysocki
2011-04-14 15:13                 ` Alan Stern
2011-04-14 15:13                 ` [linux-pm] " Alan Stern
2011-04-14 22:40                   ` Rafael J. Wysocki
2011-04-14 22:40                   ` Rafael J. Wysocki
2011-04-13 22:57               ` Mike Frysinger
2011-04-13 22:04         ` [linux-pm] [uclinux-dist-devel] " Alan Stern
2011-04-15 16:29           ` Pavel Machek
2011-04-15 16:29           ` [linux-pm] " Pavel Machek
2011-04-15 16:33             ` Mike Frysinger
2011-04-15 16:33             ` [uclinux-dist-devel] [linux-pm] " Mike Frysinger
2011-04-15 16:57               ` Pavel Machek
2011-04-15 16:57               ` [uclinux-dist-devel] " Pavel Machek
2011-04-15 23:11               ` Rafael J. Wysocki
2011-04-15 23:11               ` [uclinux-dist-devel] [linux-pm] " Rafael J. Wysocki
2011-04-15 23:24                 ` Mike Frysinger
2011-04-15 23:30                   ` [uclinux-dist-devel] " Rafael J. Wysocki
2011-04-15 23:30                   ` [uclinux-dist-devel] [linux-pm] " Rafael J. Wysocki
2011-04-15 23:24                 ` [uclinux-dist-devel] " Mike Frysinger
2011-04-13 22:04         ` Alan Stern
2011-04-13 21:11       ` Mike Frysinger
2011-04-13 21:02   ` Mike Frysinger

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=201104150034.21937.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=stern@rowland.harvard.edu \
    --cc=uclinux-dist-devel@blackfin.uclinux.org \
    --cc=vapier@gentoo.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.