Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Nick Piggin <hidden>
Date: 2007-08-17 04:40:35
Also in:
lkml, netdev
Paul Mackerras wrote:
Nick Piggin writes:quoted
Why are people making these undocumented and just plain false assumptions about atomic_t?Well, it has only been false since December 2006. Prior to that atomics *were* volatile on all platforms.
Hmm, although I don't think it has ever been guaranteed by the API documentation (concede documentation is often not treated as the authoritative source here, but for atomic it is actually very good and obviously indispensable as the memory ordering reference).
quoted
If they're using lockless code (ie. which they must be if using atomics), then they actually need to be thinking much harder about memory ordering issues.Indeed. I believe that most uses of atomic_read other than in polling loops or debug printk statements are actually racy. In some cases the race doesn't seem to matter, but I'm sure there are cases where it does.quoted
If that is too much for them, then they can just use locks.Why use locks when you can just sprinkle magic fix-the-races dust (aka atomic_t) over your code? :) :)
I agree with your skepticism of a lot of lockless code. But I think a lot of the more subtle race problems will not be fixed with volatile. The big, dumb infinite loop bugs would be fixed, but they're pretty trivial to debug and even audit for.
quoted
quoted
Precisely. And volatility is a key property of "atomic". Let's please not throw it away.It isn't, though (at least not since i386 and x86-64 don't have it).Conceptually it is, because atomic_t is specifically for variables which are liable to be modified by other CPUs, and volatile _means_ "liable to be changed by mechanisms outside the knowledge of the compiler".
Usually that is the case, yes. But also most of the time we don't care that it has been changed and don't mind it being reordered or eliminated. One of the only places we really care about that at all is for variables that are modified by the *same* CPU.
quoted
_Adding_ it is trivial, and can be done any time. Throwing it away (ie. making the API weaker) is _hard_. So let's not add it withoutWell, in one sense it's not that hard - Linus did it just 8 months ago in commit f9e9dcb3. :)
Well it would have been harder if the documentation also guaranteed that atomic_read/atomic_set was ordered. Or it would have been harder for _me_ to make such a change, anyway ;)
quoted
really good reasons. It most definitely results in worse code generation in practice.0.0008% increase in kernel text size on powerpc according to my measurement. :)
I don't think you're making a bad choice by keeping it volatile on powerpc and waiting for others to shake out more of the bugs. You get to fix everybody else's memory ordering bugs :)
quoted
I don't know why people would assume volatile of atomics. AFAIK, mostBy making something an atomic_t you're saying "other CPUs are going to be modifying this, so treat it specially". It's reasonable to assume that special treatment extends to reading and setting it.
But I don't actually know what that "special treatment" is. Well actually, I do know that operations will never result in a partial modification being exposed. I also know that the operators that do not modify and return are not guaranteed to have any sort of ordering constraints. -- SUSE Labs, Novell Inc.