Re: [PATCH 4/6] Do not call _xend if no transaction is active.

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

On Wed, Sep 11, 2013 at 03:31:12PM +0200, Torvald Riegel wrote:
> On Tue, 2013-09-03 at 09:45 +0200, Dominik Vogt wrote:
> > Example: A program uses glibc with lock elision and a library
> > "foo".  Foo provides the functions foo_lock() and foo_unlock() and
> > also uses transactions.  In foo it is perfectly valid to call
> > foo_unlock() without foo_lock().  Let foo_unlock() be implemented
> > like this (let's ignore any race conditions for simplicity):
> > 
> >   foo_unlock(foo_lock *x)
> >   {
> >       if (x->lock_count > 0)
> >           if (_xtest() != 0)
> >               _xend();
> >       else
> >           x->lock_count--;
> >   }
> > 
> > If the program looks like this:
> > 
> >     foo_lock(x);
> >     phtread_mutex_lock(m);
>       foo();  // see below
> >     foo_unlock(x);
>       bar();  // see below
> >     phtread_mutex_unlock(m);
> 
> This is an incorrect use of transactions.

(Sorry for the late reply, I've been ill for a while.)

If you remove the _xtest() and call _xend() unconditionally, this
is exactly the logic that is implemented in glibc.  Does the fact
that the glibc interfaces are backed by Posix requirements make

!!!

?

  When foo_unlock() commits a
> transaction that it has not started, it potentially breaks the atomicity
> guarantees of other code -- effectively, it makes the critical section
> shorter.  bar() would run nontransactionally, and thus, for example,
> observe data that's been changed since it was observed in foo() -- same
> as if foo_unlock() would release m early.  This already can lead to all
> sorts of crashes, and thus isn't a safe thing to do in general.

!!!

> (One could argue that the program can rely on pthread_mutex_* to just
> acquire/release this mutex in a traditional non-HLE way, without having
> to rely on anything the program does.  However, POSIX doesn't specify
> what the implementation does, and foo_unlock() clearly messes with
> potential implementations.)
> 
> Thus, I think that this is a buggy program and not a flaw in glibc.

Torvald Riegel Wrote:
> On Mon, 2013-09-02 at 10:04 +0200, Dominik Vogt wrote:
> > -    _xend();
> > +    if (_xtest () != 0)
> > +      _xend ();
> >
>  
> I disagree with this change (see my other email for details).  If we add
> such a change in the future because we see that lots of people write
> buggy programs (and they incorrectly blame glibc for it), then at the
> very least we should fail visibly here, not silent.  That is, this
> should have an else branch that does something like we do for
> error-checking mutexes.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany