This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add PTHREAD_MUTEX_NORMAL_INT
- From: Andi Kleen <andi at firstfloor dot org>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: Andi Kleen <ak at linux dot jf dot intel dot com>, Andi Kleen <andi at firstfloor dot org>, libc-alpha at sourceware dot org
- Date: Tue, 25 Jun 2013 01:26:07 +0200
- Subject: Re: [PATCH] Add PTHREAD_MUTEX_NORMAL_INT
- References: <1372105055-31254-1-git-send-email-andi at firstfloor dot org> <51C8AB50 dot 80108 at redhat dot com> <20130624203147 dot GO5643 at tassilo dot jf dot intel dot com> <51C8B797 dot 7080503 at redhat dot com>
On Mon, Jun 24, 2013 at 05:18:15PM -0400, Carlos O'Donell wrote:
> On 06/24/2013 04:31 PM, Andi Kleen wrote:
> > On Mon, Jun 24, 2013 at 04:25:52PM -0400, Carlos O'Donell wrote:
> >> On 06/24/2013 04:17 PM, Andi Kleen wrote:
> >>> From: Andi Kleen <ak@linux.intel.com>
> >>>
> >>> Carlos asked for PTHREAD_MUTEX_NORMAL_INT to be added
> >>> to make some of the internal macros be easier to understand.
> >>> It is always identical to DEFAULT, as NORMAL only makes
> >>> a difference for settype.
> >>
> >> Thanks for this.
> >>
> >> As it stands however, with pthread_mutexattr_gettype loosing
> >> the type originally entered by the user, we are going to have to
> >> propagate the new type internally.
> >
> > It will return DEFAULT|NO_ELISION, which is identical in behaviour
> > to NORMAL
> >
> > So if you do
> >
> > settype(&a, NORMAL);
> > settype(&b, gettype(&a));
> >
> > it will all work as expected. Even b will have all NORMAL semantics.
> >
> > The only case that wouldn't work is someone explicitely rechecking
> > the value returned by gettype(). Is that really a problem?
>
> Yes it is a problem.
>
> It would violate the expected semantics of the interface.
Here is an incremential patch to fix this. It moves the
conversion to pthread_mutex_init(), so get/set are symmetrical
on the attribute.
Let me know if I should repost the whole thing.
Subject: [PATCH] Fix pthread_mutexattr_gettype returning different type.
We move the NORMAL->DEFAULT conversion to pthread_mutex_init(),
so get/set on the mutex attribute structure works as expected.
2013-06-24 Andi Kleen <ak@linux.intel.com>
* pthread_mutexattr_settype.c (pthread_mutexattr_settype_worker):
Remove PTHREAD_MUTEX_NORMAL -> DEFAULT conversion.
(__pthread_mutexattr_settype_old): Set NORMAL flag
* pthread_mutex_init.c (__pthread_mutex_init): ... and move to here.
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index f6f0f80..8b8fff8 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -128,6 +128,19 @@ __pthread_mutex_init (mutex, mutexattr)
if ((imutexattr->mutexkind & (PTHREAD_MUTEXATTR_FLAG_PSHARED
| PTHREAD_MUTEXATTR_FLAG_ROBUST)) != 0)
mutex->__data.__kind |= PTHREAD_MUTEX_PSHARED_BIT;
+
+ /* When a NORMAL mutex is explicitly specified, default to no elision
+ to satisfy POSIX's deadlock requirement. Also convert the NORMAL
+ type to DEFAULT, as the rest of the lock library doesn't have
+ the code paths for them. */
+ if ((mutex->__data.__kind & PTHREAD_MUTEX_KIND_MASK_NP)
+ == PTHREAD_MUTEX_NORMAL)
+ {
+ if ((imutexattr->mutexkind & PTHREAD_MUTEX_ELISION_FLAGS_NP) == 0)
+ mutex->__data.__kind |= PTHREAD_MUTEX_NO_ELISION_NP;
+ mutex->__data.__kind = PTHREAD_MUTEX_DEFAULT
+ | (mutex->__data.__kind & PTHREAD_MUTEX_KIND_MASK_NP);
+ }
/* Drop elision bits for any unusual flags, except for PSHARED.
These can be set implicitely now, but the other code paths don't
diff --git a/nptl/pthread_mutexattr_settype.c b/nptl/pthread_mutexattr_settype.c
index ae1be45..d212148 100644
--- a/nptl/pthread_mutexattr_settype.c
+++ b/nptl/pthread_mutexattr_settype.c
@@ -36,17 +36,6 @@ pthread_mutexattr_settype_worker (pthread_mutexattr_t *attr, int kind)
if ((kind & PTHREAD_MUTEX_ELISION_FLAGS_NP) == PTHREAD_MUTEX_ELISION_FLAGS_NP)
return EINVAL;
- /* When a NORMAL mutex is explicitly specified, default to no elision
- to satisfy POSIX's deadlock requirement. Also convert the NORMAL
- type to DEFAULT, as the rest of the lock library doesn't have
- the code paths for them. */
- if (mkind == PTHREAD_MUTEX_NORMAL)
- {
- kind = PTHREAD_MUTEX_DEFAULT | (kind & PTHREAD_MUTEX_ELISION_FLAGS_NP);
- if ((kind & PTHREAD_MUTEX_ELISION_FLAGS_NP) == 0)
- kind |= PTHREAD_MUTEX_NO_ELISION_NP;
- }
-
/* When the CPU does not support elision never allow to set the elision
flags. */
if ((kind & PTHREAD_MUTEX_ELISION_FLAGS_NP) && !ENABLE_ELISION)
@@ -82,10 +71,10 @@ int
attribute_compat_text_section
__pthread_mutexattr_settype_old (pthread_mutexattr_t *attr, int kind)
{
- /* Force no elision for the old ambigious DEFAULT/NORMAL
- kind. */
+ /* Force NORMAL (= no elision) for the old ambigious
+ DEFAULT/NORMAL kind. */
if (kind == PTHREAD_MUTEX_DEFAULT)
- kind |= PTHREAD_MUTEX_NO_ELISION_NP;
+ kind |= PTHREAD_MUTEX_NORMAL;
return pthread_mutexattr_settype_worker (attr, kind);
}