Re: [PATCH] Add STATIC_GETENV macro.

[OndÅej BÃlka <neleai at seznam dot cz>]

On Thu, Nov 14, 2013 at 06:01:08PM +0100, Torvald Riegel wrote:
> On Thu, 2013-11-14 at 17:48 +0100, OndÅej BÃlka wrote:
> > On Thu, Nov 14, 2013 at 04:49:57PM +0100, Torvald Riegel wrote:
> > > On Thu, 2013-11-14 at 13:52 +0100, OndÅej BÃlka wrote:
> > > > Also when you are concerned about machines that do issue nonatomic reads
> > > > then relaxed model is appropriate, acquire/consume could yield unacceptable
> > > > performance overhead.
> > > 
> > > Which relaxed model?  Do you mean relaxed memory order?  Atomicity is
> > > orthogonal to ordering constraints enforced by particular memory orders.
> > > Also, acquire/consume don't need any HW instructions on x86, sparc, and
> > > the like (they still constrain compiler optzns, of course).  Consume
> > > should be very little overhead on Power, and even acquire shouldn't be
> > > too much on Power and ARM.
> > > 
> > All that is needed is that value returned has is from set initial/value
> > returned by getenv.
> 
> No.  You also need to ensure (or show that it holds in any case) that
> whatever getenv() did on one Thread1 is also observed by Thread2 if
> Thread2 read the value written by Thread1.
> 
> > It does not matter which one is read as long as this is from this set,
> > so you do not need synchronization.
> 
> You do need atomic access to make that happen properly (ie, so that it's
> guaranteed by what the language).  Atomic accesses are synchronization.
> You need to consider happens-before for the condition above, but whether
> you need to do that with additional synchronization beyond atomic
> accesses depends on the problem.
> 
A atomic access is needed but nothing beyond that. In fact following
should work:

#define GETENV_CACHED(c) \
({                                                              \
   static char *__p = (char *) &__p;                            \
   char *__new = __atomic_load_n (&__p, __ATOMIC_RELAXED);      \
   if (__new == (char *) &__p)                                  \
     {                                                          \
       __new = getenv (c);                                      \
       __atomic_store_n (&__p, __new, __ATOMIC_RELAXED);        \
     }                                                          \
   __new;                                                       \
})