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
next prev parent 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.