This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #15988] Fix check for PI mutex on non-x86 systems
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 01 Oct 2013 15:26:41 -0400
- Subject: Re: [PATCH][BZ #15988] Fix check for PI mutex on non-x86 systems
- Authentication-results: sourceware.org; auth=none
- References: <20131001095040 dot GB2823 at spoyarek dot pnq dot redhat dot com>
On 10/01/2013 05:50 AM, Siddhesh Poyarekar wrote:
> Hi,
>
> Fedora bug report:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1007590
>
> I don't know how I didn't notice this earlier, but tst-mutexpi8 was
> failing on s390 and ppc due to a bug I introduced while adding PI
> mutex support to pthread_cond_* for non-x86 systems. I had used the
> wrong flag to test against (PTHREAD_MUTEX_ROBUST instead of the
> correct PTHREAD_MUTEX_ROBUST_NORMAL_NP) which broke recursive PI
> mutexes. The patch below replaces this test with the USE_REQUEUE_PI
> macro. Tested on s390 and verified that it fixes tst-mutexpi8 and
> also does not introduce any new regressions. OK to commit?
>
> Siddhesh
>
> [BZ #15988]
> * pthread_cond_broadcast.c (__pthread_cond_broadcast)
> [lll_futex_cmp_requeue_pi && __ASSUME_REQUEUE_PI]: Use
> USE_REQUEUE_PI.
> * pthread_cond_signal.c (__pthread_cond_signal)
> [lll_futex_cmd_requeue_pi && __ASSUME_REQUEUE_PI]: Likewise.
>
> diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
> index 0702ec0..7ba9efa 100644
> --- a/nptl/pthread_cond_broadcast.c
> +++ b/nptl/pthread_cond_broadcast.c
> @@ -63,10 +63,7 @@ __pthread_cond_broadcast (cond)
>
> #if (defined lll_futex_cmp_requeue_pi \
> && defined __ASSUME_REQUEUE_PI)
> - int pi_flag = PTHREAD_MUTEX_PRIO_INHERIT_NP | PTHREAD_MUTEX_ROBUST_NP;
> - pi_flag &= mut->__data.__kind;
> -
> - if (pi_flag == PTHREAD_MUTEX_PRIO_INHERIT_NP)
> + if (USE_REQUEUE_PI (mut))
> {
> if (lll_futex_cmp_requeue_pi (&cond->__data.__futex, 1, INT_MAX,
> &mut->__data.__lock, futex_val,
> diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
> index 102d0b3..ffc35dc 100644
> --- a/nptl/pthread_cond_signal.c
> +++ b/nptl/pthread_cond_signal.c
> @@ -49,14 +49,9 @@ __pthread_cond_signal (cond)
>
> #if (defined lll_futex_cmp_requeue_pi \
> && defined __ASSUME_REQUEUE_PI)
> - int pi_flag = PTHREAD_MUTEX_PRIO_INHERIT_NP | PTHREAD_MUTEX_ROBUST_NP;
> pthread_mutex_t *mut = cond->__data.__mutex;
>
> - /* Do not use requeue for pshared condvars. */
> - if (mut != (void *) ~0l)
> - pi_flag &= mut->__data.__kind;
> -
> - if (__builtin_expect (pi_flag == PTHREAD_MUTEX_PRIO_INHERIT_NP, 0)
> + if (USE_REQUEUE_PI (mut)
> /* This can only really fail with a ENOSYS, since nobody can modify
> futex while we have the cond_lock. */
> && lll_futex_cmp_requeue_pi (&cond->__data.__futex, 1, 0,
>
Looks good to me.
Cheers,
Carlos.