| |||||||||||||
Declaring it volatileDeclaring it volatilePosted Aug 2, 2012 21:14 UTC (Thu) by corbet (editor, #1)In reply to: ACCESS_ONCE() by thedevil Parent article: ACCESS_ONCE() Because that would pessimize all accesses to that field. In general, use of volatile is strongly discouraged in the kernel; there's really only a few places where it is really needed.
(Log in to post comments)
Declaring it volatile Posted Aug 3, 2012 17:01 UTC (Fri) by daglwn (subscriber, #65432) [Link]
I understand Linus' reasoning but I don't agree with it. He's right that usually you want to mark an access as, "get this from memory always." However, C doesn't have a defined way to do that (ACCESS_ONCE is nonstandard). The real solution is to introduce a new keyword (not called "volatile") that can mark object access expressions.
Lacking that, it should be sufficient to load the volatile-qualified name into a temporary and use the temporary to allow optimization. This way code is safe by default (no need to remember to use ACCESS_ONCE) and can be optimized explicitly where developers know it is safe.
Declaring it volatile Posted Aug 6, 2012 17:04 UTC (Mon) by PaulMcKenney (subscriber, #9624) [Link]
Is your objection to ACCESS_ONCE() specifically, to the volatile cast, or to the use of the non-standard typeof()? The reason I ask is that I am not sure whether your recommendation of "volatile-qualified name" includes volatile casts or whether you are instead insisting that we declare the variable being loaded from as volatile.
Declaring it volatile Posted Aug 6, 2012 17:23 UTC (Mon) by daglwn (subscriber, #65432) [Link]
ACCESS_ONCE is nonstandard due to typeof. That's not too bad for the kernel because developers only expect it to be compilable by gcc anyway.
The use of a cast is fine.
I'm suggesting the variable be declared volatile for safety reasons and temporaries be used to explicitly avoid pessimizing code when developers know it is safe. That way things are safe by default and explicitly and obviously optimized when possible.
Declaring it volatile Posted Aug 6, 2012 17:24 UTC (Mon) by daglwn (subscriber, #65432) [Link]
I'll just add that my point is that declaring the variable volatile doesn't kill performance everywhere. It's perfectly possible to do performance optimizations when safe. By using temporaries.
Declaring it volatile Posted Aug 6, 2012 18:16 UTC (Mon) by PaulMcKenney (subscriber, #9624) [Link] It does sound like it just might be well past time for typeof() in the C/C++ standards. ;-) That said, both gcc and llvm seem to support typeof(), and I bet a lot of other compilers do as well, either directly or indirectly. But perhaps the Linux kernel will eventually move from typeof() to C++11 decltype(), once the commonly-used compiler versions support it.Yes, you can in principle use manually coded temporaries to enable compiler optimizations in cases where a lock is held that prevents changes to the variable being loaded, but it is not at all clear to me why this would be a better solution than using ACCESS_ONCE(), especially in the Linux kernel.
Declaring it volatile Posted Aug 6, 2012 20:49 UTC (Mon) by daglwn (subscriber, #65432) [Link]
> Yes, you can in principle use manually coded temporaries to enable
> compiler optimizations in cases where a lock is held that prevents changes > to the variable being loaded, but it is not at all clear to me why this > would be a better solution than using ACCESS_ONCE()
Safety and explicitness.
Declaring it volatile Posted Aug 6, 2012 23:12 UTC (Mon) by PaulMcKenney (subscriber, #9624) [Link]
Sorry, but it is still not clear to me, especially your point about explicitness. After all, one of the advantages of the ACCESS_ONCE() approach is that lockless accesses are very clearly and explicitly flagged.
Declaring it volatile Posted Aug 7, 2012 3:42 UTC (Tue) by daglwn (subscriber, #65432) [Link]
No, ACCESS_ONCE explicitly flags "volatile" accesses. That is, one has to explicitly use it to guarantee safety. That's an invitation for bugs.
Declaring it volatile Posted Aug 7, 2012 14:37 UTC (Tue) by PaulMcKenney (subscriber, #9624) [Link]
And one of the corresponding bugs for your suggested approach of marking the variable volatile and caching the value in a temporary is failure to update the temporary when updating the underlying variable. So I remain unconvinced.
Declaring it volatile Posted Aug 7, 2012 19:44 UTC (Tue) by daglwn (subscriber, #65432) [Link]
If the programmmer is optimizing by using a temporary, the assumption is that the programmer knows what he or she is doing. The optimization is explicit and is an immediate flag to the reader to carefully examine the (existing or new) code for bugs.
I'm not trying to convince you, simply stating what I believe is good programming practice. Maybe I'm wrong. But it's worked for me so far. :)
Declaring it volatile Posted Aug 8, 2012 15:26 UTC (Wed) by PaulMcKenney (subscriber, #9624) [Link]
To each their own, I guess.
Declaring it volatile Posted Aug 7, 2012 5:52 UTC (Tue) by viro (subscriber, #7872) [Link]
Have fun deciding how its semantics should look like for variably-modified types. Gets *really* interesting when combined with another gccism, of course - ({...}). In effect, that allows something that looked like local in one scope to leak outside; gcc used to ICE on fairly simple combinations.
IIRC, there had been other places where it didn't play well with very unexpected parts of standard (e.g. bitfield signedness; does special treatment of signed int in there extend to e.g. typeof(signed int)? Taken a few steps further it can, unless you are very careful, extend the distinction between int and signed int through all the type system...)
Declaring it volatile Posted Aug 7, 2012 15:24 UTC (Tue) by PaulMcKenney (subscriber, #9624) [Link] I must admit that I have never used "typeof(({ printf("From typeof()\n"); j; })) k = 1;" to declare a variable "k" of the same type as "j", but gcc appears quite happy with it.And yes, gcc also seems quite happy to leak types from "({...})":
#include <stdio.h>
int main(int argc, char *argv[])
{
int i = argc;
signed int j = i;
typeof(({ struct foo { int a; } q = { 1 }; printf("From typeof()\n"); q; })) k = { 2 };
printf("i = %d, j = %d, k.a = %d\n", i, j, k.a);
return 0;
}
Yow!!! Fortunately for me, ACCESS_ONCE() does not depend on the semantics of typeof() applied to either "({...})"; or bitfields, although only because it is illegal to take the address of either of them. ;-)
Declaring it volatile Posted Aug 7, 2012 15:35 UTC (Tue) by nybble41 (subscriber, #55106) [Link]
> Fortunately for me, ACCESS_ONCE() does not depend on the semantics of typeof() applied to either "({...})"; or bitfields, although only because it is illegal to take the address of either of them. ;-)
Are you sure about the first case? It seemed to work for me in GCC 4.5.1, at least in the case where the result of the "({...})" is a non-local l-value:
int x;
Declaring it volatile Posted Aug 7, 2012 15:57 UTC (Tue) by PaulMcKenney (subscriber, #9624) [Link] I am using gcc 4.6.1, and the following emits a compiler error complaining that an lvalue is needed:
#include <stdio.h>
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
int q = 3;
int main(int argc, char *argv[])
{
int i = argc;
i = ACCESS_ONCE(({ printk("foo"); q; }));
printk("i = %d\n", i);
}
Declaring it volatile Posted Aug 7, 2012 18:35 UTC (Tue) by nybble41 (subscriber, #55106) [Link]
Interesting... it seems to work only if the variable is the only expression inside the statement expression. "&({ x; })" and "&({ ; x; })" are accepted, but "&({ x; x; })" and "&({ (void)0; x; })" both fail as you described. Perhaps the compiler is reducing the trivial one-expression form to a normal expression before performing the l-value check. It would make more sense to me to either fail both cases or move the "&" operator inside the brackets, i.e. "&({ ...; x; })" internally becomes "({ ...; &x; })".
Is there any particular reason the statement expression can't be an l-value, provided the final form is an l-value, or was that case simply considered too error-prone?
Declaring it volatile Posted Aug 7, 2012 21:12 UTC (Tue) by paulj (subscriber, #341) [Link]
I have to ask, what is this ({…}) GCCism? I've searched the manual and online for it and "variably modified type" but not found anything useful.
Declaring it volatile Posted Aug 7, 2012 21:26 UTC (Tue) by PaulMcKenney (subscriber, #9624) [Link]
Here is the URL you are looking for: http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
Declaring it volatile Posted Aug 3, 2012 18:04 UTC (Fri) by BenHutchings (subscriber, #37955) [Link] Also, implicit access-once behaviour can make it harder to understand how the code works.
|
|||||||||||||