This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix pthread_cond_wait with requeue-PI on i386
- From: Dinakar Guniguntala <dino at in dot ibm dot com>
- To: Michal Schmidt <mschmidt at redhat dot com>
- Cc: libc-alpha at sources dot redhat dot com
- Date: Fri, 8 Jan 2010 13:44:02 +0530
- Subject: Re: [PATCH] Fix pthread_cond_wait with requeue-PI on i386
- References: <20100105153006.1993cf90@leela>
- Reply-to: dino at in dot ibm dot com
On Tue, Jan 05, 2010 at 03:30:06PM +0100, Michal Schmidt wrote:
> The recent addition of requeue-PI support for i386 broke PI mutexes:
> https://bugzilla.redhat.com/show_bug.cgi?id=548989
>
> When the FUTEX_WAIT_REQUEUE_PI operation was successful,
> pthread_cond_wait and pthread_cont_timedwait fail to call
> __pthread_mutex_cond_lock_adjust, leaving the mutex in a weird state.
Thank you for fixing this !
>
> When I built the patched glibc in Koji (Fedora's build system) there
> was an error in the test suite:
> tst-robustpi8: pthread_mutex_lock.c:312: __pthread_mutex_lock_full:
> Assertion `(-(e)) != 3 || !robust' failed.
> Didn't expect signal from child: got `Aborted'
> I could not reproduce this failure locally.
Hmm I cant seem to reproduce this on latest git
>
> The build finished fine though and the resulting package works for me.
> I can no longer reproduce the bug using the simple testcase attached to
> the BZ (https://bugzilla.redhat.com/show_bug.cgi?id=548989#c16).
One minor comment below
>
> Michal
>
> Index: glibc-2.11-73-g2510d01/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
> ===================================================================
> --- glibc-2.11-73-g2510d01.orig/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
> +++ glibc-2.11-73-g2510d01/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S
> @@ -247,12 +247,11 @@ __pthread_cond_wait:
> jne 10f
>
> /* With requeue_pi, the mutex lock is held in the kernel. */
> -11: xorl %eax, %eax
> +11: movl 24+FRAME_SIZE(%esp), %eax
> movl 16(%esp), %ecx
> testl %ecx, %ecx
> - jnz 20f
> + jnz 21f
>
> - movl 24+FRAME_SIZE(%esp), %eax
> call __pthread_mutex_cond_lock
> 20: addl $FRAME_SIZE, %esp
> cfi_adjust_cfa_offset(-FRAME_SIZE);
> Index: glibc-2.11-73-g2510d01/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
> ===================================================================
> --- glibc-2.11-73-g2510d01.orig/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
> +++ glibc-2.11-73-g2510d01/nptl/sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S
> @@ -326,14 +326,13 @@ __pthread_cond_timedwait:
> #endif
> jne 10f
>
> -11: xorl %eax, %eax
> +11: movl 24+FRAME_SIZE(%esp), %eax
> /* With requeue_pi, the mutex lock is held in the kernel. */
> movl 24(%esp), %ecx
> testl %ecx, %ecx
> - jnz 26f
> + jnz 27f
>
> /* Remove cancellation handler. */
You might want to move this comment to the top as well
-Dinakar
> - movl 24+FRAME_SIZE(%esp), %eax
> call __pthread_mutex_cond_lock
> 26: addl $FRAME_SIZE, %esp
> cfi_adjust_cfa_offset(-FRAME_SIZE);
> @@ -366,6 +365,7 @@ __pthread_cond_timedwait:
> cfi_restore_state
>
> 27: call __pthread_mutex_cond_lock_adjust
> + xorl %eax, %eax
> jmp 26b
>
> /* Initial locking failed. */