Re: [PATCH] "volatile considered harmful" document

Previous thread: [PATCH 4/6] UML - IRQ stacks by Jeff Dike on Wednesday, May 9, 2007 - 1:27 pm. (1 message)

Next thread: 2.6.21-git10/11: files getting truncated on xfs? or maybe an nlink problem? by Jeremy Fitzhardinge on Wednesday, May 9, 2007 - 2:09 pm. (29 messages)
From: Jonathan Corbet
Date: Wednesday, May 9, 2007 - 2:05 pm

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?

Comments welcome,

jon

Tell kernel developers why they shouldn't use volatile.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>

diff -ruNp linux-2.6/Documentation/volatile.txt volatile/Documentation/volatile.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
+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 ...
From: Heikki Orsila
Date: Wednesday, May 9, 2007 - 3:05 pm

Imo, the best style to relay information is by directly stating facts.
I'm going to try to suggest some improvements on this..


I think previous text is unnecessary. Just tell the reasons why 
volatile is bad and what should be used instead, no need to quote 

Again, I would write this in non-personal way:

"Volatile is often used to prevent optimization, but it is not the
behavior that is actually wanted. Access to data must be protected and 
handled with kernel provided synchronization, mutual exclusion and 



"Spinlock primitives will serialise execution of code regions, and 
memory barrier functions must be used inside spinlocks to force 

I would say it directly:

"Spinlock will force an implicit memory barrier, thus preventing 


Unnecessary quoting, imo. Tell the same information directly without 
personifying the statement.

-- 
Heikki Orsila			Barbie's law:
heikki.orsila@iki.fi		"Math is hard, let's go shopping!"
http://www.iki.fi/shd
-

From: Randy Dunlap
Date: Wednesday, May 9, 2007 - 3:19 pm

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-

From: Scott Preece
Date: Wednesday, May 9, 2007 - 3:35 pm

---

I think the size of this file is OK as a separate file, but much too
big for CodingStyle. It would be good to also write a pithy paragraph
to go into CodingStyle to convey the rule and point at this file.

scott
-

From: Jesper Juhl
Date: Wednesday, May 9, 2007 - 2:45 pm

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


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


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.



-- 
Jesper Juhl <jesper.juhl@gmail.com>
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
-

From: Satyam Sharma
Date: Wednesday, May 9, 2007 - 3:31 pm

Yes, I feel it's important to have a filename that strongly states the
dim view its contents take on its subject (the "volatile" type
qualifier in this case). Sort of like stable-api-nonsense.txt vs

I'm not so sure here. Why would any C programmer (worth his weight in
salt) think that volatile objects are automatically _atomic_? At
worst, the mistake someone might make would be to _implement_ locking
primitives using volatile. "that the variable could be changed outside
of the current thread of execution" sounds sufficient to me, and after

Yes, this addition would be nice :-)
-

From: Jesper Juhl
Date: Wednesday, May 9, 2007 - 3:47 pm

I honestly don't really know, but I've encountered that confusion a
few times. Both from friends who (for some reason) believed that and
from documents on the web that implied it, aparently it's a common
confusion - a few examples:

    http://lists.freebsd.org/pipermail/freebsd-perl/2004-June/000124.html
        "... volatile (atomic) fixes the problem. ..."

    http://blogs.msdn.com/ricom/archive/2006/04/28/586406.aspx
        "That's the point of the volatile keyword. It makes sure that
the line "dict = d;" is atomic."

    http://forum.java.sun.com/thread.jspa?threadID=5126877&start=0
        "A volatile variable is also guaranteed to be read or written
as an atomic operation ..."  (yes, this link talks about Java, which I
don't know, but if java volatile means atomic, that might explain why
some people assume the same for C).

In any case, it's not an uncommon belief, so I just thought it made


-- 
Jesper Juhl <jesper.juhl@gmail.com>
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
-

From: Satyam Sharma
Date: Wednesday, May 9, 2007 - 4:20 pm

Perl / Microsoft / Java programmers are probably not worth their
weight in salt anyway :-)

I'm not an expert in any of the above platforms either, so don't know
if the semantics of "volatile" in those other languages are different
from that in C -- and this document clearly applies to only the kernel
(and thus C). But if this volatile == atomic disease is indeed common
among _C_ programmers too, then your suggested addition would make
sense.
-

From: Giacomo A. Catenazzi
Date: Thursday, May 10, 2007 - 9:14 am

I don't think it deserves to be added in documentation, but just for
reference: in userspace "volatile" is needed in signals (posix mandates
some variables to be volatile, as API, not as funtionality). I don't
know if this was also on the original signal handling.

Anyway user space APIs are not kernel problem ;-)

ciao
	cate
-

From: H. Peter Anvin
Date: Thursday, May 10, 2007 - 12:35 pm

I have found one use of volatile which I consider legitimate: pointers
to data structures read or written by I/O devices in coherent memory
(typically pci_coherent memory.)  This is local to device drivers, but
as far as I can tell, the use of volatile here is legitimate, although
arguably it will be redundant in > 90% of all cases due to the
incidental presence of other memory barriers.

In Ethernet drivers, for example, it is common for the network card to
maintain a pointer in host memory the the latest descriptor written; you
will generally have a loop of the form:

	while ((this_pointer = *pointer_ptr) > my_last_pointer) {
		for (pkt = my_last_pointer; pkt < this_pointer; pkt++)
			receeive_packet(pkt);
		my_last_pointer = this_pointer;
	}

pointer_p can then be a volatile pointer into said coherent memory.

	-hpa

		
-

From: Alan Cox
Date: Thursday, May 10, 2007 - 3:00 pm

True but you can happily use rmb/wmb for this which are clearer and fit
the rest of the Linux model better.

Alan
-

From: Bill Davidsen
Date: Thursday, May 10, 2007 - 2:54 pm

It would seem that any variable which is (a) subject to change by other 
threads or hardware, and (b) the value of which is going to be used 
without writing the variable, would be a valid use for volatile.

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot
-

From: Thomas De Schampheleire
Date: Wednesday, May 16, 2007 - 2:30 am

Just a small spelling mistake: coments should be comments.
-

Previous thread: [PATCH 4/6] UML - IRQ stacks by Jeff Dike on Wednesday, May 9, 2007 - 1:27 pm. (1 message)

Next thread: 2.6.21-git10/11: files getting truncated on xfs? or maybe an nlink problem? by Jeremy Fitzhardinge on Wednesday, May 9, 2007 - 2