diff mbox

Get rid of __LT_SPINLOCK_INIT

Message ID 20140317135545.GM1850@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar March 17, 2014, 1:55 p.m. UTC 
  Hi,

We got rid of LinuxThreads in 2005, but we didn't remove
__LT_SPINLOCK_INIT back then.  Do it now.

Siddhesh

	* nptl/sysdeps/pthread/bits/libc-lockP.h [defined NOT_IN_libc
	&& !defined IS_IN_libpthread && __LT_SPINNOCK_INIT != 0]:
	Remove.

Comments

Adhemerval Zanella Netto March 17, 2014, 1:58 p.m. UTC | #1 
On 17-03-2014 10:55, Siddhesh Poyarekar wrote:
> Hi,
>
> We got rid of LinuxThreads in 2005, but we didn't remove
> __LT_SPINLOCK_INIT back then.  Do it now.
>
> Siddhesh
>
> 	* nptl/sysdeps/pthread/bits/libc-lockP.h [defined NOT_IN_libc
> 	&& !defined IS_IN_libpthread && __LT_SPINNOCK_INIT != 0]:
> 	Remove.
>
> diff --git a/nptl/sysdeps/pthread/bits/libc-lockP.h b/nptl/sysdeps/pthread/bits/libc-lockP.h
> index bacc678..ec20271 100644
> --- a/nptl/sysdeps/pthread/bits/libc-lockP.h
> +++ b/nptl/sysdeps/pthread/bits/libc-lockP.h
> @@ -78,13 +78,8 @@ typedef pthread_key_t __libc_key_t;
>    CLASS __libc_lock_t NAME = LLL_LOCK_INITIALIZER;
>  # endif
>  #else
> -# if __LT_SPINLOCK_INIT == 0
> -#  define __libc_lock_define_initialized(CLASS,NAME) \
> +# define __libc_lock_define_initialized(CLASS,NAME) \
>    CLASS __libc_lock_t NAME;
> -# else
> -#  define __libc_lock_define_initialized(CLASS,NAME) \
> -  CLASS __libc_lock_t NAME = PTHREAD_MUTEX_INITIALIZER;
> -# endif
>  #endif
>  
>  #define __libc_rwlock_define_initialized(CLASS,NAME) \
I'm ok with this patch (In fact I was about to send this patch too).
Will Newton March 17, 2014, 3:18 p.m. UTC | #2 
On 17 March 2014 13:55, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:

Hi Siddesh,

> We got rid of LinuxThreads in 2005, but we didn't remove
> __LT_SPINLOCK_INIT back then.  Do it now.

This looks straightforward, however it seems the only arch that ever
set __LT_SPINLOCK_INIT was hppa, so I assume this conditional was
meant once upon a time to do something for hppa. I guess the patch is
fine (as it does not change existing behaviour) but maybe someone who
cares about hppa should verify that something isn't needed for that
arch here.
Adhemerval Zanella Netto March 17, 2014, 3:31 p.m. UTC | #3 
On 17-03-2014 12:18, Will Newton wrote:
> On 17 March 2014 13:55, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
>
> Hi Siddesh,
>
>> We got rid of LinuxThreads in 2005, but we didn't remove
>> __LT_SPINLOCK_INIT back then.  Do it now.
> This looks straightforward, however it seems the only arch that ever
> set __LT_SPINLOCK_INIT was hppa, so I assume this conditional was
> meant once upon a time to do something for hppa. I guess the patch is
> fine (as it does not change existing behaviour) but maybe someone who
> cares about hppa should verify that something isn't needed for that
> arch here.
>
Carlos O'Donnel is the maintainer for hppa. However, if you ever check on 
GLIBC (a simple grep), you will see __LT_SPINLOCK_INIT is never defined in
any arch config file and it was only defined by when glibc used linuxpthread.
So I think it safe to just get rid of it.
Siddhesh Poyarekar March 17, 2014, 3:39 p.m. UTC | #4 
On Mon, Mar 17, 2014 at 12:31:18PM -0300, Adhemerval Zanella wrote:
> Carlos O'Donnel is the maintainer for hppa. However, if you ever check on 
> GLIBC (a simple grep), you will see __LT_SPINLOCK_INIT is never defined in
> any arch config file and it was only defined by when glibc used linuxpthread.
> So I think it safe to just get rid of it.

That's what I did, in addition to trying to figure out when it got
added in, which led me to the hppa bits too.

Siddhesh
Carlos O'Donell March 17, 2014, 10:35 p.m. UTC | #5 
On 03/17/2014 11:39 AM, Siddhesh Poyarekar wrote:
> On Mon, Mar 17, 2014 at 12:31:18PM -0300, Adhemerval Zanella wrote:
>> Carlos O'Donnel is the maintainer for hppa. However, if you ever check on 
>> GLIBC (a simple grep), you will see __LT_SPINLOCK_INIT is never defined in
>> any arch config file and it was only defined by when glibc used linuxpthread.
>> So I think it safe to just get rid of it.
> 
> That's what I did, in addition to trying to figure out when it got
> added in, which led me to the hppa bits too.

The hppa port no longer needs __LT_SPINLOCK_INIT. As of NPTL we switched
to zero-initialized locks to avoid the terrible headache of teaching
developers that calloc() is not the right way to initialize a mutex,
despite it working on x86*.

We used to have one-initialized locks because hppa's only atomic operation
is load-and-clear-word, and because of that we had our own special
initialization in LinuxThreads.

Cheers,
Carlos.
diff mbox

Patch

diff --git a/nptl/sysdeps/pthread/bits/libc-lockP.h b/nptl/sysdeps/pthread/bits/libc-lockP.h
index bacc678..ec20271 100644
--- a/nptl/sysdeps/pthread/bits/libc-lockP.h
+++ b/nptl/sysdeps/pthread/bits/libc-lockP.h
@@ -78,13 +78,8 @@  typedef pthread_key_t __libc_key_t;
   CLASS __libc_lock_t NAME = LLL_LOCK_INITIALIZER;
 # endif
 #else
-# if __LT_SPINLOCK_INIT == 0
-#  define __libc_lock_define_initialized(CLASS,NAME) \
+# define __libc_lock_define_initialized(CLASS,NAME) \
   CLASS __libc_lock_t NAME;
-# else
-#  define __libc_lock_define_initialized(CLASS,NAME) \
-  CLASS __libc_lock_t NAME = PTHREAD_MUTEX_INITIALIZER;
-# endif
 #endif
 
 #define __libc_rwlock_define_initialized(CLASS,NAME) \