Re: Synchronizing auxiliary mutex data

[Torvald Riegel <triegel at redhat dot com>]

On Tue, 2017-06-20 at 20:37 +0300, Alexander Monakov wrote:
> On Tue, 20 Jun 2017, Andreas Schwab wrote:
> 
> > On Jun 19 2017, Torvald Riegel <triegel@redhat.com> wrote:
> > 
> > > __owner accesses need to use atomics (which they don't, currently).
> > 
> > Does that mean mutexes are broken right now?
> 
> Plain accesses to fields like __data.owner are fine as long as they all are
> within critical sections set up by LLL_MUTEX_{LOCK,UNLOCK}, but there are some
> outside of them. So e.g. in nptl/pthread_mutex_lock:
> 
>   95   else if (__builtin_expect (PTHREAD_MUTEX_TYPE (mutex)
>   96                              == PTHREAD_MUTEX_RECURSIVE_NP, 1))
>   97     {
>   98       /* Recursive mutex.  */
>   99       pid_t id = THREAD_GETMEM (THREAD_SELF, tid);
>  100 
>  101       /* Check whether we already hold the mutex.  */
>  102       if (mutex->__data.__owner == id)
>  103         {
>  104           /* Just bump the counter.  */
>  105           if (__glibc_unlikely (mutex->__data.__count + 1 == 0))
>  106             /* Overflow of the counter.  */
>  107             return EAGAIN;
>  108 
>  109           ++mutex->__data.__count;
>  110 
>  111           return 0;
>  112         }
>  113 
>  114       /* We have to get the mutex.  */
>  115       LLL_MUTEX_LOCK (mutex);
>  116 
>  117       assert (mutex->__data.__owner == 0);
> 
> afaict the access at line 102 can invoke undefined behavior due to a data race.

All of them can.  Even if the access is in the critical section, because
it's not atomic, the compiler is technically allowed to use it as
scratch space for a while.

> In practice I think it works fine because the compiler doesn't tear the load,
> and the hardware also doesn't tear the load. I think in this specific example
> no-tearing guarantee is sufficient: if this thread never owned this mutex, it
> cannot observe __owner == id, or if it is no longer an owner, it stored 0 to
> __owner during unlock, followed by a release barrier, and will now observe the
> result of that or any later store.

For this particular point, the release barrier is irrelevant (of course,
it's required to actually order the stores associated with the different
critical sections).