All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@osdl.org>
To: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: Albert Cahalan <acahalan@gmail.com>,
	linux-kernel@vger.kernel.org, linux-os@analogic.com,
	khc@pm.waw.pl, mingo@elte.hu, akpm@osdl.org, arjan@infradead.org
Subject: Re: [patch] spinlocks: remove 'volatile'
Date: Sat, 8 Jul 2006 11:53:09 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0607081125440.3869@g5.osdl.org> (raw)
In-Reply-To: <44AF532B.4050504@yahoo.com.au>



On Sat, 8 Jul 2006, Nick Piggin wrote:
> 
> The volatile casting in atomic_* and *_bit seems to be a good idea
> (now that I think about it) [1].
> 
> Because if you had a barrier there, you'd have to reload everything
> used after an atomic_read or set_bit, etc.

Yes and no. The problem _there_ is that we use the wrong inline asm 
constraints.

The "+m" constraint is basically undocumented, and while I think it has 
existed _internally_ in gcc for a long time (gcc inline asms are actually 
fairly closely related to the internal gcc code generation templates, but 
the internal templates generally have capabilities that the inline asms 
don't have), I don't think it has been actually supported for inline asms 
all that time.

But "+m" means "I will both read and write from this memory location", 
which is exactly what we want to have for things like atomic_add() and 
friends.

However, because it is badly documented, and because it didn't even exist 
long ago, we have lots of code (and lots of people) that doesn't even 
_know_ about "+m". So we have code that fakes out the "both read and 
write" part by marking things either volatile, or doing 

	...
	:"=m" (*ptr)
	:"m" (*ptr)
	...

in the constraints (or, in many cases, both).

> [1] Even though I still can't tell the exact semantics of these
>     operations eg. why do we need volatile at all? why do we have
>     volatile in the double underscore (non-atomic) versions?

So we tend to have "volatile" for a couple of different reasons:

 - the above kind of "we couldn't tell the inline assembly well enough 
   what the instruction actually _does_, so we just tell gcc to not mess 
   with it".

   This _generally_ should be replaced with using "+m", so that gcc just 
   knows that we both read and write to the location, and that allows gcc 
   to generate the best possible code, while still generating _correct_ 
   code because gcc knows what is going on and doesn't think the write 
   totally clobbers the old value.

 - many functions are used with random data, and if the caller has a 
   "volatile long array[]" (bad - but hey, it happens), then a function 
   that _acts_ on that array, like the bitops functions, need to take a
   an argument like "volatile long *".

   So for example, "test_bit()", which can act both on volatile arrays 
   _and_ on const arrays, will look like

	int test_bit(int nr, const volatile void * addr);

   which some people think is strange ("Both 'const' _and_ 'volatile'? 
   Isn't that a contradiction in terms?"), but the fact is, it reflects 
   the different callers, not necessarily test_bit() itself.

 - some functions actually really want the volatile access. The x86 IO 
   functions are the best actual example of this:

	static inline unsigned int readl(const volatile void __iomem *addr)
	{
		return *(volatile unsigned int __force *) addr;
	}

   which actually has a _combination_ of the above reason (the incoming 
   argument is already marked "volatile" just because the _caller_ may 
   have marked it that way) and the cast to volatile would be there 
   _regardless_ of the calling convention "volatile", because in this case 
   we actually use it as a way to avoid inline assembly (which a number of
   other architectures need to do, and which x86 too needs to do for the 
   PIO accesses, but we can avoid it in this case)

So those are all real reasons to use volatile, although the first one is 
obviously a case where we no longer should (but at least we have 
reasonably good historical reasons for why we did).

The thing to note that is all of the above reasons are basically 
"volatile" markers on the _code_. We haven't really marked any _data_ 
volatile, we're just saying that certain _code_ will want to act on 
the data in a certain way.

And I think that's generally a sign of "good" use of volatile - it's 
making it obvious that certain specific _use_ of a data may have certain 
rules. 

As mentioned, there is _one_ case where it is valid to use "volatile" on 
real data: it's when you have a "I don't care about locking, and I'm not 
accessign IO memory or something else that may need special care" 
situation.

In the kernel, that _one_ special case used to basically be the "jiffies" 
counter. There's nothing to "lock" there - it just keeps ticking. And it's 
still obviously normal memory, so there's no question about any special 
rules for accesses. And there are no SMP memory ordering issues for 
reading it (for the low bits), since the "jiffies" counter is not really 
tied to anything else, so there are no larger "coherency" rules either.

So in that ONE case, "volatile" is actually fine. We really don't care if 
we read the old value or the new value when it changes, and there's no 
reason to try to synchronize with anythign else. 

There _may_ be some other cases where that would be true, but quite 
frankly, I can't think of any. If the CPU were to have a built-in random 
number generator mapped into memory, that would fall under the same kind 
of rules, but that's basically it.

One final word: in user space, because of how signal handlers work, 
"volatile" can still make sense for exactly the same reasons that 
"jiffies" makes sense in the kernel. You may, for example, have a signal 
handler that updates some flag in memory, and that would basically look 
exactly like the "jiffies" case for your program. 

(In fact, because signals are very expensive to block, you may have more 
of a reason to use a "jiffies" like flag in user space than you have in 
kernel. In the kernel, you'd tend to use a spinlock to protect things. In 
user space, with signals, you may have to use some non-locking algorithm, 
where the generation count etc migth well look like "jiffies").

			Linus

  reply	other threads:[~2006-07-08 18:53 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-08  3:54 [patch] spinlocks: remove 'volatile' Albert Cahalan
2006-07-08  5:25 ` Linus Torvalds
2006-07-08  6:39   ` Nick Piggin
2006-07-08 18:53     ` Linus Torvalds [this message]
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
  -- strict thread matches above, loose matches on Subject: below --
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
2006-07-07 10:21 Chuck Ebbert
2006-07-07 17:09 ` Linus Torvalds
2006-07-05 11:46 [patch] uninline init_waitqueue_*() functions 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:21           ` Linus Torvalds
2006-07-05 21:45             ` Ingo Molnar
2006-07-05 22:09               ` Linus Torvalds
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-06 16:07                       ` 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
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

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=Pine.LNX.4.64.0607081125440.3869@g5.osdl.org \
    --to=torvalds@osdl.org \
    --cc=acahalan@gmail.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=nickpiggin@yahoo.com.au \
    /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.