All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: "linux-os (Dick Johnson)" <linux-os@analogic.com>
Cc: Linus Torvalds <torvalds@osdl.org>,
	Krzysztof Halasa <khc@pm.waw.pl>, 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 00:09:54 +0200	[thread overview]
Message-ID: <20060707220954.GA23651@elte.hu> (raw)
In-Reply-To: <Pine.LNX.4.61.0607071657580.15580@chaos.analogic.com>


* linux-os (Dick Johnson) <linux-os@analogic.com> wrote:

> >> Now Linus declares that instead of declaring an object volatile
> >> so that it is actually accessed every time it is referenced, he wants
> >> to use a GNU-ism with assembly that tells the compiler to re-read
> >> __every__ variable existing im memory, instead of just one. Go figure!
> >
> > Actually, it's not just me.
> >
> > Read things like the Intel CPU documentation.
> >
> > IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU
> > potentially over-heat, causing degreaded performance, and you're simply
> > not supposed to do it.
> 
> This is a bait and switch argument. [...]

how could it possibly be? Linus has simply replied to your point:

> > > If I have some code that does:
> > > 
> > > extern int spinner;
> > > 
> > > funct(){
> > >      while(spinner)
> > >            ;
> > >
> > > The 'C' compiler has no choice but to actually make that memory 
> > > access and read the variable [...]

Linus observed that the only possible purpose of the code you cited, 
busy-looping, makes no sense.

If you dont intend to busy-loop, why does the code above look like a 
busy-loop, and what do you need the volatile keyword for?

> [...] In fact, your spin-lock code already inserts "rep nops" and I 
> never implied that they should be removed. I said only that "volatile" 
> still needs to be used, not some macro that tells the compiler that 
> everything in memory probably got trashed. [...]

your position here does seem to make much sense to me, so please help me 
understand it. You suggest that the assembly code should be left alone. 
But then why do you need the volatile keyword to begin with?

> Well, in fact I do know what I'm talking about. Your bait-and-switch 
> arguments just won't work with me.

you seem to be quite self-confident. That is a nice thing to have for 
say a pro boxer, but it can be a disadvantage when dealing with a 
complex OS. Just curious, if you turn out to be wrong will you apologize 
for wasting Linus' (and others') time and for insulting him like that? 
(Or is this a totally impossible scenario not even worth discussing?)

> > Repeat after me (or just shut up about things that you not only don't know
> > about, but are apparently not willing to even learn):
> >
> > 	"'volatile' is useless. The things it did 30 years ago are much
> > 	 more complex these days, and need to be tied to much more
> > 	 detailed rules that depend on the actual particular problem,
> > 	 rather than one keyword to the compiler that doesn't actually
> > 	 give enough information for the compiler to do anything useful"
> >
> > Comprende?

here Linus claims that 'volatile' is useless and that the use of 
'volatile' is almost always a sign of bugs. That includes your 
code-samples too. So a proper answer on a technical forum like lkml 
would _not_ be to ignore Linus' point like you did, but to answer it 
directly. There are a couple of ways to answer it:

- you could explain why your sample code _does_ make sense. That's a 
  powerful way of making your point and you'll sure see an answer 
  either rebutting your point or conceding the point.

- or if you know that it makes no sense (minor nit: in which case you 
  should have pointed that out when writing them) then you could either
  correct it, or cite actual kernel code to which your point applies.

- an even better (and by far my most favorite) method of reply would be 
  if you sent an actual kernel patch (ontop of 2.6.18-rc1 plus my 
  volatile-removal patch) that implements your suggestions! That
  would be a perfect way of making your point - one line of patch is 
  worth a thousand words.

	Ingo

  parent reply	other threads:[~2006-07-07 22:14 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
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 [this message]
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
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=20060707220954.GA23651@elte.hu \
    --to=mingo@elte.hu \
    --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=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 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.