Re: [PATCH] Add STATIC_GETENV macro.

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

On Fri, Nov 15, 2013 at 10:43:49AM +0100, Torvald Riegel wrote:
> On Thu, 2013-11-14 at 21:40 +0100, OndÅej BÃlka wrote:
> > 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;                                                       \
> > })
> 
> That can work under certain assumptions.  Thus, you still need to
> document those; that this "should work" isn't helpful to anyone else
> looking at this code at a later time.  These are properties of /
> requirements for getenv (or, if you want to generalize this, the
> particular initialization function).
> 
A generalization would be a reentrant version of pthread_once, if
initialization routine is reentrant and returns valid pointer then you
can run this routine and it will always return result of first run.

For slow initialization funcitions it may needlessly recompute result
but for that I would need a reentrant mutex.

#define once_r_define(CLASS, NAME) CLASS void *NAME = &NAME

inline void *
once_r (void **name, void *(init) (void), void (destroy)(void *))
{
  void *addr = __atomic_load_n (name, __ATOMIC_CONSUME);
  if (addr == (void *) name)
    {
      void *computed = init ();
      if (__atomic_compare_exchange_n (name, &addr, computed, false, __ATOMIC_ACQ_REL, __ATOMIC_RELAXED))
	{
	  return computed;
	}
      else
	{
	  destroy (computed);
	  return addr;
	}
    }
  return addr;
}