lkml.org 
[lkml]   [2007]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Subject[PATCH] "volatile considered harmful", take 2
    From
    Date
    Who knew a documentation patch would get so many reviews?  I like it...

    Anyway, here's a new version in which I attempt to respond to all the
    comments that came in. Thanks to everybody for looking it over.

    jon

    Steer developers away from the volatile type.

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

    diff --git a/Documentation/volatile-considered-harmful.txt b/Documentation/volatile-considered-harmful.txt
    new file mode 100644
    index 0000000..e67745f
    --- /dev/null
    +++ b/Documentation/volatile-considered-harmful.txt
    @@ -0,0 +1,118 @@
    +Why the "volatile" type class should not be used
    +------------------------------------------------
    +
    +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. In other words, they have been known to treat volatile types
    +as a sort of easy atomic variable, which they are not. The use of volatile in
    +kernel code is almost never correct; this document describes why.
    +
    +The key point to understand 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 shared data structures against
    +unwanted concurrent access, which is very much a different task. As it
    +happens, once the critical sections are properly implemented, the compiler
    +optimization issues which volatile was added to prevent will have been
    +taken care of in a more efficient way.
    +
    +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, since it acts as a memory barrier, 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_data _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. 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 strongly frowned upon. Jiffies is considered
    + to be a "stupid legacy" issue in this regard.
    +
    + - Pointers to data structures in coherent memory which might be modified
    + by I/O devices can, sometimes, legitimately be volatile. A ring buffer
    + used by a network adapter, where that adapter changes pointers to
    + indicate which descriptors have been processed, is an example of this
    + type of situation.
    +
    +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.
    +
    +Patches to remove volatile variables are generally welcome - as long as
    +they come with a justification which shows that the concurrency issues have
    +been properly thought through.
    +
    +
    +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, Heikki Orsila, and H. Peter Anvin
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2007-05-10 22:23    [W:0.047 / U:0.988 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site