public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Chase Venters <chase.venters@clientec.com>
To: Krzysztof Halasa <khc@pm.waw.pl>
Cc: "linux-os \\\\(Dick Johnson\\\\)" <linux-os@analogic.com>,
	Linus Torvalds <torvalds@osdl.org>, Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@osdl.org>,
	Linux kernel <linux-kernel@vger.kernel.org>,
	arjan@infradead.org
Subject: Re: [patch] spinlocks: remove 'volatile'
Date: Sat, 8 Jul 2006 08:41:14 -0500	[thread overview]
Message-ID: <200607080841.38235.chase.venters@clientec.com> (raw)
In-Reply-To: <m3wtaoe7rk.fsf@defiant.localdomain>

On Saturday 08 July 2006 04:59, Krzysztof Halasa wrote:
> Chase Venters <chase.venters@clientec.com> writes:
> > Locks are supposed to be syncronization points, which is why they
> > ALREADY HAVE "memory" on the clobber list! "memory" IS NECESSARY. The
> > fact that "=m" is changing to "+m" in Linus's patches is because "=m"
> > is in fact insufficient, because it would let the compiler believe
> > we're just going to over-write the value. "volatile" merely hides that
> > bug -- once that bug is _fixed_ (by going to "+m"), "volatile" is no
> > longer useful.
>
> This is a completely different story. "volatile", barrier() and "+m"/"=m"
> aren't sync points. If the variable access isn't atomic you must use
> locking even with volatiles, barriers etc.

You're mincing my words. The reason "memory" is on the clobber list is because 
a lock is supposed to synchronize all memory accesses up to that point. It's 
a fence / a barrier, because if the compiler re-orders your loads/stores 
across the lock, you're in trouble. That's exactly what I was pointing out. 

> > If "volatile" is in use elsewhere (other than locks), it's still
> > probably wrong. In these cases, you can use a barrier, a volatile
> > cast, or an inline asm with a specific clobber.
>
> A volatile cast is just a volatile, moved from data declaration to
> all access points. It doesn't buy you anything.
> barrier() is basically "all-volatile". All of them operate on the same,
> compiler level.

A volatile cast lets you prevent the compiler from always treating the 
variable as volatile.

> If the "volatile" is used the wrong way (which is probably true for most
> cases), then volatile cast and barrier() will be wrong as well. You need
> locks or atomic access, meaningful on hardware level.

No. Linus already described what some example invalid uses of "volatile" are. 
One example is the very one this whole thread is about - using 'volatile' on 
the declaration of the spinlock counter. That usage is _wrong_, and barrier() 
would not be. (Of course, it doesn't directly apply here, because barrier() 
already existed, by necessity, due to the "memory" clobber. And the lock's 
not done in pure C.)

Volatile originally existed to tell the compiler a variable could change at 
will. Because of reordering, it's almost never sufficient with our modern 
compilers and CPUs. That's precisely where barrier() (and/or its hardware 
equivalents) help in places where 'volatile' is wrong. Your statement is 
additionally wrong because one use-case of memory barriers is to safely write 
lock-free code.

Thanks,
Chase

  reply	other threads:[~2006-07-08 13:41 UTC|newest]

Thread overview: 163+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-05  8:49 [patch] uninline init_waitqueue_*() functions Ingo Molnar
2006-07-05  9:31 ` Andrew Morton
2006-07-05  9:32   ` Ingo Molnar
2006-07-05  9:53     ` Andrew Morton
2006-07-05 10:26       ` Ingo Molnar
2006-07-05 11:30         ` Ingo Molnar
2006-07-05 11:46           ` Ingo Molnar
2006-07-05 17:10             ` Andrew Morton
2006-07-05 19:35               ` Ingo Molnar
2006-07-05 20:18                 ` Andrew Morton
2006-07-05 20:36                   ` Linus Torvalds
2006-07-05 20:47                     ` Ingo Molnar
2006-07-05 21:15                       ` Ingo Molnar
2006-07-05 21:21                       ` Linus Torvalds
2006-07-05 21:45                         ` Ingo Molnar
2006-07-05 21:58                           ` Randy.Dunlap
2006-07-05 22:00                             ` Ingo Molnar
2006-07-05 22:10                               ` Randy.Dunlap
2006-07-05 22:27                                 ` Ingo Molnar
2006-07-05 23:15                               ` Adrian Bunk
2006-07-05 22:09                           ` Linus Torvalds
2006-07-05 22:30                             ` Ingo Molnar
2006-07-05 23:11                             ` Linus Torvalds
2006-07-06  8:16                               ` [patch] spinlocks: remove 'volatile' Ingo Molnar
2006-07-06  8:23                                 ` Ingo Molnar
2006-07-06  9:27                                 ` Heiko Carstens
2006-07-06  9:34                                   ` Arjan van de Ven
2006-07-06 11:59                                 ` linux-os (Dick Johnson)
2006-07-06 12:01                                   ` Arjan van de Ven
2006-07-06 12:29                                     ` linux-os (Dick Johnson)
2006-07-06 12:39                                       ` Arjan van de Ven
2006-07-06 13:39                                         ` J.A. Magallón
2006-07-06 13:43                                           ` Arjan van de Ven
2006-07-06 14:05                                           ` Chase Venters
2006-07-06 14:26                                           ` Andreas Schwab
2006-07-06 16:40                                       ` Nick Piggin
2006-07-06 23:19                                       ` David Schwartz
2006-07-06 18:15                                     ` Mark Lord
2006-07-06 19:15                                       ` Linus Torvalds
2006-07-06 19:33                                         ` Chris Friesen
2006-07-06 19:37                                           ` Mark Lord
2006-07-06 20:28                                             ` Chris Friesen
2006-07-06 20:32                                               ` Linus Torvalds
2006-07-06 19:38                                           ` Arjan van de Ven
2006-07-06 19:41                                           ` Måns Rullgård
2006-07-06 19:42                                           ` Jeff Garzik
2006-07-06 19:58                                           ` Linus Torvalds
2006-07-06 20:27                                             ` Mark Lord
2006-07-06 20:40                                             ` Chris Friesen
2006-07-06 21:00                                               ` Linus Torvalds
2006-07-08  8:40                                         ` Avi Kivity
2006-07-08  8:51                                           ` Arjan van de Ven
2006-07-08  9:20                                             ` Avi Kivity
2006-07-08  9:51                                               ` Arjan van de Ven
2006-07-08 10:18                                                 ` Avi Kivity
2006-07-08 10:28                                                   ` Thomas Gleixner
2006-07-09  4:19                                                   ` Benjamin Herrenschmidt
2006-07-09 12:47                                                     ` Avi Kivity
2006-07-09 19:16                                                       ` David Schwartz
2006-07-09 19:51                                                         ` Theodore Tso
2006-07-09 20:40                                                           ` [OT] 'volatile' in userspace Rutger Nijlunsing
2006-07-10  3:42                                                             ` Theodore Tso
2006-07-10 17:00                                                               ` Joshua Hudson
2006-07-10 17:54                                                                 ` Nick Piggin
2006-07-11  7:48                                                                   ` Jan Engelhardt
2006-07-11 11:53                                                                     ` Nick Piggin
2006-07-11 19:09                                                                       ` Björn Steinbrink
2006-07-11 20:55                                                                       ` Jeff Dike
2006-07-10 18:54                                                                 ` Jeff Dike
2006-07-10 20:09                                                                   ` Philippe Troin
2006-07-10 20:52                                                                     ` Jeff Dike
2006-07-06 16:07                                   ` [patch] spinlocks: remove 'volatile' Linus Torvalds
2006-07-06 16:13                                     ` Linus Torvalds
2006-07-06 17:04                                       ` Jeff Garzik
2006-07-06 17:52                                       ` linux-os (Dick Johnson)
2006-07-06 18:00                                         ` Linus Torvalds
2006-07-06 21:02                                           ` J.A. Magallón
2006-07-06 21:12                                             ` Linus Torvalds
2006-07-06 18:10                                         ` Michael Buesch
2006-07-06 18:16                                         ` Chase Venters
2006-07-07 18:16                                         ` Krzysztof Halasa
2006-07-07 19:51                                           ` linux-os (Dick Johnson)
2006-07-07 20:25                                             ` Linus Torvalds
2006-07-07 21:22                                               ` linux-os (Dick Johnson)
2006-07-07 21:48                                                 ` Chase Venters
2006-07-08 10:00                                                   ` Krzysztof Halasa
2006-07-08 13:41                                                     ` Chase Venters [this message]
2006-07-08 20:09                                                       ` Krzysztof Halasa
2006-07-08 20:40                                                         ` Chase Venters
2006-07-08 20:47                                                           ` Chase Venters
2006-07-09 10:57                                                           ` Krzysztof Halasa
2006-07-07 21:59                                                 ` Linus Torvalds
2006-07-07 22:06                                                   ` Linus Torvalds
2006-07-08 20:49                                                   ` Pavel Machek
2006-07-08 21:43                                                     ` Linus Torvalds
2006-07-07 22:05                                                 ` J.A. Magallón
2006-07-07 22:22                                                   ` Chase Venters
2006-07-07 22:37                                                     ` J.A. Magallón
2006-07-08  9:33                                                       ` David Schwartz
2006-07-07 22:49                                                     ` J.A. Magallón
2006-07-07 22:59                                                       ` Vadim Lobanov
2006-07-07 23:18                                                       ` Chase Venters
2006-07-07 23:36                                                   ` Davide Libenzi
2006-07-07 22:09                                                 ` Ingo Molnar
2006-07-08  7:36                                                   ` Ingo Molnar
2006-07-07 20:39                                             ` Krzysztof Halasa
2006-07-07 23:06                                             ` Björn Steinbrink
2006-07-08  8:36                                             ` Avi Kivity
2006-07-06 19:32                                       ` Jan Engelhardt
2006-07-06 20:26                                         ` Jeremy Fitzhardinge
2006-07-06 20:55                                           ` Jan Engelhardt
2006-07-06 21:07                                             ` Linus Torvalds
2006-07-06 19:56                                 ` Linus Torvalds
2006-07-06 20:34                                   ` Linus Torvalds
2006-07-08 22:50                                     ` Ralf Baechle
2006-07-09  3:09                                       ` Linus Torvalds
2006-07-09  3:07                                     ` Keith Owens
2006-07-09  3:29                                       ` Linus Torvalds
2006-07-09  3:43                                         ` Keith Owens
2006-07-09  3:58                                       ` Linus Torvalds
2006-07-09  6:13                                         ` David Miller
2006-07-09 14:28                                         ` Roman Zippel
2006-07-09 15:27                                           ` Linus Torvalds
2006-07-06  8:18                               ` [patch] lockdep: clean up completion initializer in smpboot.c Ingo Molnar
2006-07-06  8:23                               ` [patch] uninline init_waitqueue_*() functions Ingo Molnar
2006-07-06  9:02                                 ` Andrew Morton
2006-07-06  9:12                                   ` [patch] uninline init_waitqueue_head() Ingo Molnar
2006-07-05 20:45                   ` [patch] uninline init_waitqueue_*() functions Ingo Molnar
2006-07-05 10:37     ` Christoph Hellwig
2006-07-05 10:44       ` Andrew Morton
2006-07-05 10:46         ` Andrew Morton
2006-07-05 10:47         ` Ingo Molnar
2006-07-05  9:54 ` [patch] uninline init_waitqueue_*() functions, fix Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2006-07-07 10:21 [patch] spinlocks: remove 'volatile' Chuck Ebbert
2006-07-07 17:09 ` Linus Torvalds
2006-07-08  3:54 Albert Cahalan
2006-07-08  5:25 ` Linus Torvalds
2006-07-08  6:39   ` Nick Piggin
2006-07-08 18:53     ` Linus Torvalds
2006-07-08  9:45 ` Joe Korty
2006-07-08  9:52   ` Arjan van de Ven
2006-07-08  9:59   ` David Schwartz
2006-07-08 10:24   ` Thomas Gleixner
2006-07-08 15:49     ` Albert Cahalan
2006-07-08 18:31       ` Thomas Gleixner
2006-07-08 19:33         ` Albert Cahalan
2006-07-08 20:11           ` Linus Torvalds
2006-07-09 21:10             ` Pavel Machek
2006-07-09 22:18               ` Linus Torvalds
2006-07-10 16:25               ` marty fouts
2006-07-08 23:10           ` David Schwartz
2006-07-08 13:57   ` Andi Kleen
2006-07-08 19:13     ` Linus Torvalds
2006-07-08 10:45 ` Krzysztof Halasa
2006-07-08  6:12 trajce nedev
2006-07-08  6:19 ` Chase Venters
2006-07-08  6:45   ` trajce nedev
2006-07-08  6:58     ` Arjan van de Ven
2006-07-08  7:02     ` Vadim Lobanov
2006-07-08 13:46     ` Chase Venters
2006-07-09  4:39       ` Benjamin Herrenschmidt
2006-07-14  3:30       ` Steven Rostedt
2006-07-08 18:23 ` Linus Torvalds

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=200607080841.38235.chase.venters@clientec.com \
    --to=chase.venters@clientec.com \
    --cc=akpm@osdl.org \
    --cc=arjan@infradead.org \
    --cc=khc@pm.waw.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-os@analogic.com \
    --cc=mingo@elte.hu \
    --cc=torvalds@osdl.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox