| Message ID | 533338C4.5030107@linux.vnet.ibm.com |
|---|---|
| State | Committed |
| Delegated to: | Carlos O'Donell |
| Headers |
Return-Path: <x14307373@homiemail-mx21.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx21.g.dreamhost.com (caibbdcaaahb.dreamhost.com [208.113.200.71]) by wilcox.dreamhost.com (Postfix) with ESMTP id 9C4CA360249 for <siddhesh@wilcox.dreamhost.com>; Wed, 26 Mar 2014 13:30:10 -0700 (PDT) Received: by homiemail-mx21.g.dreamhost.com (Postfix, from userid 14307373) id 3D519E3FE76; Wed, 26 Mar 2014 13:30:10 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx21.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx21.g.dreamhost.com (Postfix) with ESMTPS id EDC82DE0E08 for <glibc@patchwork.siddhesh.in>; Wed, 26 Mar 2014 13:30:09 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; q=dns; s=default; b=HzMs/on7sBjRHIMWbFM0a0z2OBrWJpuY5UPHzN5gf5D UP4VfchOodv/EOk7CATfTXWLVlWIRJkaW89ufBcsix/eimfXbEgvpAMB1a0zdjQw wYkFEfSTwGa80VjZ1U794vKeAxeisH2W9i6DV3wdCQMEiBt2VEUfUwR+MjnmLGhw = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; s=default; bh=IikzwMohnOX9y4BwcCK3VvG2s6k=; b=cifRR0m+NaU8c2NnK qHNrS0PNNPt2+SLytm9iv/+VqO2bIzjEkX7vvvQ2/OTyaT845DVp6zTKfhohaFtG I/jWf52C51uOlQuvkqaQR/21sS0rlI3Ezlt2PtfN7RuI9ZkDzX0wiobyiBFxI5Uy gNT5qTbIeA85SIXX4DPBM8zYwU= Received: (qmail 2128 invoked by alias); 26 Mar 2014 20:30:07 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 1978 invoked by uid 89); 26 Mar 2014 20:30:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e24smtp03.br.ibm.com Message-ID: <533338C4.5030107@linux.vnet.ibm.com> Date: Wed, 26 Mar 2014 17:29:56 -0300 From: Adhemerval Zanella <azanella@linux.vnet.ibm.com> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: libc-alpha@sourceware.org Subject: Re: [PATCH] Remove __PTHREAD_MUTEX_HAVE_ELISION undefined warning References: <5332D913.5050903@linux.vnet.ibm.com> <20140326163647.3E42A74493@topped-with-meat.com> <53330F3B.6090009@linux.vnet.ibm.com> <20140326175007.D832074496@topped-with-meat.com> In-Reply-To: <20140326175007.D832074496@topped-with-meat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14032620-9564-0000-0000-0000006FC2E4 X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Adhemerval Zanella Netto
March 26, 2014, 8:29 p.m. UTC
On 26-03-2014 14:50, Roland McGrath wrote: > I don't understand what "support for 64 bits" or "support for 32 bits" > means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it > seems pretty wrong to pretend this is a generic 32/64 sort of thing when it > is really just about the x86-private layout of pthread_mutex_t. It seems > more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. > > That can be a separate cleanup if you want, and others may want to kibitz. > But that might involve dropping the header you're adding here, so maybe you > want to just resolve it now. > > If you want to go ahead with this change, then it's OK with the other nits > above and this comment rewritten to describe concretely what the macro > means. In actual usage so far, it doesn't actually mean anything about > elision support per se. It just means something about how the fields of > pthread_mutex_t are structured and hence what the initializer must look like. > If that's all it's for, it should be made clear. > Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t layout is also arch specific and does not make sense disassociate them. This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. -- * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION): Remove macro usage. (__PTHREAD_SPINS): Move definition to ... * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h (__PTHREAD_SPINS): ... here. * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Likewise. ports/ChangeLog.hppa * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h (__PTHREAD_SPIN): Moved defintion from pthread.h. ---
Comments
This looks great to me, except for ...
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */
... this typo ("initializer") repeated many times over.
But for final approval I'll defer to anyone else who is an arch maintainer
and/or has touched this area of the code more recently than I have.
Thanks,
Roland
Ping for arch maintainers. On 26-03-2014 17:29, Adhemerval Zanella wrote: > On 26-03-2014 14:50, Roland McGrath wrote: >> I don't understand what "support for 64 bits" or "support for 32 bits" >> means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it >> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it >> is really just about the x86-private layout of pthread_mutex_t. It seems >> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. >> >> That can be a separate cleanup if you want, and others may want to kibitz. >> But that might involve dropping the header you're adding here, so maybe you >> want to just resolve it now. >> >> If you want to go ahead with this change, then it's OK with the other nits >> above and this comment rewritten to describe concretely what the macro >> means. In actual usage so far, it doesn't actually mean anything about >> elision support per se. It just means something about how the fields of >> pthread_mutex_t are structured and hence what the initializer must look like. >> If that's all it's for, it should be made clear. >> > Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch > moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t > layout is also arch specific and does not make sense disassociate them. > This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. > > -- > > * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION): > Remove macro usage. > (__PTHREAD_SPINS): Move definition to ... > * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > (__PTHREAD_SPINS): ... here. > * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > > ports/ChangeLog.hppa > * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Moved defintion from pthread.h. > > --- > > diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h > index 1e0c5dc..40a3e21 100644 > --- a/nptl/sysdeps/pthread/pthread.h > +++ b/nptl/sysdeps/pthread/pthread.h > @@ -82,15 +82,6 @@ enum > #endif > > > -/* Mutex initializers. */ > -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */ > -#define __PTHREAD_SPINS 0, 0 > -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */ > -#define __PTHREAD_SPINS { 0, 0 } > -#else > -#define __PTHREAD_SPINS 0 > -#endif > - > #ifdef __PTHREAD_MUTEX_HAVE_PREV > # define PTHREAD_MUTEX_INITIALIZER \ > { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } > diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > index 71bd3ae..20e5fc0 100644 > --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > index 23a1698..716f151 100644 > --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > @@ -20,8 +20,6 @@ > > #include <bits/wordsize.h> > > -# define __PTHREAD_MUTEX_HAVE_ELISION 0 > - > #if __WORDSIZE == 64 > # define __SIZEOF_PTHREAD_ATTR_T 56 > # define __SIZEOF_PTHREAD_MUTEX_T 40 > @@ -107,6 +105,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > index e42d94e..f92958c 100644 > --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > index be615b6..7b9edf2 100644 > --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > index 28e5144..0ca10f2 100644 > --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > @@ -105,7 +105,8 @@ typedef union > short __elision; > __pthread_list_t __list; > # define __PTHREAD_MUTEX_HAVE_PREV 1 > -# define __PTHREAD_MUTEX_HAVE_ELISION 1 > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +# define __PTHREAD_SPINS 0, 0 > #else > unsigned int __nusers; > __extension__ union > @@ -116,7 +117,7 @@ typedef union > short __elision; > # define __spins d.__espins > # define __elision d.__elision > -# define __PTHREAD_MUTEX_HAVE_ELISION 2 > +# define __PTHREAD_SPINS { 0, 0 } > } d; > __pthread_slist_t __list; > }; > diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > index deec4da..621895b 100644 > --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > @@ -94,6 +94,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > index f11eeab..76b94b4 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > @@ -74,6 +74,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > index 6f85eae..c633c6d 100644 > --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > @@ -73,6 +73,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > index 26edce5..0a0af56 100644 > --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > index b77b80a..49969bb 100644 > --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > @@ -74,6 +74,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > index 283f240..b21f5f8 100644 > --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > index ca053e3..d4c5600 100644 > --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > @@ -76,6 +76,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > index 9d9386b..712076c 100644 > --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > index f469352..17bb112 100644 > --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > @@ -112,6 +112,9 @@ typedef union > int __align; > } pthread_mutexattr_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + > > /* Data structure for conditional variable handling. The structure of > the attribute type is not exposed on purpose. */ >
Hi, this is okay on s390. I have tested the patch with s390/s390x. Thanks. On 04/02/2014 02:35 PM, Adhemerval Zanella wrote: > Ping for arch maintainers. > > On 26-03-2014 17:29, Adhemerval Zanella wrote: >> On 26-03-2014 14:50, Roland McGrath wrote: >>> I don't understand what "support for 64 bits" or "support for 32 bits" >>> means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it >>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it >>> is really just about the x86-private layout of pthread_mutex_t. It seems >>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. >>> >>> That can be a separate cleanup if you want, and others may want to kibitz. >>> But that might involve dropping the header you're adding here, so maybe you >>> want to just resolve it now. >>> >>> If you want to go ahead with this change, then it's OK with the other nits >>> above and this comment rewritten to describe concretely what the macro >>> means. In actual usage so far, it doesn't actually mean anything about >>> elision support per se. It just means something about how the fields of >>> pthread_mutex_t are structured and hence what the initializer must look like. >>> If that's all it's for, it should be made clear. >>> >> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch >> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t >> layout is also arch specific and does not make sense disassociate them. >> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. >> >> -- >> >> * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION): >> Remove macro usage. >> (__PTHREAD_SPINS): Move definition to ... >> * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> (__PTHREAD_SPINS): ... here. >> * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Likewise. >> >> ports/ChangeLog.hppa >> * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >> (__PTHREAD_SPIN): Moved defintion from pthread.h. >> >> --- >> >> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h >> index 1e0c5dc..40a3e21 100644 >> --- a/nptl/sysdeps/pthread/pthread.h >> +++ b/nptl/sysdeps/pthread/pthread.h >> @@ -82,15 +82,6 @@ enum >> #endif >> >> >> -/* Mutex initializers. */ >> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */ >> -#define __PTHREAD_SPINS 0, 0 >> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */ >> -#define __PTHREAD_SPINS { 0, 0 } >> -#else >> -#define __PTHREAD_SPINS 0 >> -#endif >> - >> #ifdef __PTHREAD_MUTEX_HAVE_PREV >> # define PTHREAD_MUTEX_INITIALIZER \ >> { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } >> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> index 71bd3ae..20e5fc0 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >> @@ -106,6 +106,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >> index 23a1698..716f151 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >> @@ -20,8 +20,6 @@ >> >> #include <bits/wordsize.h> >> >> -# define __PTHREAD_MUTEX_HAVE_ELISION 0 >> - >> #if __WORDSIZE == 64 >> # define __SIZEOF_PTHREAD_ATTR_T 56 >> # define __SIZEOF_PTHREAD_MUTEX_T 40 >> @@ -107,6 +105,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >> index e42d94e..f92958c 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >> @@ -77,6 +77,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >> index be615b6..7b9edf2 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >> @@ -106,6 +106,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >> index 28e5144..0ca10f2 100644 >> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >> @@ -105,7 +105,8 @@ typedef union >> short __elision; >> __pthread_list_t __list; >> # define __PTHREAD_MUTEX_HAVE_PREV 1 >> -# define __PTHREAD_MUTEX_HAVE_ELISION 1 >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +# define __PTHREAD_SPINS 0, 0 >> #else >> unsigned int __nusers; >> __extension__ union >> @@ -116,7 +117,7 @@ typedef union >> short __elision; >> # define __spins d.__espins >> # define __elision d.__elision >> -# define __PTHREAD_MUTEX_HAVE_ELISION 2 >> +# define __PTHREAD_SPINS { 0, 0 } >> } d; >> __pthread_slist_t __list; >> }; >> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >> index deec4da..621895b 100644 >> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >> @@ -94,6 +94,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >> index f11eeab..76b94b4 100644 >> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >> @@ -74,6 +74,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >> index 6f85eae..c633c6d 100644 >> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >> @@ -73,6 +73,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >> index 26edce5..0a0af56 100644 >> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >> @@ -77,6 +77,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >> index b77b80a..49969bb 100644 >> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >> @@ -74,6 +74,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >> index 283f240..b21f5f8 100644 >> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >> @@ -77,6 +77,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >> index ca053e3..d4c5600 100644 >> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >> @@ -76,6 +76,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >> index 9d9386b..712076c 100644 >> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >> @@ -106,6 +106,9 @@ typedef union >> long int __align; >> } pthread_mutex_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> typedef union >> { >> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >> index f469352..17bb112 100644 >> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >> @@ -112,6 +112,9 @@ typedef union >> int __align; >> } pthread_mutexattr_t; >> >> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >> +#define __PTHREAD_SPINS 0 >> + >> >> /* Data structure for conditional variable handling. The structure of >> the attribute type is not exposed on purpose. */ >> > >
Thanks for checking it. I'm pinging again because without this patch or similar hack (define __PTHREAD_MUTEX_HAVE_ELISION somewhere) 'make check' *fails* while building nptl tests on platform that does not define __PTHREAD_MUTEX_HAVE_ELISION. On 04-04-2014 10:09, Stefan Liebler wrote: > Hi, > > this is okay on s390. I have tested the patch with s390/s390x. > Thanks. > > On 04/02/2014 02:35 PM, Adhemerval Zanella wrote: >> Ping for arch maintainers. >> >> On 26-03-2014 17:29, Adhemerval Zanella wrote: >>> On 26-03-2014 14:50, Roland McGrath wrote: >>>> I don't understand what "support for 64 bits" or "support for 32 bits" >>>> means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it >>>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it >>>> is really just about the x86-private layout of pthread_mutex_t. It seems >>>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. >>>> >>>> That can be a separate cleanup if you want, and others may want to kibitz. >>>> But that might involve dropping the header you're adding here, so maybe you >>>> want to just resolve it now. >>>> >>>> If you want to go ahead with this change, then it's OK with the other nits >>>> above and this comment rewritten to describe concretely what the macro >>>> means. In actual usage so far, it doesn't actually mean anything about >>>> elision support per se. It just means something about how the fields of >>>> pthread_mutex_t are structured and hence what the initializer must look like. >>>> If that's all it's for, it should be made clear. >>>> >>> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch >>> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t >>> layout is also arch specific and does not make sense disassociate them. >>> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. >>> >>> -- >>> >>> * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION): >>> Remove macro usage. >>> (__PTHREAD_SPINS): Move definition to ... >>> * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> (__PTHREAD_SPINS): ... here. >>> * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Likewise. >>> >>> ports/ChangeLog.hppa >>> * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >>> (__PTHREAD_SPIN): Moved defintion from pthread.h. >>> >>> --- >>> >>> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h >>> index 1e0c5dc..40a3e21 100644 >>> --- a/nptl/sysdeps/pthread/pthread.h >>> +++ b/nptl/sysdeps/pthread/pthread.h >>> @@ -82,15 +82,6 @@ enum >>> #endif >>> >>> >>> -/* Mutex initializers. */ >>> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */ >>> -#define __PTHREAD_SPINS 0, 0 >>> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */ >>> -#define __PTHREAD_SPINS { 0, 0 } >>> -#else >>> -#define __PTHREAD_SPINS 0 >>> -#endif >>> - >>> #ifdef __PTHREAD_MUTEX_HAVE_PREV >>> # define PTHREAD_MUTEX_INITIALIZER \ >>> { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } >>> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> index 71bd3ae..20e5fc0 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h >>> @@ -106,6 +106,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >>> index 23a1698..716f151 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h >>> @@ -20,8 +20,6 @@ >>> >>> #include <bits/wordsize.h> >>> >>> -# define __PTHREAD_MUTEX_HAVE_ELISION 0 >>> - >>> #if __WORDSIZE == 64 >>> # define __SIZEOF_PTHREAD_ATTR_T 56 >>> # define __SIZEOF_PTHREAD_MUTEX_T 40 >>> @@ -107,6 +105,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >>> index e42d94e..f92958c 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h >>> @@ -77,6 +77,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >>> index be615b6..7b9edf2 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h >>> @@ -106,6 +106,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >>> index 28e5144..0ca10f2 100644 >>> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >>> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h >>> @@ -105,7 +105,8 @@ typedef union >>> short __elision; >>> __pthread_list_t __list; >>> # define __PTHREAD_MUTEX_HAVE_PREV 1 >>> -# define __PTHREAD_MUTEX_HAVE_ELISION 1 >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +# define __PTHREAD_SPINS 0, 0 >>> #else >>> unsigned int __nusers; >>> __extension__ union >>> @@ -116,7 +117,7 @@ typedef union >>> short __elision; >>> # define __spins d.__espins >>> # define __elision d.__elision >>> -# define __PTHREAD_MUTEX_HAVE_ELISION 2 >>> +# define __PTHREAD_SPINS { 0, 0 } >>> } d; >>> __pthread_slist_t __list; >>> }; >>> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >>> index deec4da..621895b 100644 >>> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >>> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h >>> @@ -94,6 +94,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >>> index f11eeab..76b94b4 100644 >>> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h >>> @@ -74,6 +74,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >>> index 6f85eae..c633c6d 100644 >>> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h >>> @@ -73,6 +73,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >>> index 26edce5..0a0af56 100644 >>> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h >>> @@ -77,6 +77,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >>> index b77b80a..49969bb 100644 >>> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h >>> @@ -74,6 +74,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >>> index 283f240..b21f5f8 100644 >>> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h >>> @@ -77,6 +77,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >>> index ca053e3..d4c5600 100644 >>> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h >>> @@ -76,6 +76,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >>> index 9d9386b..712076c 100644 >>> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h >>> @@ -106,6 +106,9 @@ typedef union >>> long int __align; >>> } pthread_mutex_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> typedef union >>> { >>> char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; >>> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >>> index f469352..17bb112 100644 >>> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >>> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h >>> @@ -112,6 +112,9 @@ typedef union >>> int __align; >>> } pthread_mutexattr_t; >>> >>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ >>> +#define __PTHREAD_SPINS 0 >>> + >>> >>> /* Data structure for conditional variable handling. The structure of >>> the attribute type is not exposed on purpose. */ >>> >> >> > >
On 03/26/2014 04:29 PM, Adhemerval Zanella wrote: > On 26-03-2014 14:50, Roland McGrath wrote: >> I don't understand what "support for 64 bits" or "support for 32 bits" >> means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it >> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it >> is really just about the x86-private layout of pthread_mutex_t. It seems >> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. >> >> That can be a separate cleanup if you want, and others may want to kibitz. >> But that might involve dropping the header you're adding here, so maybe you >> want to just resolve it now. >> >> If you want to go ahead with this change, then it's OK with the other nits >> above and this comment rewritten to describe concretely what the macro >> means. In actual usage so far, it doesn't actually mean anything about >> elision support per se. It just means something about how the fields of >> pthread_mutex_t are structured and hence what the initializer must look like. >> If that's all it's for, it should be made clear. >> > Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch > moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t > layout is also arch specific and does not make sense disassociate them. > This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. My opinion is that you should *not* wait for arch maintainers to review this patch. It's a fairly mechanical patch that should not break any configuration and should not cause any undue ABI changes. A simple grep'ing shows you won't have any problems. If you want to be nice send a follow-up email and TO all of the arch maintainers letting them know you made the change. This should cut through the noise and let them test at their own time and pace. I pulled the patch down, applied, and tested with it and it looks good for x86-64, no regressions. OK to checkin. > -- > > * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION): > Remove macro usage. > (__PTHREAD_SPINS): Move definition to ... > * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > (__PTHREAD_SPINS): ... here. > * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. > * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Likewise. OK. These look good to me. > ports/ChangeLog.hppa > * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > (__PTHREAD_SPIN): Moved defintion from pthread.h. OK for hppa. > --- > > diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h > index 1e0c5dc..40a3e21 100644 > --- a/nptl/sysdeps/pthread/pthread.h > +++ b/nptl/sysdeps/pthread/pthread.h > @@ -82,15 +82,6 @@ enum > #endif > > > -/* Mutex initializers. */ > -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */ > -#define __PTHREAD_SPINS 0, 0 > -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */ > -#define __PTHREAD_SPINS { 0, 0 } > -#else > -#define __PTHREAD_SPINS 0 > -#endif > - OK. > #ifdef __PTHREAD_MUTEX_HAVE_PREV > # define PTHREAD_MUTEX_INITIALIZER \ > { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } > diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > index 71bd3ae..20e5fc0 100644 > --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > index 23a1698..716f151 100644 > --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h > @@ -20,8 +20,6 @@ > > #include <bits/wordsize.h> > > -# define __PTHREAD_MUTEX_HAVE_ELISION 0 > - OK. > #if __WORDSIZE == 64 > # define __SIZEOF_PTHREAD_ATTR_T 56 > # define __SIZEOF_PTHREAD_MUTEX_T 40 > @@ -107,6 +105,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > index e42d94e..f92958c 100644 > --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > index be615b6..7b9edf2 100644 > --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > index 28e5144..0ca10f2 100644 > --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h > @@ -105,7 +105,8 @@ typedef union > short __elision; > __pthread_list_t __list; > # define __PTHREAD_MUTEX_HAVE_PREV 1 > -# define __PTHREAD_MUTEX_HAVE_ELISION 1 > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +# define __PTHREAD_SPINS 0, 0 OK. > #else > unsigned int __nusers; > __extension__ union > @@ -116,7 +117,7 @@ typedef union > short __elision; > # define __spins d.__espins > # define __elision d.__elision > -# define __PTHREAD_MUTEX_HAVE_ELISION 2 > +# define __PTHREAD_SPINS { 0, 0 } OK. > } d; > __pthread_slist_t __list; > }; > diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > index deec4da..621895b 100644 > --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h > @@ -94,6 +94,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > index f11eeab..76b94b4 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h > @@ -74,6 +74,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > index 6f85eae..c633c6d 100644 > --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h > @@ -73,6 +73,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > index 26edce5..0a0af56 100644 > --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > index b77b80a..49969bb 100644 > --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h > @@ -74,6 +74,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > index 283f240..b21f5f8 100644 > --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h > @@ -77,6 +77,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > index ca053e3..d4c5600 100644 > --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h > @@ -76,6 +76,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > index 9d9386b..712076c 100644 > --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h > @@ -106,6 +106,9 @@ typedef union > long int __align; > } pthread_mutex_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > typedef union > { > char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; > diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > index f469352..17bb112 100644 > --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h > @@ -112,6 +112,9 @@ typedef union > int __align; > } pthread_mutexattr_t; > > +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ > +#define __PTHREAD_SPINS 0 > + OK. > > /* Data structure for conditional variable handling. The structure of > the attribute type is not exposed on purpose. */ > Cheers, Carlos.
On 08-04-2014 17:30, Carlos O'Donell wrote: > On 03/26/2014 04:29 PM, Adhemerval Zanella wrote: >> On 26-03-2014 14:50, Roland McGrath wrote: >>> I don't understand what "support for 64 bits" or "support for 32 bits" >>> means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it >>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it >>> is really just about the x86-private layout of pthread_mutex_t. It seems >>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. >>> >>> That can be a separate cleanup if you want, and others may want to kibitz. >>> But that might involve dropping the header you're adding here, so maybe you >>> want to just resolve it now. >>> >>> If you want to go ahead with this change, then it's OK with the other nits >>> above and this comment rewritten to describe concretely what the macro >>> means. In actual usage so far, it doesn't actually mean anything about >>> elision support per se. It just means something about how the fields of >>> pthread_mutex_t are structured and hence what the initializer must look like. >>> If that's all it's for, it should be made clear. >>> >> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch >> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t >> layout is also arch specific and does not make sense disassociate them. >> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. > My opinion is that you should *not* wait for arch maintainers to review this > patch. It's a fairly mechanical patch that should not break any configuration > and should not cause any undue ABI changes. A simple grep'ing shows you won't > have any problems. > > If you want to be nice send a follow-up email and TO all of the arch > maintainers letting them know you made the change. This should cut through > the noise and let them test at their own time and pace. > > I pulled the patch down, applied, and tested with it and it looks good > for x86-64, no regressions. > > OK to checkin. > Pushed upstream as 01f8eac224421f07f28f91cc05db7459ea433ea4 and CCing all the arch maintainers. PS: sorry for the spam.
From: Adhemerval Zanella <azanella@linux.vnet.ibm.com> Date: Wed, 09 Apr 2014 11:01:18 -0300 > Pushed upstream as 01f8eac224421f07f28f91cc05db7459ea433ea4 and CCing all the arch > maintainers. > > PS: sorry for the spam. Thanks for taking care of this.
On Wed, 2014-03-26 at 17:29 -0300, Adhemerval Zanella wrote: > On 26-03-2014 14:50, Roland McGrath wrote: > > I don't understand what "support for 64 bits" or "support for 32 bits" > > means. OK, I've looked at bits/pthreadtypes.h so I do understand. But it > > seems pretty wrong to pretend this is a generic 32/64 sort of thing when it > > is really just about the x86-private layout of pthread_mutex_t. It seems > > more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS. > > > > That can be a separate cleanup if you want, and others may want to kibitz. > > But that might involve dropping the header you're adding here, so maybe you > > want to just resolve it now. > > > > If you want to go ahead with this change, then it's OK with the other nits > > above and this comment rewritten to describe concretely what the macro > > means. In actual usage so far, it doesn't actually mean anything about > > elision support per se. It just means something about how the fields of > > pthread_mutex_t are structured and hence what the initializer must look like. > > If that's all it's for, it should be made clear. > > > Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure. This patch > moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t > layout is also arch specific and does not make sense disassociate them. > This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required. I know I'm late with this comment, but I would have preferred if this was called __PTHREAD_MUTEX_SPINS or something like this. It's not a generic spinning-related thing.
diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h index 1e0c5dc..40a3e21 100644 --- a/nptl/sysdeps/pthread/pthread.h +++ b/nptl/sysdeps/pthread/pthread.h @@ -82,15 +82,6 @@ enum #endif -/* Mutex initializers. */ -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout. */ -#define __PTHREAD_SPINS 0, 0 -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout. */ -#define __PTHREAD_SPINS { 0, 0 } -#else -#define __PTHREAD_SPINS 0 -#endif - #ifdef __PTHREAD_MUTEX_HAVE_PREV # define PTHREAD_MUTEX_INITIALIZER \ { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h index 71bd3ae..20e5fc0 100644 --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h @@ -106,6 +106,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h index 23a1698..716f151 100644 --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h @@ -20,8 +20,6 @@ #include <bits/wordsize.h> -# define __PTHREAD_MUTEX_HAVE_ELISION 0 - #if __WORDSIZE == 64 # define __SIZEOF_PTHREAD_ATTR_T 56 # define __SIZEOF_PTHREAD_MUTEX_T 40 @@ -107,6 +105,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h index e42d94e..f92958c 100644 --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h @@ -77,6 +77,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h index be615b6..7b9edf2 100644 --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h @@ -106,6 +106,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h index 28e5144..0ca10f2 100644 --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h @@ -105,7 +105,8 @@ typedef union short __elision; __pthread_list_t __list; # define __PTHREAD_MUTEX_HAVE_PREV 1 -# define __PTHREAD_MUTEX_HAVE_ELISION 1 +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +# define __PTHREAD_SPINS 0, 0 #else unsigned int __nusers; __extension__ union @@ -116,7 +117,7 @@ typedef union short __elision; # define __spins d.__espins # define __elision d.__elision -# define __PTHREAD_MUTEX_HAVE_ELISION 2 +# define __PTHREAD_SPINS { 0, 0 } } d; __pthread_slist_t __list; }; diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h index deec4da..621895b 100644 --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h @@ -94,6 +94,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h index f11eeab..76b94b4 100644 --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h @@ -74,6 +74,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h index 6f85eae..c633c6d 100644 --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h @@ -73,6 +73,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h index 26edce5..0a0af56 100644 --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h @@ -77,6 +77,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h index b77b80a..49969bb 100644 --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h @@ -74,6 +74,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h index 283f240..b21f5f8 100644 --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h @@ -77,6 +77,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h index ca053e3..d4c5600 100644 --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h @@ -76,6 +76,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h index 9d9386b..712076c 100644 --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h @@ -106,6 +106,9 @@ typedef union long int __align; } pthread_mutex_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + typedef union { char __size[__SIZEOF_PTHREAD_MUTEXATTR_T]; diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h index f469352..17bb112 100644 --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h @@ -112,6 +112,9 @@ typedef union int __align; } pthread_mutexattr_t; +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER. */ +#define __PTHREAD_SPINS 0 + /* Data structure for conditional variable handling. The structure of the attribute type is not exposed on purpose. */