lkml.org 
[lkml]   [2013]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [lttng-dev] [PATCH] Add ACCESS_ONCE() to avoid compiler splitting assignments
* Mathieu Desnoyers (mathieu.desnoyers@efficios.com) wrote:
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> > As noted by Konstantin Khlebnikov, gcc can split assignment of
> > constants to long variables (https://lkml.org/lkml/2013/1/15/141),
> > though assignment of NULL (0) is OK. Assuming that a gcc bug is
> > fixed (http://gcc.gnu.org/bugzilla/attachment.cgi?id=29169&action=diff
> > has a patch), making the store be volatile keeps gcc from splitting.
> >
> > This commit therefore applies ACCESS_ONCE() to CMM_STORE_SHARED(),
> > which is the underlying primitive used by rcu_assign_pointer().
>
> Hi Paul,
>
> I recognise that this is an issue in the Linux kernel, since a simple
> store is used and expected to be performed atomically when aligned.
> However, I think this does not affect liburcu, see below:

Side question: what gcc versions may issue non-atomic volatile stores ?
I think we should at least document those. Bug
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55981 seems to target gcc
4.7.2, but I wonder when this issue first appeared on x86 and x86-64
(and if it affects other architectures as well).

Thanks,

Mathieu

>
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > diff --git a/urcu/system.h b/urcu/system.h
> > index 2a45f22..7a1887e 100644
> > --- a/urcu/system.h
> > +++ b/urcu/system.h
> > @@ -49,7 +49,7 @@
> > */
> > #define CMM_STORE_SHARED(x, v) \
> > ({ \
> > - __typeof__(x) _v = _CMM_STORE_SHARED(x, v); \
> > + __typeof__(x) CMM_ACCESS_ONCE(_v) = _CMM_STORE_SHARED(x, v); \
>
> Here, the macro "_CMM_STORE_SHARED(x, v)" is doing the actual store.
> It stores v into "x". So adding a CMM_ACCESS_ONCE(_v), as you propose
> here, is really only making sure the return value (usually unused),
> located on the stack, is accessed with a volatile access, which does not
> make much sense.
>
> What really matters is the _CMM_STORE_SHARED() macro:
>
> #define _CMM_STORE_SHARED(x, v) ({ CMM_ACCESS_ONCE(x) = (v); })
>
> which already uses a volatile access for the store. So this seems to be
> a case where our preemptive use of volatile for stores in addition to
> loads made us bug-free for a gcc behavior unexpected at the time we
> implemented this macro. Just a touch of paranoia seems to be a good
> thing sometimes. ;-)
>
> Thoughts ?
>
> Thanks,
>
> Mathieu
>
> > cmm_smp_wmc(); \
> > _v; \
> > })
> >
> >
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com


\
 
 \ /
  Last update: 2013-01-16 14:21    [W:0.043 / U:0.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site