Re: Synchronizing auxiliary mutex data

[Torvald Riegel <triegel at redhat dot com>]

On Tue, 2017-06-20 at 22:35 +0300, Alexander Monakov wrote:
> On Tue, 20 Jun 2017, Torvald Riegel wrote:
> > > 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.
> 
> No, the compiler could only use that scratch space within that critical section,
> and thus other threads wouldn't be able to observe that.

There are loads from __owner outside of the critical sections.  That's
the data race we're talking about, and they would be able to observe
spurious writes in the critical sections.

> Generally the compiler can't introduce data races

I agree that it must not introduce data races, but that's always under
the precondition that the program is data-race-free itself.

> so in the quoted code snippet
> the compiler can't introduce spurious writes to __owner outside of the critical
> section (as the original code only reads it), and may introduce spurious writes
> to __count only under __owner==id condition.

The compiler is allowed to introduce spurious writes to __count in the
critical section if it writes to it nonatomically (again, because of
DRF) and if there's no other (potentially) synchronizing code (eg,  the
atomic accesses to __lock) between the original and the spurious write
(because of DRF). 
So, it could introduce spurious writes in the __owner==id branch
potentially, but in practice this would require whole-program analysis.