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: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: Chris Metcalf <cmetcalf at tilera dot com>
- Cc: libc-alpha at sourceware dot org, David Miller <davem at davemloft dot net>
- Date: Thu, 3 Oct 2013 08:26:08 +0530
- 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> <524B254A dot 20408 at tilera dot com>
On Tue, Oct 01, 2013 at 03:40:58PM -0400, Chris Metcalf wrote:
> On 10/1/2013 5: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?
>
> Great! This also fixes the tile architecture bug 15760, which I've now closed as a duplicate of 15988. I updated the NEWS file to mention 15760 as well.
>
> I've also updated the Release/2.18 wiki page to indicate that this bug is architecture-independent rather than a tilegx-specific bug :-)
>
> I assume it makes sense to backport this fix to the 2.18 branch.
I've created a 2.18 clone for the bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=15996
David, do you think this backport is OK for 2.18?
Siddhesh
[BZ #15996]
* 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.
commit d73eae369d571766451a04c7e0b887ac3bb33d59
Author: Siddhesh Poyarekar <siddhesh@redhat.com>
Date: Thu Oct 3 08:23:59 2013 +0530
Fix PI mutex check in pthread_cond_broadcast and pthread_cond_signal
Fixes BZ #15996.
The check had a typo - it checked for PTHREAD_MUTEX_ROBUST_NP instead
of PTHREAD_MUTEX_ROBUST_NORMAL_NP. It has now been replaced by the
already existing convenience macro USE_REQUEUE_PI.
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,