Re: [Gc] glibc 2.19 lock elision bug

On Sat, Jul 19, 2014 at 04:48:39PM +0400, Ivan Maidanski wrote:
>  Hi Paul,
> 
> Thank you for the patch.
> I reviewed it and applied several changes - see  https://github.com/ivmai/bdwgc/commit/757af8aa17ed107ff2915f93967087835c0100dc :
> * fix version check: major >= 2 && minor >= 19 -> major > 2 || major == 2 && minor >= 19
> * fix scope of workaround application - only Linux/x64 with Glibc (instead of Linux on all CPUs)
> * avoid potential recursion (in case strdup/free redirected to GC_strdup/free)
> * avoid extra configure checks (currently all major targets could be compiled even without running configure&make)
> * code refactoring:
> ** move version parsing to a separate function
> ** use mutex static initializer if workaround is not needed
> ** call ABORT on pthread function call error
> 
> Please test new code.
> 
> But still I don't know whether the proposed solution is good enough to be in master. The questions are:
> * is there a link pointing to a lock elision bug implementation in Glibc? If not, it is better to register such
> * this workaround simply disables lock elision (causing potentially degradation of performance). which pthread primitive causes malfunction? if trylock then we could avoid it.
> 

Hi Ivan,

I've finally made time to test this, it works without any problems.  I made
one minor change and that is to use PTHREAD_MUTEX_NORMAL rather than
PTHREAD_MUTEX_ERRORCHECK.  My colleague Peter Wang discovered that
PTHREAD_MUTEX_NOMRAL is sufficient to disable lock elision on current
versions of glibc (and I've tested it).  The benefit is that without error
checking this may be faster.

There's no glibc bug report that I know of yet but I wrote to the author of
glibc's lock elision, Andi Kleen, who isn't yet convinced that it's a bug in
glibc.  He's pointed me at a patch to glibc that I should try and report
back but I haven't yet had the time.

The pthread primitive that throws the error is pthread_cond_wait.

I wrote the following to Andi:

    I noticed the following error:
    mercury_compile: ../nptl/pthread_mutex_lock.c:80: __pthread_mutex_cond_lock: Assertion `mutex->__data.__owner == 0' failed.

    This is thrown (indirectly) from a call to pthread_cond_wait in
    pthread_support.c line 2036 in Boehm GC 7.4.2  I have the same problem
    with Boehm Gc 7.2.  There doesn't appear to be anything suspicious about
    the use of the mutex or condition variable involved here.

    A different bug affecting libtirpc and mount_nfs also started occuring
    when I upgraded to eglibc 2.19.  When investigating this I found that
    eglibc 2.19 introduced lock elision using TSX extensions and found your
    article here: http://lwn.net/Articles/534758/ I use an i7-4770 processor
    which supports TSX.  (I chose this one because I wanted to experiment
    with some lock free code myself.)

    I've looked at the NTPL code and the Boehm code and I don't see anything
    obvious - not that the NTPL assembler is easy to read.  Given that the
    assertion refers to the __owner field, and that on elision paths don't
    update this field I wonder if they're related, that is that not updating
    the __owner field has other issues.

    This mutex and condition variable refer to the Boehm collector's marking
    phase, which will read and update a lot of memory.  Is the mutex code
    falling back from lock elision to normal locks for this mutex and then
    triggering the assertion because the owner field hasn't been updated?

So I'm afraid that this isn't enough to pin down the cause of the issue.
What I guess is happening is that the mutex starts being used with HLE, but
then due to some reason, possibly pthread_cond_wait, HLE cannot be used but
the __owner field hasn't been fixed.  Or pthread_cond_wait simply doesn't
account for mutexes that use HLE and therefore may have an invalid __owner
field.  I'm afraid that the glibc sources are quite opaque.

All the best.


-- 
Paul Bone