Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Paul E. McKenney <hidden>
Date: 2007-08-15 14:25:38
Also in:
linux-arch, lkml
On Wed, Aug 15, 2007 at 07:17:29PM +0530, Satyam Sharma wrote:
On Wed, 15 Aug 2007, Stefan Richter wrote:quoted
Satyam Sharma wrote:quoted
On Wed, 15 Aug 2007, Stefan Richter wrote:quoted
Doesn't "atomic WRT all processors" require volatility?No, it definitely doesn't. Why should it? "Atomic w.r.t. all processors" is just your normal, simple "atomicity" for SMP systems (ensure that that object is modified / set / replaced in main memory atomically) and has nothing to do with "volatile" behaviour. "Volatile behaviour" itself isn't consistently defined (at least definitely not consistently implemented in various gcc versions across platforms), but it is /expected/ to mean something like: "ensure that every such access actually goes all the way to memory, and is not re-ordered w.r.t. to other accesses, as far as the compiler can take care of these". The last "as far as compiler can take care" disclaimer comes about due to CPUs doing their own re-ordering nowadays. For example (say on i386):[...]quoted
In (A) the compiler optimized "a = 10;" away, but the actual store of the final value "20" to "a" was still "atomic". (B) and (C) also exhibit "volatile" behaviour apart from the "atomicity". But as others replied, it seems some callers out there depend upon atomic ops exhibiting "volatile" behaviour as well, so that answers my initial question, actually. I haven't looked at the code Paul pointed me at, but I wonder if that "forget(x)" macro would help those cases. I'd wish to avoid the "volatile" primitive, personally.So, looking at load instead of store, understand I correctly that in your opinion int b; b = atomic_read(&a); if (b) do_something_time_consuming(); b = atomic_read(&a); if (b) do_something_more(); should be changed to explicitly forget(&a) after do_something_time_consuming?No, I'd actually prefer something like what Christoph Lameter suggested, i.e. users (such as above) who want "volatile"-like behaviour from atomic ops can use alternative functions. How about something like: #define atomic_read_volatile(v) \ ({ \ forget(&(v)->counter); \ ((v)->counter); \ })
Wouldn't the above "forget" the value, throw it away, then forget that it forgot it, giving non-volatile semantics?
Or possibly, implement these "volatile" atomic ops variants in inline asm like the patch that Sebastian Siewior has submitted on another thread just a while back.
Given that you are advocating a change (please keep in mind that atomic_read() and atomic_set() had volatile semantics on almost all platforms), care to give some example where these historical volatile semantics are causing a problem?
Of course, if we find there are more callers in the kernel who want the volatility behaviour than those who don't care, we can re-define the existing ops to such variants, and re-name the existing definitions to somethine else, say "atomic_read_nonvolatile" for all I care.
Do we really need another set of APIs? Can you give even one example where the pre-existing volatile semantics are causing enough of a problem to justify adding yet more atomic_*() APIs? Thanx, Paul