All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Chris Snook <csnook@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	torvalds@linux-foundation.org, netdev@vger.kernel.org,
	akpm@linux-foundation.org, ak@suse.de, heiko.carstens@de.ibm.com,
	davem@davemloft.net, schwidefsky@de.ibm.com,
	wensong@linux-vs.org, horms@verge.net.au, wjiang@resilience.com,
	cfriesen@nortel.com, zlynx@acm.org, rpjday@mindspring.com,
	jesper.juhl@gmail.com
Subject: Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
Date: Thu, 9 Aug 2007 08:04:45 -0700	[thread overview]
Message-ID: <20070809150445.GB8424@linux.vnet.ibm.com> (raw)
In-Reply-To: <46BB2A5A.5090006@redhat.com>

On Thu, Aug 09, 2007 at 10:53:14AM -0400, Chris Snook wrote:
> Paul E. McKenney wrote:
> >Why not the same access-once semantics for atomic_set() as
> >for atomic_read()?  As this patch stands, it might introduce
> >architecture-specific compiler-induced bugs due to the fact that
> >atomic_set() used to imply volatile behavior but no longer does.
> 
> When we make the volatile cast in atomic_read(), we're casting an rvalue to 
> volatile.  This unambiguously tells the compiler that we want to re-load 
> that register from memory.  What's "volatile behavior" for an lvalue?

I was absolutely -not- suggesting volatile behavior for lvalues.

Instead, I am asking for volatile behavior from an -rvalue-.  In the
case of atomic_read(), it is the atomic_t being read from.  In the case
of atomic_set(), it is the atomic_t being written to.  As suggested in
my previous email:

#define atomic_set(v,i)		((*(volatile int *)&(v)->counter) = (i))
#define atomic64_set(v,i)	((*(volatile long *)&(v)->counter) = (i))

Again, the architectures that used to have their "counter" declared
as volatile will lose volatile semantics on atomic_set() with your
patch, which might result in bugs due to overly imaginative compiler
optimizations.  The above would prevent any such bugs from appearing.

>                                                                       A 
> write to an lvalue already implies an eventual write to memory, so this 
> would be a no-op. Maybe you'll write to the register a few times before 
> flushing it to memory, but it will happen eventually.  With an rvalue, 
> there's no guarantee that it will *ever* load from memory, which is what 
> volatile fixes.
> 
> I think what you have in mind is LOCK_PREFIX behavior, which is not the 
> purpose of atomic_set.  We use LOCK_PREFIX in the inline assembly for the 
> atomic_* operations that read, modify, and write a value, only because it 
> is necessary to perform that entire transaction atomically.

No LOCK_PREFIX, thank you!!!  I just want to make sure that the compiler
doesn't push the store down out of a loop, split the store, allow the
store to happen twice (e.g., to allow different code paths to be merged),
and all the other tricks that the C standard permits compilers to pull.

						Thanx, Paul

  reply	other threads:[~2007-08-09 15:04 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-09 13:24 [PATCH 1/24] make atomic_read() behave consistently on alpha Chris Snook
2007-08-09 14:32 ` Paul E. McKenney
2007-08-09 14:53   ` Chris Snook
2007-08-09 15:04     ` Paul E. McKenney [this message]
2007-08-09 15:24       ` Chris Snook
2007-08-09 15:50         ` Segher Boessenkool
2007-08-09 16:20           ` Chris Snook
2007-08-09 18:38             ` Segher Boessenkool
2007-08-09 19:05               ` Chris Snook
2007-08-09 19:19                 ` Segher Boessenkool
2007-08-09 19:25                 ` Geert Uytterhoeven
2007-08-09 19:47                   ` Chris Snook
2007-08-09 23:02                     ` Segher Boessenkool
2007-08-09 16:10         ` Paul E. McKenney
2007-08-09 16:36           ` Chris Snook
2007-08-09 16:58             ` Paul E. McKenney
2007-08-09 17:14               ` Chris Snook
2007-08-09 17:41                 ` Paul E. McKenney
2007-08-09 18:13                   ` Chris Snook
2007-08-09 18:45                     ` Paul E. McKenney
2007-08-09 19:24                       ` Chris Snook
2007-08-10  1:28                         ` Paul E. McKenney
2007-08-10 19:49                           ` Chris Snook
2007-08-10 20:26                             ` Paul E. McKenney
2007-08-09 19:17                 ` Segher Boessenkool
2007-08-09 18:51             ` Segher Boessenkool
2007-08-09 19:30               ` Chris Snook
2007-08-10  8:21           ` Herbert Xu
2007-08-10  9:08             ` Andi Kleen
2007-08-10 15:02               ` Paul E. McKenney
2007-08-10 20:07             ` Segher Boessenkool
2007-08-11  0:00               ` Herbert Xu
2007-08-11  0:38                 ` Segher Boessenkool
2007-08-11  0:43                   ` Herbert Xu
2007-08-11  0:50                     ` Segher Boessenkool
2007-08-11  4:38                   ` Valdis.Kletnieks

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=20070809150445.GB8424@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=ak@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=cfriesen@nortel.com \
    --cc=csnook@redhat.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=horms@verge.net.au \
    --cc=jesper.juhl@gmail.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rpjday@mindspring.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wensong@linux-vs.org \
    --cc=wjiang@resilience.com \
    --cc=zlynx@acm.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.