From patchwork Wed Feb 8 14:49:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Liebler X-Patchwork-Id: 19179 Received: (qmail 25968 invoked by alias); 8 Feb 2017 14:49:48 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 25869 invoked by uid 89); 8 Feb 2017 14:49:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=bye, D*de.ibm.com, S390, uintmax_t X-HELO: mx0a-001b2d01.pphosted.com Subject: Re: [PATCH 2/2] S390: Use generic spinlock code. To: libc-alpha@sourceware.org References: <1481905917-15654-1-git-send-email-stli@linux.vnet.ibm.com> <1481905917-15654-2-git-send-email-stli@linux.vnet.ibm.com> From: Stefan Liebler Date: Wed, 8 Feb 2017 15:49:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1481905917-15654-2-git-send-email-stli@linux.vnet.ibm.com> X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17020814-0032-0000-0000-00000719790D X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17020814-0033-0000-0000-0000235B75AE Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-02-08_10:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=4 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702080145 This is an updated version of the patch, which adjusts the s390 specific atomic-macros in the same way as in include/atomic.h. Thus passing a volatile int pointer is fine, too. Okay to commit? Bye. Stefan ChangeLog: * sysdeps/s390/atomic-machine.h: (__arch_compare_and_exchange_val_32_acq): Cast type to omit volatile qualifier. (__arch_compare_and_exchange_val_64_acq): Likewise. (atomic_exchange_acq): Likewise. * sysdeps/s390/nptl/pthread_spin_init.c: Delete File. * sysdeps/s390/nptl/pthread_spin_unlock.c: Likewise. * sysdeps/s390/nptl/pthread_spin_lock.c: (SPIN_LOCK_READS_BETWEEN_CMPXCHG): New define. Use generic spinlock code. * sysdeps/s390/nptl/pthread_spin_trylock.c: (SPIN_TRYLOCK_USE_CMPXCHG_INSTEAD_OF_XCHG): New define. Use generic spinlock code. commit 472bccee286f2f40522b8f670c9b54b7ef58425c Author: Stefan Liebler Date: Wed Feb 8 15:24:14 2017 +0100 S390: Use generic spinlock code. This patch removes the s390 specific implementation of spinlock code and is now using the generic one. For pthread_spin_trylock an explicit load and test before executing compare and swap instruction is done as it is an interlock update even if the lock is already acquired. The macros in s390 specific atomic-machine.h are aligned in order to omit the storing to and loading from stack. ChangeLog: * sysdeps/s390/atomic-machine.h: (__arch_compare_and_exchange_val_32_acq): Cast type to omit volatile qualifier. (__arch_compare_and_exchange_val_64_acq): Likewise. (atomic_exchange_acq): Likewise. * sysdeps/s390/nptl/pthread_spin_init.c: Delete File. * sysdeps/s390/nptl/pthread_spin_unlock.c: Likewise. * sysdeps/s390/nptl/pthread_spin_lock.c: (SPIN_LOCK_READS_BETWEEN_CMPXCHG): New define. Use generic spinlock code. * sysdeps/s390/nptl/pthread_spin_trylock.c: (SPIN_TRYLOCK_USE_CMPXCHG_INSTEAD_OF_XCHG): New define. Use generic spinlock code. diff --git a/sysdeps/s390/atomic-machine.h b/sysdeps/s390/atomic-machine.h index 211d3d6..d98c836 100644 --- a/sysdeps/s390/atomic-machine.h +++ b/sysdeps/s390/atomic-machine.h @@ -54,7 +54,7 @@ typedef uintmax_t uatomic_max_t; #define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval) \ ({ __typeof (mem) __archmem = (mem); \ - __typeof (*mem) __archold = (oldval); \ + __typeof ((__typeof (*(mem))) *(mem)) __archold = (oldval); \ __asm__ __volatile__ ("cs %0,%2,%1" \ : "+d" (__archold), "=Q" (*__archmem) \ : "d" (newval), "m" (*__archmem) : "cc", "memory" ); \ @@ -64,7 +64,7 @@ typedef uintmax_t uatomic_max_t; # define __HAVE_64B_ATOMICS 1 # define __arch_compare_and_exchange_val_64_acq(mem, newval, oldval) \ ({ __typeof (mem) __archmem = (mem); \ - __typeof (*mem) __archold = (oldval); \ + __typeof ((__typeof (*(mem))) *(mem)) __archold = (oldval); \ __asm__ __volatile__ ("csg %0,%2,%1" \ : "+d" (__archold), "=Q" (*__archmem) \ : "d" ((long) (newval)), "m" (*__archmem) : "cc", "memory" ); \ @@ -86,8 +86,8 @@ typedef uintmax_t uatomic_max_t; #ifdef __s390x__ # define atomic_exchange_acq(mem, newvalue) \ ({ __typeof (mem) __atg5_memp = (mem); \ - __typeof (*(mem)) __atg5_oldval = *__atg5_memp; \ - __typeof (*(mem)) __atg5_value = (newvalue); \ + __typeof ((__typeof (*(mem))) *(mem)) __atg5_oldval = *__atg5_memp; \ + __typeof ((__typeof (*(mem))) *(mem)) __atg5_value = (newvalue); \ if (sizeof (*mem) == 4) \ __asm__ __volatile__ ("0: cs %0,%2,%1\n" \ " jl 0b" \ @@ -106,8 +106,8 @@ typedef uintmax_t uatomic_max_t; #else # define atomic_exchange_acq(mem, newvalue) \ ({ __typeof (mem) __atg5_memp = (mem); \ - __typeof (*(mem)) __atg5_oldval = *__atg5_memp; \ - __typeof (*(mem)) __atg5_value = (newvalue); \ + __typeof ((__typeof (*(mem))) *(mem)) __atg5_oldval = *__atg5_memp; \ + __typeof ((__typeof (*(mem))) *(mem)) __atg5_value = (newvalue); \ if (sizeof (*mem) == 4) \ __asm__ __volatile__ ("0: cs %0,%2,%1\n" \ " jl 0b" \ diff --git a/sysdeps/s390/nptl/pthread_spin_init.c b/sysdeps/s390/nptl/pthread_spin_init.c deleted file mode 100644 index d826871..0000000 --- a/sysdeps/s390/nptl/pthread_spin_init.c +++ /dev/null @@ -1,19 +0,0 @@ -/* Copyright (C) 2003-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Martin Schwidefsky , 2003. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -/* Not needed. pthread_spin_init is an alias for pthread_spin_unlock. */ diff --git a/sysdeps/s390/nptl/pthread_spin_lock.c b/sysdeps/s390/nptl/pthread_spin_lock.c index 7349940..39d2926 100644 --- a/sysdeps/s390/nptl/pthread_spin_lock.c +++ b/sysdeps/s390/nptl/pthread_spin_lock.c @@ -16,17 +16,9 @@ License along with the GNU C Library; if not, see . */ -#include "pthreadP.h" +#define SPIN_LOCK_READS_BETWEEN_CMPXCHG 1000 -int -pthread_spin_lock (pthread_spinlock_t *lock) -{ - int oldval; - - __asm__ __volatile__ ("0: lhi %0,0\n" - " cs %0,%2,%1\n" - " jl 0b" - : "=&d" (oldval), "=Q" (*lock) - : "d" (1), "m" (*lock) : "cc" ); - return 0; -} +/* We can't use the normal "#include " because + it will resolve to this very file. Using "sysdeps/.." as reference to the + top level directory does the job. */ +#include diff --git a/sysdeps/s390/nptl/pthread_spin_trylock.c b/sysdeps/s390/nptl/pthread_spin_trylock.c index 0e848da..f3937b5 100644 --- a/sysdeps/s390/nptl/pthread_spin_trylock.c +++ b/sysdeps/s390/nptl/pthread_spin_trylock.c @@ -16,17 +16,9 @@ License along with the GNU C Library; if not, see . */ -#include -#include "pthreadP.h" +#define SPIN_TRYLOCK_USE_CMPXCHG_INSTEAD_OF_XCHG 1 -int -pthread_spin_trylock (pthread_spinlock_t *lock) -{ - int old; - - __asm__ __volatile__ ("cs %0,%3,%1" - : "=d" (old), "=Q" (*lock) - : "0" (0), "d" (1), "m" (*lock) : "cc" ); - - return old != 0 ? EBUSY : 0; -} +/* We can't use the normal "#include " because + it will resolve to this very file. Using "sysdeps/.." as reference to the + top level directory does the job. */ +#include diff --git a/sysdeps/s390/nptl/pthread_spin_unlock.c b/sysdeps/s390/nptl/pthread_spin_unlock.c deleted file mode 100644 index 54e7378..0000000 --- a/sysdeps/s390/nptl/pthread_spin_unlock.c +++ /dev/null @@ -1,32 +0,0 @@ -/* Copyright (C) 2003-2017 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Martin Schwidefsky , 2003. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -/* Ugly hack to avoid the declaration of pthread_spin_init. */ -#define pthread_spin_init pthread_spin_init_XXX -#include "pthreadP.h" -#undef pthread_spin_init - -int -pthread_spin_unlock (pthread_spinlock_t *lock) -{ - __asm__ __volatile__ (" xc %O0(4,%R0),%0\n" - " bcr 15,0" - : "=Q" (*lock) : "m" (*lock) : "cc" ); - return 0; -} -strong_alias (pthread_spin_unlock, pthread_spin_init)