Messages in this thread | | | Subject | Re: >Re: [RFC] should VM_BUG_ON(cond) really evaluate cond | From | Eric Dumazet <> | Date | Fri, 28 Oct 2011 14:09:41 +0200 |
| |
Le vendredi 28 octobre 2011 à 04:37 -0700, Linus Torvalds a écrit : > On Thu, Oct 27, 2011 at 9:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > The only requirement of atomic_read() is that it must return value > > before or after an atomic_write(), not a garbled value. > > The problem is that gcc *can* return a garbled value. > > > In fact, if a compiler is stupid enough to issue two reads on following > > code : > > The compiler really *can* be that "stupid". Except the code tends to > look like this: > > int value = atomic_read(&atomic_var); > if (value > 10) > return; > .. do something with value .. > > and gcc may decide - under register pressure, and in the absense of a > 'volatile' - to read 'value' first once for that "> 10" check, and > then it drops the registers and instead of saving it on the stack > frame, it can decide to re-load it from atomic_var. > > IOW, "value" could be two or more different values: one value when > testing, and *another* value in "do something with value". > > This is why we have "ACCESS_ONCE()". > > Whether atomics guarantee ACCESS_ONCE() semantics or not is not > entirely clear. But afaik, there is no way to tell gcc "access at > *most* once, and never ever reload". >
What you describe is true for non atomic variables as well, its not part of the atomic_ops documented semantic.
And we do use ACCESS_ONCE() on the rare cases we need to make sure no reload is done.
RCU use makes this implied (ACCESS_ONCE() being done in rcu_dereference()), so we dont have many raw ACCESS_ONCE() in our code.
int value = ACCESS_ONCE(atomic_read(&atomic_var)); if (value > 10) return; .. do something with value ..
One of such rare use is explained in commit f1987257 (tcp: protect sysctl_tcp_cookie_size reads)
Since its a bit ugly, I suggested :
int value = atomic_read_once(&atomic_var); if (value > 10) return; .. do something with value ..
I dont know, it seems the right way, but yes it might break things.
We can take the otherway and patch thousand atomic_read() to atomic_read_stable(), its safer but very boring :)
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |