Linux relies on unsigned long to behave like atomic for read/write.
diff --git a/Documentation/atomic_ops.txt b/Documentation/atomic_ops.txt
index 4ef2450..0a7d180 100644
--- a/Documentation/atomic_ops.txt
+++ b/Documentation/atomic_ops.txt
@@ -21,6 +21,9 @@ local_t is very similar to atomic_t. If the counter is per CPU and only
updated by one CPU, local_t is probably more appropriate. Please see
Documentation/local_ops.txt for the semantics of local_t.
+unsigned long can be used instead of atomic_t, if all you
+need is atomic setting and atomic reading.
+
The first operations to implement for atomic_t's are the initializers and
plain reads.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sat, 3 Jan 2009 13:44:00 +0100
Pavel Machek <[email protected]> wrote:
> Linux relies on unsigned long to behave like atomic for read/write.
Actually it isn't that simple and this advice shouldn't be given IMHO.
unsigned long is not the same as atomic in several respects including
ordering and caching of the result.
Alan
On Sat 2009-01-03 20:19:55, Alan Cox wrote:
> On Sat, 3 Jan 2009 13:44:00 +0100
> Pavel Machek <[email protected]> wrote:
>
> > Linux relies on unsigned long to behave like atomic for read/write.
>
> Actually it isn't that simple and this advice shouldn't be given IMHO.
>
> unsigned long is not the same as atomic in several respects including
> ordering and caching of the result.
Ok... I keep seeing patches using int/long instead of atomic and
claiming that it is okay.
If it is okay and linux relies on it, it should be documented.
If it is not okay, I guess we should document it, too -- it seems to
be common mistake.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> If it is okay and linux relies on it, it should be documented.
>
> If it is not okay, I guess we should document it, too -- it seems to
> be common mistake.
A lot of old code did it knowing it was under the BKL, outside of the BKL
its a very bad idea. There were lots of them in the tty layer and I don't
doubt there are some left I missed too 8(
On Sat 2009-01-03 20:30:44, Alan Cox wrote:
> > If it is okay and linux relies on it, it should be documented.
> >
> > If it is not okay, I guess we should document it, too -- it seems to
> > be common mistake.
>
> A lot of old code did it knowing it was under the BKL, outside of the BKL
> its a very bad idea. There were lots of them in the tty layer and I don't
> doubt there are some left I missed too 8(
I have seen this in new code (some LED driver last time), definitely
no BKL.
Is there concrete architecture where it breaks? I'd expect i386/x86-64
to be safe, and pretty much everyone to be safe as long as that long
is aligned.... or that was the result of arch-maintainers
discussion...
I'd really like to document if it is right or not, so that I can point
people to documentation...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Sat, 3 Jan 2009, Pavel Machek wrote:
> On Sat 2009-01-03 20:30:44, Alan Cox wrote:
>>> If it is okay and linux relies on it, it should be documented.
>>>
>>> If it is not okay, I guess we should document it, too -- it seems to
>>> be common mistake.
>>
>> A lot of old code did it knowing it was under the BKL, outside of the BKL
>> its a very bad idea. There were lots of them in the tty layer and I don't
>> doubt there are some left I missed too 8(
>
> I have seen this in new code (some LED driver last time), definitely
> no BKL.
>
> Is there concrete architecture where it breaks? I'd expect i386/x86-64
> to be safe, and pretty much everyone to be safe as long as that long
> is aligned.... or that was the result of arch-maintainers
> discussion...
>
> I'd really like to document if it is right or not, so that I can point
> people to documentation...
you may want to take a look at the new C/C++/POSIX standards (some just
standardized, some still in development), they explicitly address this
area.
David Lang
> Is there concrete architecture where it breaks? I'd expect i386/x86-64
> to be safe, and pretty much everyone to be safe as long as that long
> is aligned.... or that was the result of arch-maintainers
> discussion...
It'll break on x86 if gcc decides to cache the value and you don't have
explicit barriers. If the long is not aligned it's not safe on x86 at all.
> I'd really like to document if it is right or not, so that I can point
> people to documentation...
We should always tell people to use atomic/set_bit etc. There *are* cases
you can get away with it but it is far far better that the default is the
safe one because most driver writers do not have a detailed knowledge of
gcc code generation, processor quirks and barriers. If in a specific case
its a performance hit then its worth optimising that case.
Alan
Pavel Machek wrote:
> On Sat 2009-01-03 20:30:44, Alan Cox wrote:
>>> If it is okay and linux relies on it, it should be documented.
>>>
>>> If it is not okay, I guess we should document it, too -- it seems to
>>> be common mistake.
>> A lot of old code did it knowing it was under the BKL, outside of the BKL
>> its a very bad idea. There were lots of them in the tty layer and I don't
>> doubt there are some left I missed too 8(
>
> I have seen this in new code (some LED driver last time), definitely
> no BKL.
>
> Is there concrete architecture where it breaks? I'd expect i386/x86-64
> to be safe, and pretty much everyone to be safe as long as that long
> is aligned.... or that was the result of arch-maintainers
> discussion...
>
> I'd really like to document if it is right or not, so that I can point
> people to documentation...
> Pavel
If you look at the atomic implementation on x86 all it does is assign
and read the internal int variable directly for atomic_set and
atomic_read, so I suppose it would be OK to just use a normal variable
in that case.. but then there's no performance hit so you might as well
use atomic_t anyway. On some architectures like arm and sparc there is
some magic involved in atomic_set and/or atomic_read (but those may just
be to guard against other concurrent atomic ops, I'm not sure).
Certainly unless the code is really performance critical there is no
point messing around, just use an atomic if it needs to be accessed
without locking. Note that memory barriers may be an issue as well..
On Sat 2009-01-03 20:19:55, Alan Cox wrote:
> On Sat, 3 Jan 2009 13:44:00 +0100
> Pavel Machek <[email protected]> wrote:
>
> > Linux relies on unsigned long to behave like atomic for read/write.
>
> Actually it isn't that simple and this advice shouldn't be given IMHO.
>
> unsigned long is not the same as atomic in several respects including
> ordering and caching of the result.
I'm confused. I remember distinctly being told, when merging the
Gigaset driver, that reading and writing of "pointer sized objects"
(I specifically remember that term) was assumed to be atomic across
large parts of the kernel anyway, that locking around a single read
or write of such an item was therefore pointless, and that "atomic_t"
only made sense if I wanted to use atomic_inc or atomic_dec on them.
The patches I submitted in following that advice, specifically commit
4d1ff582246de67b15e3cd2427a39875943ae895 "gigaset: remove pointless
locking" and 9d4bee2b9de9e30057a860d2d6794f874caffc5e "gigaset: atomic
cleanup", were confirmed to do the right thing and merged without any
objection.
What am I missing?
Thanks,
Tilman
--
Tilman Schmidt E-Mail: [email protected]
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Unge?ffnet mindestens haltbar bis: (siehe R?ckseite)
On Sunday 04 January 2009 10:14:19 Alan Cox wrote:
> > Is there concrete architecture where it breaks? I'd expect i386/x86-64
> > to be safe, and pretty much everyone to be safe as long as that long
> > is aligned.... or that was the result of arch-maintainers
> > discussion...
>
> It'll break on x86 if gcc decides to cache the value and you don't have
> explicit barriers.
Same as atomic_read on x86.
> If the long is not aligned it's not safe on x86 at all.
Same as atomic_t.
AFAIK, Linux requires aligned loads and stores to int and long (and pointer)
to be atomic.
Pretty much everywhere that uses RCU for example does so using atomic pointer
loads and stores. The nastiest issue IMO actually is reloading the value
through the pointer even if it isn't explicitly dereferenced. RCU gets this
right with ACCESS_ONCE. Probably a lot of code using basic types does not.
x86 atomic_read maybe should be using ACCESS_ONCE too...
> Pretty much everywhere that uses RCU for example does so using atomic pointer
> loads and stores. The nastiest issue IMO actually is reloading the value
> through the pointer even if it isn't explicitly dereferenced. RCU gets this
> right with ACCESS_ONCE. Probably a lot of code using basic types does not.
> x86 atomic_read maybe should be using ACCESS_ONCE too...
I'm pretty sure it should. gcc makes no guarantees about not being clever
with accesses.
Alan
On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > Pretty much everywhere that uses RCU for example does so using atomic
> > pointer loads and stores. The nastiest issue IMO actually is reloading
> > the value through the pointer even if it isn't explicitly dereferenced.
> > RCU gets this right with ACCESS_ONCE. Probably a lot of code using basic
> > types does not. x86 atomic_read maybe should be using ACCESS_ONCE too...
>
> I'm pretty sure it should. gcc makes no guarantees about not being clever
> with accesses.
Arguably it should. I don't know what the concurrent C standard looks like,
but prohibiting reloads of potentially concurrently modified memory when
there is no explicit pointer dereference is the natural complement to
prohibiting stores to potentially concurrently read memory when there is
no explicit store (which I think is begrudgingly agreed to be a problem).
http://lkml.org/lkml/2007/10/24/673
I think I would like to see multiple reloads to local variables prohibited,
to avoid potential really subtle problems... But if ACCESS_ONCE is here to
stay, then I do think that atomic_read etc should use it.
On Mon, Jan 05, 2009 at 11:00:24PM +1100, Nick Piggin wrote:
> On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > > Pretty much everywhere that uses RCU for example does so using atomic
> > > pointer loads and stores. The nastiest issue IMO actually is reloading
> > > the value through the pointer even if it isn't explicitly dereferenced.
> > > RCU gets this right with ACCESS_ONCE. Probably a lot of code using basic
> > > types does not. x86 atomic_read maybe should be using ACCESS_ONCE too...
> >
> > I'm pretty sure it should. gcc makes no guarantees about not being clever
> > with accesses.
>
> Arguably it should. I don't know what the concurrent C standard looks like,
> but prohibiting reloads of potentially concurrently modified memory when
> there is no explicit pointer dereference is the natural complement to
> prohibiting stores to potentially concurrently read memory when there is
> no explicit store (which I think is begrudgingly agreed to be a problem).
>
> http://lkml.org/lkml/2007/10/24/673
>
> I think I would like to see multiple reloads to local variables prohibited,
> to avoid potential really subtle problems... But if ACCESS_ONCE is here to
> stay, then I do think that atomic_read etc should use it.
The concurrency stuff in c++0x still permits the compiler to have its
way with loads and stores to normal variables, but provides an "atomic"
type that must be loaded and stored as specified in the program.
The issue with ACCESS_ONCE() is that gcc doesn't do any optimizations on
volatile accesses, even the obvious ones. Speaking of which, the gcc
guys kicked out my bug 33102, which was complaining about this
situation. :-/
Thanx, Paul
On Tuesday 06 January 2009 03:05:01 Paul E. McKenney wrote:
> On Mon, Jan 05, 2009 at 11:00:24PM +1100, Nick Piggin wrote:
> > On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > > > Pretty much everywhere that uses RCU for example does so using atomic
> > > > pointer loads and stores. The nastiest issue IMO actually is
> > > > reloading the value through the pointer even if it isn't explicitly
> > > > dereferenced. RCU gets this right with ACCESS_ONCE. Probably a lot of
> > > > code using basic types does not. x86 atomic_read maybe should be
> > > > using ACCESS_ONCE too...
> > >
> > > I'm pretty sure it should. gcc makes no guarantees about not being
> > > clever with accesses.
> >
> > Arguably it should. I don't know what the concurrent C standard looks
> > like, but prohibiting reloads of potentially concurrently modified memory
> > when there is no explicit pointer dereference is the natural complement
> > to prohibiting stores to potentially concurrently read memory when there
> > is no explicit store (which I think is begrudgingly agreed to be a
> > problem).
> >
> > http://lkml.org/lkml/2007/10/24/673
> >
> > I think I would like to see multiple reloads to local variables
> > prohibited, to avoid potential really subtle problems... But if
> > ACCESS_ONCE is here to stay, then I do think that atomic_read etc should
> > use it.
>
> The concurrency stuff in c++0x still permits the compiler to have its
> way with loads and stores to normal variables, but provides an "atomic"
> type that must be loaded and stored as specified in the program.
So:
if (trylock())
locked = 1;
...
if (locked)
*var = blah;
...
if (locked)
unlock();
So the second part can still be transformed into a predicated calculation
of blah, then an unconditional store to *var?
> The issue with ACCESS_ONCE() is that gcc doesn't do any optimizations on
> volatile accesses, even the obvious ones. Speaking of which, the gcc
> guys kicked out my bug 33102, which was complaining about this
> situation. :-/
Hmm. It's still quite annoying even to have to switch everything to the
atomic type. I guarantee there will be bugs in Linux caused by the
compiler reloading pointers/longs/ints to access local variables...
On Tue, Jan 06, 2009 at 03:25:12AM +1100, Nick Piggin wrote:
> On Tuesday 06 January 2009 03:05:01 Paul E. McKenney wrote:
> > On Mon, Jan 05, 2009 at 11:00:24PM +1100, Nick Piggin wrote:
> > > On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > > > > Pretty much everywhere that uses RCU for example does so using atomic
> > > > > pointer loads and stores. The nastiest issue IMO actually is
> > > > > reloading the value through the pointer even if it isn't explicitly
> > > > > dereferenced. RCU gets this right with ACCESS_ONCE. Probably a lot of
> > > > > code using basic types does not. x86 atomic_read maybe should be
> > > > > using ACCESS_ONCE too...
> > > >
> > > > I'm pretty sure it should. gcc makes no guarantees about not being
> > > > clever with accesses.
> > >
> > > Arguably it should. I don't know what the concurrent C standard looks
> > > like, but prohibiting reloads of potentially concurrently modified memory
> > > when there is no explicit pointer dereference is the natural complement
> > > to prohibiting stores to potentially concurrently read memory when there
> > > is no explicit store (which I think is begrudgingly agreed to be a
> > > problem).
> > >
> > > http://lkml.org/lkml/2007/10/24/673
> > >
> > > I think I would like to see multiple reloads to local variables
> > > prohibited, to avoid potential really subtle problems... But if
> > > ACCESS_ONCE is here to stay, then I do think that atomic_read etc should
> > > use it.
> >
> > The concurrency stuff in c++0x still permits the compiler to have its
> > way with loads and stores to normal variables, but provides an "atomic"
> > type that must be loaded and stored as specified in the program.
>
> So:
> if (trylock())
> locked = 1;
> ...
> if (locked)
> *var = blah;
> ...
> if (locked)
> unlock();
>
> So the second part can still be transformed into a predicated calculation
> of blah, then an unconditional store to *var?
Assuming that "var" is an ordinary variable...
If you are asking whether the compiler guys believe that they are within
their rights to do a store to *var and then store the old value to *var
if !locked, the unfortunate answer is "yes". Hence ACCESS_ONCE().
> > The issue with ACCESS_ONCE() is that gcc doesn't do any optimizations on
> > volatile accesses, even the obvious ones. Speaking of which, the gcc
> > guys kicked out my bug 33102, which was complaining about this
> > situation. :-/
>
> Hmm. It's still quite annoying even to have to switch everything to the
> atomic type. I guarantee there will be bugs in Linux caused by the
> compiler reloading pointers/longs/ints to access local variables...
Another approach would be to change ACCESS_ONCE() to use the atomic
types. Then we have a bigger hammer with which to beat the gcc guys
about optimizations. We can hope, anyway...
Thanx, Paul
On Tuesday 06 January 2009 04:30:04 Paul E. McKenney wrote:
> On Tue, Jan 06, 2009 at 03:25:12AM +1100, Nick Piggin wrote:
> > On Tuesday 06 January 2009 03:05:01 Paul E. McKenney wrote:
> > > On Mon, Jan 05, 2009 at 11:00:24PM +1100, Nick Piggin wrote:
> > > > On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > > > > > Pretty much everywhere that uses RCU for example does so using
> > > > > > atomic pointer loads and stores. The nastiest issue IMO actually
> > > > > > is reloading the value through the pointer even if it isn't
> > > > > > explicitly dereferenced. RCU gets this right with ACCESS_ONCE.
> > > > > > Probably a lot of code using basic types does not. x86
> > > > > > atomic_read maybe should be using ACCESS_ONCE too...
> > > > >
> > > > > I'm pretty sure it should. gcc makes no guarantees about not being
> > > > > clever with accesses.
> > > >
> > > > Arguably it should. I don't know what the concurrent C standard looks
> > > > like, but prohibiting reloads of potentially concurrently modified
> > > > memory when there is no explicit pointer dereference is the natural
> > > > complement to prohibiting stores to potentially concurrently read
> > > > memory when there is no explicit store (which I think is begrudgingly
> > > > agreed to be a problem).
> > > >
> > > > http://lkml.org/lkml/2007/10/24/673
> > > >
> > > > I think I would like to see multiple reloads to local variables
> > > > prohibited, to avoid potential really subtle problems... But if
> > > > ACCESS_ONCE is here to stay, then I do think that atomic_read etc
> > > > should use it.
> > >
> > > The concurrency stuff in c++0x still permits the compiler to have its
> > > way with loads and stores to normal variables, but provides an "atomic"
> > > type that must be loaded and stored as specified in the program.
> >
> > So:
> > if (trylock())
> > locked = 1;
> > ...
> > if (locked)
> > *var = blah;
> > ...
> > if (locked)
> > unlock();
> >
> > So the second part can still be transformed into a predicated calculation
> > of blah, then an unconditional store to *var?
>
> Assuming that "var" is an ordinary variable...
>
> If you are asking whether the compiler guys believe that they are within
> their rights to do a store to *var and then store the old value to *var
> if !locked, the unfortunate answer is "yes". Hence ACCESS_ONCE().
Right. Part of the problem is that it might not be so clear. Each part of
the above sequence might be somewhat hidden by different call chains, gotos,
etc. *var = blah may not be that simple in form but actually be a call to
some helper function (which ends up being inlined, etc).
if (!trylock())
return;
*var = blah;
unlock();
This is basically the same sequence, isn't it? var definitely doesn't need
to be atomic. It actually shouldn't have to be subject to any compiler
constraints within the critical section in order for the compiler to
generate an optimal critical section.
> > > The issue with ACCESS_ONCE() is that gcc doesn't do any optimizations
> > > on volatile accesses, even the obvious ones. Speaking of which, the
> > > gcc guys kicked out my bug 33102, which was complaining about this
> > > situation. :-/
> >
> > Hmm. It's still quite annoying even to have to switch everything to the
> > atomic type. I guarantee there will be bugs in Linux caused by the
> > compiler reloading pointers/longs/ints to access local variables...
>
> Another approach would be to change ACCESS_ONCE() to use the atomic
> types. Then we have a bigger hammer with which to beat the gcc guys
> about optimizations. We can hope, anyway...
Well... I'm less concerned about performance as correctness and difficulty
of programming this model.
On Tue, Jan 06, 2009 at 05:25:30AM +1100, Nick Piggin wrote:
> On Tuesday 06 January 2009 04:30:04 Paul E. McKenney wrote:
> > On Tue, Jan 06, 2009 at 03:25:12AM +1100, Nick Piggin wrote:
> > > On Tuesday 06 January 2009 03:05:01 Paul E. McKenney wrote:
> > > > On Mon, Jan 05, 2009 at 11:00:24PM +1100, Nick Piggin wrote:
> > > > > On Monday 05 January 2009 22:23:50 Alan Cox wrote:
> > > > > > > Pretty much everywhere that uses RCU for example does so using
> > > > > > > atomic pointer loads and stores. The nastiest issue IMO actually
> > > > > > > is reloading the value through the pointer even if it isn't
> > > > > > > explicitly dereferenced. RCU gets this right with ACCESS_ONCE.
> > > > > > > Probably a lot of code using basic types does not. x86
> > > > > > > atomic_read maybe should be using ACCESS_ONCE too...
> > > > > >
> > > > > > I'm pretty sure it should. gcc makes no guarantees about not being
> > > > > > clever with accesses.
> > > > >
> > > > > Arguably it should. I don't know what the concurrent C standard looks
> > > > > like, but prohibiting reloads of potentially concurrently modified
> > > > > memory when there is no explicit pointer dereference is the natural
> > > > > complement to prohibiting stores to potentially concurrently read
> > > > > memory when there is no explicit store (which I think is begrudgingly
> > > > > agreed to be a problem).
> > > > >
> > > > > http://lkml.org/lkml/2007/10/24/673
> > > > >
> > > > > I think I would like to see multiple reloads to local variables
> > > > > prohibited, to avoid potential really subtle problems... But if
> > > > > ACCESS_ONCE is here to stay, then I do think that atomic_read etc
> > > > > should use it.
> > > >
> > > > The concurrency stuff in c++0x still permits the compiler to have its
> > > > way with loads and stores to normal variables, but provides an "atomic"
> > > > type that must be loaded and stored as specified in the program.
> > >
> > > So:
> > > if (trylock())
> > > locked = 1;
> > > ...
> > > if (locked)
> > > *var = blah;
> > > ...
> > > if (locked)
> > > unlock();
> > >
> > > So the second part can still be transformed into a predicated calculation
> > > of blah, then an unconditional store to *var?
> >
> > Assuming that "var" is an ordinary variable...
> >
> > If you are asking whether the compiler guys believe that they are within
> > their rights to do a store to *var and then store the old value to *var
> > if !locked, the unfortunate answer is "yes". Hence ACCESS_ONCE().
>
> Right. Part of the problem is that it might not be so clear. Each part of
> the above sequence might be somewhat hidden by different call chains, gotos,
> etc. *var = blah may not be that simple in form but actually be a call to
> some helper function (which ends up being inlined, etc).
>
> if (!trylock())
> return;
>
> *var = blah;
>
> unlock();
>
> This is basically the same sequence, isn't it? var definitely doesn't need
> to be atomic. It actually shouldn't have to be subject to any compiler
> constraints within the critical section in order for the compiler to
> generate an optimal critical section.
Agreed, in this case, the compiler should be permitted to optimize to
its heart's content. Which is one of the reasons that we will probably
eventually have to explicitly mark the cases that the compiler cannot be
permitted to optimize -- preferably with such marking buried in some
other primitive!
> > > > The issue with ACCESS_ONCE() is that gcc doesn't do any optimizations
> > > > on volatile accesses, even the obvious ones. Speaking of which, the
> > > > gcc guys kicked out my bug 33102, which was complaining about this
> > > > situation. :-/
> > >
> > > Hmm. It's still quite annoying even to have to switch everything to the
> > > atomic type. I guarantee there will be bugs in Linux caused by the
> > > compiler reloading pointers/longs/ints to access local variables...
> >
> > Another approach would be to change ACCESS_ONCE() to use the atomic
> > types. Then we have a bigger hammer with which to beat the gcc guys
> > about optimizations. We can hope, anyway...
>
> Well... I'm less concerned about performance as correctness and difficulty
> of programming this model.
Indeed! And the compiler optimizations certainly can degrade
correctness more than a bit at times. :-/
Thanx, Paul