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

On Wed, May 09, 2007 at 03:05:44PM -0600, Jonathan Corbet wrote:
quoted text
> 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.

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

quoted text
> +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.  

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 
barriers tools. These tools make volatile useless."

quoted text
> +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.

The previous suggestion takes care of explaining that, remove this.

quoted text
> 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.

Ok

quoted text
> +The spinlock
> +primitives act as memory barriers - they are explicitly written to do so -
> +meaning that data accesses will not be optimized across them.

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

quoted text
> 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.

I would say it directly:

"Spinlock will force an implicit memory barrier, thus preventing 
optimizations to data access."

quoted text
> +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.

ok

quoted text
>   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".

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
-
unsubscribe notice
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/