All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Halasa <khc@pm.waw.pl>
To: "linux-os \(Dick Johnson\)" <linux-os@analogic.com>
Cc: "Linus Torvalds" <torvalds@osdl.org>,
	"Ingo Molnar" <mingo@elte.hu>, "Andrew Morton" <akpm@osdl.org>,
	<linux-kernel@vger.kernel.org>, <arjan@infradead.org>
Subject: Re: [patch] spinlocks: remove 'volatile'
Date: Fri, 07 Jul 2006 20:16:16 +0200	[thread overview]
Message-ID: <m34pxt8emn.fsf@defiant.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.61.0607061333450.11071@chaos.analogic.com> (linux-os@analogic.com's message of "Thu, 6 Jul 2006 13:52:25 -0400")

"linux-os \(Dick Johnson\)" <linux-os@analogic.com> writes:

> extern int spinner;
>
> funct(){
>      while(spinner)
>          ;
>
> The 'C' compiler has no choice but to actually make that memory access
> and read the variable because the variable is in another module (a.k.a.
> file).

defiant:/tmp/khc$ gcc --version
gcc (GCC) 4.1.1 20060525 (Red Hat 4.1.1-1)
defiant:/tmp/khc$ cat test.c 
extern int spinner;

void funct(void)
{
     while(spinner)
         ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o 

test.o:     file format elf32-i386

Disassembly of section .text:

00000000 <funct>:
   0:   a1 00 00 00 00          mov    0x0,%eax
   5:   55                      push   %ebp
   6:   89 e5                   mov    %esp,%ebp
   8:   85 c0                   test   %eax,%eax
   a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
  10:   75 fe                   jne    10 <funct+0x10>
  12:   5d                      pop    %ebp
  13:   c3                      ret    

"0x0" is, of course, for relocation.

> However, if I have the same code, but the variable is visible during
> compile time, i.e.,
>
> int spinner=0;
>
> funct(){
>      while(spinner)
>          ;
>
> ... the compiler may eliminate that code altogether because it
> 'knows' that spinner is FALSE, having initialized it to zero
> itself.

defiant:/tmp/khc$ cat test.c 
int spinner = 0;

void funct(void)
{
     while(spinner)
         ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o 

00000000 <funct>:
   0:   a1 00 00 00 00          mov    0x0,%eax
   5:   55                      push   %ebp
   6:   89 e5                   mov    %esp,%ebp
   8:   85 c0                   test   %eax,%eax
   a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
  10:   75 fe                   jne    10 <funct+0x10>
  12:   5d                      pop    %ebp
  13:   c3                      ret    

> Since spinner is global in scope, somebody surely could have
> changed it before funct() was even called, but the current gcc
> 'C' compiler doesn't care and may optimize it away entirely.

Personally I don't think such C compiler even existed. HISOFT C
on ZX Spectrum could be a good candidate but I think it didn't
have any optimizer :-)

> To
> prevent this, you must declare the variable volatile. To do
> otherwise is a bug.

Nope. Volatile just means that every read (and write) must actually
access the variable. Note that the compiler optimized out accesses
to the variable in the loop - while it has to check at the beginning
of funct(), it knows that the variable is constant through funct().

Note that "volatile" is not exactly what we usually want, but it
does the job (i.e., the program doesn't crash, but the code is
probably suboptimal).

> That said, I think that the current
> implementation of 'volatile' is broken because the compiler
> seems to believe that the variable has moved! It recalculates
> the address of the variable as well as accessing its value.
> This is what makes the code generation problematical.

You must be using a heavily broken compiler:

defiant:/tmp/khc$ cat test.c 
volatile int spinner = 0;

void funct(void)
{
     while(spinner)
         ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o 

00000000 <funct>:
   0:   55                      push   %ebp
   1:   89 e5                   mov    %esp,%ebp
   3:   a1 00 00 00 00          mov    0x0,%eax
   8:   85 c0                   test   %eax,%eax
   a:   75 f7                   jne    3 <funct+0x3>
   c:   5d                      pop    %ebp
   d:   c3                      ret    
-- 
Krzysztof Halasa

  parent reply	other threads:[~2006-07-07 18:16 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 [this message]
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
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=m34pxt8emn.fsf@defiant.localdomain \
    --to=khc@pm.waw.pl \
    --cc=akpm@osdl.org \
    --cc=arjan@infradead.org \
    --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 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.