On 09/05/07, Jonathan Corbet <[email protected]> wrote:
OK, here's an updated version of the volatile document - as a plain text
file this time. It drops a new file in Documentation/, but might it be
better as an addition to CodingStyle?
IMHO this is better as a sepperate document rather than an adition to
CodingStyle. The use of volatile is not a style issue, it's a
correctness issue, so it doesn't belong in the CodingStyle document.
Comments welcome,
Find a few below :)
jon
Tell kernel developers why they shouldn't use volatile.
Signed-off-by: Jonathan Corbet <[email protected]>
diff -ruNp linux-2.6/Documentation/volatile.txt volatile/Documentation/volatile.txt
Might I suggest a different filename: volatile-considered-harmful.txt
--- linux-2.6/Documentation/volatile.txt 1969-12-31 17:00:00.000000000 -0700
+++ volatile/Documentation/volatile.txt 2007-05-09 14:56:40.000000000 -0600
@@ -0,0 +1,127 @@
+Why the "volatile" type class should not be used
+------------------------------------------------
+
+The C Programming Language, Second Edition (copyright 1988, still known as
+"the new C book") has the following to say about the volatile keyword:
+
+ The purpose of volatile is to force an implementation to suppress
+ optimization that could otherwise occur. For example, for a
+ machine with memory-mapped input/output, a pointer to a device
+ register might be declared as a pointer to volatile, in
+ order to prevent the compiler from removing apparently redundant
+ references through the pointer.
+
+C programmers have often taken volatile to mean that the variable could be
+changed outside of the current thread of execution; as a result, they are
you write: "... that the variable could be changed outside of the
current thread of execution ..."
I suggest: "... that the variable could be changed outside of the
current thread of execution - a sort of simple atomic variable ..."
+sometimes tempted to use it in kernel code when shared data structures are
+being used. Andrew Morton recently called out[1] use of volatile in a
+submitted patch, saying:
+
+ The volatiles are a worry - volatile is said to be
+ basically-always-wrong in-kernel, although we've never managed to
+ document why, and i386 cheerfully uses it in readb() and friends.
+
+In response, Randy Dunlap pulled together some email from Linus[2] on the
+topic and suggested that we could maybe "document why." Here is the
+result.
+
+The point that Linus often makes with regard to volatile is that
+its purpose is to suppress optimization, which is almost never what one
+really wants to do. In the kernel, one must protect accesses to data
+against race conditions, which is very much a different task.
+
+Like volatile, the kernel primitives which make concurrent access to data
+safe (spinlocks, mutexes, memory barriers, etc.) are designed to prevent
+unwanted optimization. If they are being used properly, there will be no
+need to use volatile as well. If volatile is still necessary, there is
+almost certainly a bug in the code somewhere. In properly-written kernel
+code, volatile can only serve to slow things down.
+
+Consider a typical block of kernel code:
+
+ spin_lock(&the_lock);
+ do_something_on(&shared_data);
+ do_something_else_with(&shared_data);
+ spin_unlock(&the_lock);
+
+If all the code follows the locking rules, the value of shared_data cannot
+change unexpectedly while the_lock is held. Any other code which might
+want to play with that data will be waiting on the lock. The spinlock
+primitives act as memory barriers - they are explicitly written to do so -
+meaning that data accesses will not be optimized across them. So the
+compiler might think it knows what will be in some_data, but the
+spin_lock() call will force it to forget anything it knows. There will be
+no optimization problems with accesses to that data.
+
+If shared_data were declared volatile, the locking would
+still be necessary. But the compiler would also be prevented from
+optimizing access to shared _within_ the critical section,
+when we know that nobody else can be working with it. While the lock is
+held, shared_data is not volatile. This is why Linus says:
+
+ Also, more importantly, "volatile" is on the wrong _part_ of the
+ whole system. In C, it's "data" that is volatile, but that is
+ insane. Data isn't volatile - _accesses_ are volatile. So it may
+ make sense to say "make this particular _access_ be careful", but
+ not "make all accesses to this data use some random strategy".
+
+When dealing with shared data, proper locking makes volatile unnecessary -
+and potentially harmful.
+
+The volatile storage class was originally meant for memory-mapped I/O
+registers. Within the kernel, register accesses, too, should be protected
+by locks, but one also does not want the compiler "optimizing" register
+accesses within a critical section. But, within the kernel, I/O memory
+accesses are always done through accessor functions; accessing I/O memory
+directly through pointers is frowned upon and does not work on all
+architectures. Those accessors are written to prevent unwanted
+optimization, so, once again, volatile is unnecessary.
+
+Another situation where one might be tempted to use volatile is
+when the processor is busy-waiting on the value of a variable. The right
+way to perform a busy wait is:
+
+ while (my_variable != what_i_want)
+ cpu_relax();
+
+The cpu_relax() call can lower CPU power consumption or yield to a
+hyperthreaded twin processor; it also happens to serve as a memory barrier,
+so, once again, volatile is unnecessary. Of course, busy-waiting is
+generally an anti-social act to begin with.
+
+There are still a few rare situations where volatile makes sense in the
+kernel:
+
+ - The above-mentioned accessor functions might use volatile on
+ architectures where direct I/O memory access does work. Essentially,
+ each accessor call becomes a little critical section on its own and
+ ensures that the access happens as expected by the programmer.
+
+ - Inline assembly code which changes memory, but which has no other
+ visible side effects, risks being deleted by GCC. Adding the volatile
+ keyword to asm statements will prevent this removal.
+
+ - The jiffies variable is special in that it can have a different value
+ every time it is referenced, but it can be read without any special
+ locking. So jiffies can be volatile, but the addition of other
+ variables of this type is frowned upon. Jiffies is considered to be a
suggestion: "frowned strongly upon"
+ "stupid legacy" issue in this regard.
+
+For most code, none of the above justifications for volatile
+apply. As a result, the use of volatile is likely to be seen as a
+bug and will bring additional scrutiny to the code. Developers who are
+tempted to use volatile should take a step back and think about
+what they are truly trying to accomplish.
+
Suggested addition :
Patches that remove volatile from current code (provided there's a
good explanation of why the volatile can be removed and how the bug it
was hiding has been dealt with) are a good thing.
Friends don't let friends use volatile.
+NOTES
+-----
+
+[1] http://lwn.net/Articles/233481/
+[2] http://lwn.net/Articles/233482/
+
+CREDITS
+-------
+
+Original impetus and research by Randy Dunlap
+Written by Jonathan Corbet
+Improvements via coments from Satyam Sharma Johannes Stezenbach
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
