* atomics: document that linux expects certain atomic behaviour from unsigned long
@ 2009-01-03 12:44 Pavel Machek
2009-01-03 20:19 ` Alan Cox
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2009-01-03 12:44 UTC (permalink / raw)
To: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 12:44 atomics: document that linux expects certain atomic behaviour from unsigned long Pavel Machek
@ 2009-01-03 20:19 ` Alan Cox
2009-01-03 20:27 ` Pavel Machek
0 siblings, 1 reply; 17+ messages in thread
From: Alan Cox @ 2009-01-03 20:19 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
On Sat, 3 Jan 2009 13:44:00 +0100
Pavel Machek <pavel@suse.cz> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:19 ` Alan Cox
@ 2009-01-03 20:27 ` Pavel Machek
2009-01-03 20:30 ` Alan Cox
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2009-01-03 20:27 UTC (permalink / raw)
To: Alan Cox; +Cc: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
On Sat 2009-01-03 20:19:55, Alan Cox wrote:
> On Sat, 3 Jan 2009 13:44:00 +0100
> Pavel Machek <pavel@suse.cz> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:27 ` Pavel Machek
@ 2009-01-03 20:30 ` Alan Cox
2009-01-03 20:56 ` Pavel Machek
2009-01-04 18:05 ` Tilman Schmidt
0 siblings, 2 replies; 17+ messages in thread
From: Alan Cox @ 2009-01-03 20:30 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
> 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(
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:30 ` Alan Cox
@ 2009-01-03 20:56 ` Pavel Machek
2009-01-03 23:01 ` david
` (2 more replies)
2009-01-04 18:05 ` Tilman Schmidt
1 sibling, 3 replies; 17+ messages in thread
From: Pavel Machek @ 2009-01-03 20:56 UTC (permalink / raw)
To: Alan Cox; +Cc: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:56 ` Pavel Machek
@ 2009-01-03 23:01 ` david
2009-01-03 23:14 ` Alan Cox
2009-01-03 23:53 ` Robert Hancock
2 siblings, 0 replies; 17+ messages in thread
From: david @ 2009-01-03 23:01 UTC (permalink / raw)
To: Pavel Machek
Cc: Alan Cox, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:56 ` Pavel Machek
2009-01-03 23:01 ` david
@ 2009-01-03 23:14 ` Alan Cox
2009-01-05 10:54 ` Nick Piggin
2009-01-03 23:53 ` Robert Hancock
2 siblings, 1 reply; 17+ messages in thread
From: Alan Cox @ 2009-01-03 23:14 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list, Andrew Morton, mtk.manpages, rdunlap, linux-doc
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:56 ` Pavel Machek
2009-01-03 23:01 ` david
2009-01-03 23:14 ` Alan Cox
@ 2009-01-03 23:53 ` Robert Hancock
2 siblings, 0 replies; 17+ messages in thread
From: Robert Hancock @ 2009-01-03 23:53 UTC (permalink / raw)
To: Pavel Machek
Cc: Alan Cox, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
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..
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 20:30 ` Alan Cox
2009-01-03 20:56 ` Pavel Machek
@ 2009-01-04 18:05 ` Tilman Schmidt
1 sibling, 0 replies; 17+ messages in thread
From: Tilman Schmidt @ 2009-01-04 18:05 UTC (permalink / raw)
To: Alan Cox
Cc: Pavel Machek, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
On Sat 2009-01-03 20:19:55, Alan Cox wrote:
> On Sat, 3 Jan 2009 13:44:00 +0100
> Pavel Machek <pavel@suse.cz> 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: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-03 23:14 ` Alan Cox
@ 2009-01-05 10:54 ` Nick Piggin
2009-01-05 11:23 ` Alan Cox
0 siblings, 1 reply; 17+ messages in thread
From: Nick Piggin @ 2009-01-05 10:54 UTC (permalink / raw)
To: Alan Cox
Cc: Pavel Machek, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
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...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 10:54 ` Nick Piggin
@ 2009-01-05 11:23 ` Alan Cox
2009-01-05 12:00 ` Nick Piggin
0 siblings, 1 reply; 17+ messages in thread
From: Alan Cox @ 2009-01-05 11:23 UTC (permalink / raw)
To: Nick Piggin
Cc: Pavel Machek, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 11:23 ` Alan Cox
@ 2009-01-05 12:00 ` Nick Piggin
2009-01-05 16:05 ` Paul E. McKenney
0 siblings, 1 reply; 17+ messages in thread
From: Nick Piggin @ 2009-01-05 12:00 UTC (permalink / raw)
To: Alan Cox, paulmck
Cc: Pavel Machek, kernel list, Andrew Morton, mtk.manpages, rdunlap,
linux-doc
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 12:00 ` Nick Piggin
@ 2009-01-05 16:05 ` Paul E. McKenney
2009-01-05 16:25 ` Nick Piggin
0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2009-01-05 16:05 UTC (permalink / raw)
To: Nick Piggin
Cc: Alan Cox, Pavel Machek, kernel list, Andrew Morton, mtk.manpages,
rdunlap, linux-doc, segher, rth
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 16:05 ` Paul E. McKenney
@ 2009-01-05 16:25 ` Nick Piggin
2009-01-05 17:30 ` Paul E. McKenney
0 siblings, 1 reply; 17+ messages in thread
From: Nick Piggin @ 2009-01-05 16:25 UTC (permalink / raw)
To: paulmck
Cc: Alan Cox, Pavel Machek, kernel list, Andrew Morton, mtk.manpages,
rdunlap, linux-doc, segher, rth
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...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 16:25 ` Nick Piggin
@ 2009-01-05 17:30 ` Paul E. McKenney
2009-01-05 18:25 ` Nick Piggin
0 siblings, 1 reply; 17+ messages in thread
From: Paul E. McKenney @ 2009-01-05 17:30 UTC (permalink / raw)
To: Nick Piggin
Cc: Alan Cox, Pavel Machek, kernel list, Andrew Morton, mtk.manpages,
rdunlap, linux-doc, segher, rth
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 17:30 ` Paul E. McKenney
@ 2009-01-05 18:25 ` Nick Piggin
2009-01-05 22:01 ` Paul E. McKenney
0 siblings, 1 reply; 17+ messages in thread
From: Nick Piggin @ 2009-01-05 18:25 UTC (permalink / raw)
To: paulmck
Cc: Alan Cox, Pavel Machek, kernel list, Andrew Morton, mtk.manpages,
rdunlap, linux-doc, segher, rth
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: atomics: document that linux expects certain atomic behaviour from unsigned long
2009-01-05 18:25 ` Nick Piggin
@ 2009-01-05 22:01 ` Paul E. McKenney
0 siblings, 0 replies; 17+ messages in thread
From: Paul E. McKenney @ 2009-01-05 22:01 UTC (permalink / raw)
To: Nick Piggin
Cc: Alan Cox, Pavel Machek, kernel list, Andrew Morton, mtk.manpages,
rdunlap, linux-doc, segher, rth
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2009-01-05 22:01 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-03 12:44 atomics: document that linux expects certain atomic behaviour from unsigned long Pavel Machek
2009-01-03 20:19 ` Alan Cox
2009-01-03 20:27 ` Pavel Machek
2009-01-03 20:30 ` Alan Cox
2009-01-03 20:56 ` Pavel Machek
2009-01-03 23:01 ` david
2009-01-03 23:14 ` Alan Cox
2009-01-05 10:54 ` Nick Piggin
2009-01-05 11:23 ` Alan Cox
2009-01-05 12:00 ` Nick Piggin
2009-01-05 16:05 ` Paul E. McKenney
2009-01-05 16:25 ` Nick Piggin
2009-01-05 17:30 ` Paul E. McKenney
2009-01-05 18:25 ` Nick Piggin
2009-01-05 22:01 ` Paul E. McKenney
2009-01-03 23:53 ` Robert Hancock
2009-01-04 18:05 ` Tilman Schmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox