* 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 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
* 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
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