From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-out.m-online.net (mail-out.m-online.net [212.18.0.9]) by sourceware.org (Postfix) with ESMTPS id 6521C3986804 for ; Wed, 30 Sep 2020 13:12:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6521C3986804 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=lukma@denx.de Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 4C1c9S2wyCz1qs3X; Wed, 30 Sep 2020 15:12:12 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 4C1c9S1NrXz1sM8t; Wed, 30 Sep 2020 15:12:12 +0200 (CEST) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new, port 10024) with ESMTP id EfoWFL4XEhmC; Wed, 30 Sep 2020 15:12:09 +0200 (CEST) X-Auth-Info: dk/Lp9v3lkxvyD8JBs8wKEebcnT8ExcUDRAsS1J+26w= Received: from jawa (85-222-111-42.dynamic.chello.pl [85.222.111.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA; Wed, 30 Sep 2020 15:12:09 +0200 (CEST) Date: Wed, 30 Sep 2020 15:12:01 +0200 From: Lukasz Majewski To: Adhemerval Zanella Cc: Joseph Myers , Paul Eggert , Alistair Francis , Arnd Bergmann , Alistair Francis , GNU C Library , Florian Weimer , Carlos O'Donell , Stepan Golosunov , Andreas Schwab , Zack Weinberg , Jeff Law Subject: Re: [PATCH v2 3/3] y2038: nptl: Convert pthread_rwlock_{clock|timed}{rd|wr}lock to support 64 bit time Message-ID: <20200930151201.19522e3b@jawa> In-Reply-To: <55d2e158-d3df-2c3e-757c-5673d436d4a2@linaro.org> References: <20200919130759.31916-1-lukma@denx.de> <20200919130759.31916-3-lukma@denx.de> <55d2e158-d3df-2c3e-757c-5673d436d4a2@linaro.org> Organization: denx.de X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; boundary="Sig_/nOG469cjsGYt024D/dWSMr3"; protocol="application/pgp-signature" X-Spam-Status: No, score=-16.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Sep 2020 13:12:15 -0000 --Sig_/nOG469cjsGYt024D/dWSMr3 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Adhemerval, > On 19/09/2020 10:07, Lukasz Majewski wrote: > > The pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock, > > pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock have been > > converted to support 64 bit time. > >=20 > > This change uses new futex_abstimed_wait64 function in > > ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where > > possible. > >=20 > > The pthread_rwlock_{clock|timed}{rd|wr}lock only accepts absolute > > time. Moreover, there is no need to check for NULL passed as > > *abstime pointer to the syscalls as those calls have exported > > symbols marked with __nonull attribute for abstime. > >=20 > > For systems with __TIMESIZE !=3D 64 && __WORDSIZE =3D=3D 32: > > - Conversions between 64 bit time to 32 bit are necessary > > - Redirection to pthread_rwlock_{clock|timed}{rd|wr}lock will > > provide support for 64 bit time > >=20 > > Build tests: > > ./src/scripts/build-many-glibcs.py glibcs > >=20 > > Run-time tests: > > - Run specific tests on ARM/x86 32bit systems (qemu): > > https://github.com/lmajewski/meta-y2038 and run tests: > > https://github.com/lmajewski/y2038-tests/commits/master > >=20 > > Above tests were performed with Y2038 redirection applied as well > > as without to test the proper usage of both > > __pthread_rwlock_{clock|timed}{rd|wr}lock64 and > > __pthread_rwlock_{clock|timed}{rd|wr}lock. > >=20 > > Reviewed-by: Alistair Francis =20 >=20 > Looks good, there is only a issue regarding LIBC_PROBE (below). >=20 >=20 > >=20 > > --- > > Changes for v2: > > - None > > --- > > nptl/pthreadP.h | 18 +++++++++++++++++ > > nptl/pthread_rwlock_clockrdlock.c | 19 +++++++++++++++--- > > nptl/pthread_rwlock_clockwrlock.c | 19 +++++++++++++++--- > > nptl/pthread_rwlock_common.c | 33 > > +++++++++++++++---------------- nptl/pthread_rwlock_rdlock.c | > > 2 +- nptl/pthread_rwlock_timedrdlock.c | 19 +++++++++++++++--- > > nptl/pthread_rwlock_timedwrlock.c | 19 +++++++++++++++--- > > nptl/pthread_rwlock_wrlock.c | 2 +- > > 8 files changed, 100 insertions(+), 31 deletions(-) > >=20 > > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > > index 9bb44c8535..5bcc8a2db5 100644 > > --- a/nptl/pthreadP.h > > +++ b/nptl/pthreadP.h > > @@ -464,6 +464,10 @@ extern int __pthread_cond_wait (pthread_cond_t > > *cond, pthread_mutex_t *mutex); # define __pthread_timedjoin_np64 > > __pthread_timedjoin_np # define __pthread_cond_timedwait64 > > __pthread_cond_timedwait # define __pthread_cond_clockwait64 > > __pthread_cond_clockwait +# define __pthread_rwlock_clockrdlock64 > > __pthread_rwlock_clockrdlock +# define > > __pthread_rwlock_clockwrlock64 __pthread_rwlock_clockwrlock +# > > define __pthread_rwlock_timedrdlock64 __pthread_rwlock_timedrdlock > > +# define __pthread_rwlock_timedwrlock64 > > __pthread_rwlock_timedwrlock #else extern int > > __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, > > clockid_t clockid, @@ -481,6 +485,20 @@ extern int > > __pthread_cond_clockwait64 (pthread_cond_t *cond, clockid_t clockid, > > const struct __timespec64 > > *abstime); libpthread_hidden_proto (__pthread_cond_clockwait64) > > +extern int __pthread_rwlock_clockrdlock64 (pthread_rwlock_t > > *rwlock, > > + clockid_t clockid, > > + const struct > > __timespec64 *abstime); +libpthread_hidden_proto > > (__pthread_rwlock_clockrdlock64) +extern int > > __pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock, > > + clockid_t clockid, > > + const struct > > __timespec64 *abstime); +libpthread_hidden_proto > > (__pthread_rwlock_clockwrlock64) +extern int > > __pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock, > > + const struct > > __timespec64 *abstime); +libpthread_hidden_proto > > (__pthread_rwlock_timedrdlock64) +extern int > > __pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock, > > + const struct > > __timespec64 *abstime); +libpthread_hidden_proto > > (__pthread_rwlock_timedwrlock64) #endif > > =20 > > extern int __pthread_cond_timedwait (pthread_cond_t *cond, =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_clockrdlock.c > > b/nptl/pthread_rwlock_clockrdlock.c index 4cedfd1dcd..d93b133c9e > > 100644 --- a/nptl/pthread_rwlock_clockrdlock.c > > +++ b/nptl/pthread_rwlock_clockrdlock.c > > @@ -21,8 +21,21 @@ > > =20 > > /* See pthread_rwlock_common.c. */ > > int > > -pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t > > clockid, > > - const struct timespec *abstime) > > +__pthread_rwlock_clockrdlock64 (pthread_rwlock_t *rwlock, > > clockid_t clockid, > > + const struct __timespec64 *abstime) > > { > > - return __pthread_rwlock_rdlock_full (rwlock, clockid, abstime); > > + return __pthread_rwlock_rdlock_full64 (rwlock, clockid, abstime); > > } > > + > > +#if __TIMESIZE !=3D 64 > > +libpthread_hidden_def (__pthread_rwlock_clockrdlock64) =20 >=20 > Maybe an extra space here? Ok. Fixed. >=20 > > +int > > +__pthread_rwlock_clockrdlock (pthread_rwlock_t *rwlock, clockid_t > > clockid, > > + const struct timespec *abstime) > > +{ > > + struct __timespec64 ts64 =3D valid_timespec_to_timespec64 > > (*abstime); + > > + return __pthread_rwlock_clockrdlock64 (rwlock, clockid, &ts64); > > +} > > +#endif > > +weak_alias (__pthread_rwlock_clockrdlock, > > pthread_rwlock_clockrdlock) =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_clockwrlock.c > > b/nptl/pthread_rwlock_clockwrlock.c index 7a954cf529..10a314c76d > > 100644 --- a/nptl/pthread_rwlock_clockwrlock.c > > +++ b/nptl/pthread_rwlock_clockwrlock.c > > @@ -21,8 +21,21 @@ > > =20 > > /* See pthread_rwlock_common.c. */ > > int > > -pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t > > clockid, > > - const struct timespec *abstime) > > +__pthread_rwlock_clockwrlock64 (pthread_rwlock_t *rwlock, > > clockid_t clockid, > > + const struct __timespec64 *abstime) > > { > > - return __pthread_rwlock_wrlock_full (rwlock, clockid, abstime); > > + return __pthread_rwlock_wrlock_full64 (rwlock, clockid, abstime); > > } > > + =20 >=20 > Ok. >=20 > > +#if __TIMESIZE !=3D 64 > > +libpthread_hidden_def (__pthread_rwlock_clockwrlock64) =20 >=20 > Maybe an extra space here? Ok. Fixed. >=20 > > +int > > +__pthread_rwlock_clockwrlock (pthread_rwlock_t *rwlock, clockid_t > > clockid, > > + const struct timespec *abstime) > > +{ > > + struct __timespec64 ts64 =3D valid_timespec_to_timespec64 > > (*abstime); + > > + return __pthread_rwlock_clockwrlock64 (rwlock, clockid, &ts64); > > +} > > +#endif > > +weak_alias (__pthread_rwlock_clockwrlock, > > pthread_rwlock_clockwrlock) =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_common.c > > b/nptl/pthread_rwlock_common.c index 3fbc66ded2..4c9f582d3d 100644 > > --- a/nptl/pthread_rwlock_common.c > > +++ b/nptl/pthread_rwlock_common.c > > @@ -278,9 +278,8 @@ __pthread_rwlock_rdunlock (pthread_rwlock_t > > *rwlock)=20 > > =20 > > static __always_inline int > > -__pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock, > > - clockid_t clockid, > > - const struct timespec *abstime) > > +__pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, > > clockid_t clockid, > > + const struct __timespec64 *abstime) > > { > > unsigned int r; > > =20 >=20 > Ok. >=20 > > @@ -330,8 +329,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t > > *rwlock, & PTHREAD_RWLOCK_RWAITING) !=3D 0) > > { > > int private =3D __pthread_rwlock_get_private > > (rwlock); > > - int err =3D futex_abstimed_wait > > (&rwlock->__data.__readers, > > - r, clockid, > > abstime, private); > > + int err =3D __futex_abstimed_wait64 > > (&rwlock->__data.__readers, > > + r, clockid, > > abstime, > > + private); > > /* We ignore EAGAIN and EINTR. On time-outs, we > > can just return because we don't need to clean up anything. */ > > if (err =3D=3D ETIMEDOUT) =20 >=20 > Ok. >=20 > > @@ -457,9 +457,9 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t > > *rwlock, (&rwlock->__data.__wrphase_futex, > > &wpf, wpf | PTHREAD_RWLOCK_FUTEX_USED))) > > continue; > > - int err =3D futex_abstimed_wait > > (&rwlock->__data.__wrphase_futex, > > - 1 | > > PTHREAD_RWLOCK_FUTEX_USED, > > - clockid, abstime, > > private); > > + int err =3D __futex_abstimed_wait64 > > (&rwlock->__data.__wrphase_futex, > > + 1 | > > PTHREAD_RWLOCK_FUTEX_USED, > > + clockid, abstime, > > private); if (err =3D=3D ETIMEDOUT) > > { > > /* If we timed out, we need to unregister. If no > > read phase =20 >=20 > Ok. >=20 > > @@ -585,9 +585,8 @@ __pthread_rwlock_wrunlock (pthread_rwlock_t > > *rwlock)=20 > > =20 > > static __always_inline int > > -__pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock, > > - clockid_t clockid, > > - const struct timespec *abstime) > > +__pthread_rwlock_wrlock_full64 (pthread_rwlock_t *rwlock, > > clockid_t clockid, > > + const struct __timespec64 *abstime) > > { > > /* Make sure any passed in clockid and timeout value are valid. > > Note that the previous implementation assumed that this check > > *must* not be @@ -728,9 +727,9 @@ __pthread_rwlock_wrlock_full > > (pthread_rwlock_t *rwlock, share the flag, and another writer will > > wake one of the writers in this group. */ > > may_share_futex_used_flag =3D true; > > - int err =3D futex_abstimed_wait > > (&rwlock->__data.__writers_futex, > > - 1 | > > PTHREAD_RWLOCK_FUTEX_USED, > > - clockid, abstime, > > private); > > + int err =3D __futex_abstimed_wait64 > > (&rwlock->__data.__writers_futex, > > + 1 | > > PTHREAD_RWLOCK_FUTEX_USED, > > + clockid, abstime, > > private); if (err =3D=3D ETIMEDOUT) > > { > > if (prefer_writer) =20 >=20 > Ok. >=20 > > @@ -827,9 +826,9 @@ __pthread_rwlock_wrlock_full (pthread_rwlock_t > > *rwlock, (&rwlock->__data.__wrphase_futex, &wpf, > > PTHREAD_RWLOCK_FUTEX_USED))) > > continue; > > - int err =3D futex_abstimed_wait > > (&rwlock->__data.__wrphase_futex, > > - PTHREAD_RWLOCK_FUTEX_USED, > > - clockid, abstime, > > private); > > + int err =3D __futex_abstimed_wait64 > > (&rwlock->__data.__wrphase_futex, > > + > > PTHREAD_RWLOCK_FUTEX_USED, > > + clockid, abstime, > > private); if (err =3D=3D ETIMEDOUT) > > { > > if (rwlock->__data.__flags !=3D > > PTHREAD_RWLOCK_PREFER_READER_NP) =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_rdlock.c > > b/nptl/pthread_rwlock_rdlock.c index 04cecad395..2b8509a49c 100644 > > --- a/nptl/pthread_rwlock_rdlock.c > > +++ b/nptl/pthread_rwlock_rdlock.c > > @@ -24,7 +24,7 @@ __pthread_rwlock_rdlock (pthread_rwlock_t *rwlock) > > { > > LIBC_PROBE (rdlock_entry, 1, rwlock); =20 >=20 > We need to move the systemtap probe to the 64-bit variant, so the > 64-bit time will still trigger it. Those are syscalls, which don't require timespec time. The LIBC_PROBE() macros were there before, so I left them as is and only changed __pthread_rwlock_rdlock_full() to __pthread_rwlock_rdlock_full64(). On the other hand - for example: __pthread_rwlock_clockwrlock() and __pthread_rwlock_clockwrlock64() are only calling __pthread_rwlock_wrlock_full64(), and there were no LIBC_PROBE() macros before. Do I understand correctly that you propose to add LIBC_PROBE() macros to e.g. __pthread_rwlock_clockwrlock64() ? >=20 > > =20 > > - int result =3D __pthread_rwlock_rdlock_full (rwlock, > > CLOCK_REALTIME, NULL); > > + int result =3D __pthread_rwlock_rdlock_full64 (rwlock, > > CLOCK_REALTIME, NULL); LIBC_PROBE (rdlock_acquire_read, 1, rwlock); > > return result; > > } =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_timedrdlock.c > > b/nptl/pthread_rwlock_timedrdlock.c index c5d8aee909..d5999a1419 > > 100644 --- a/nptl/pthread_rwlock_timedrdlock.c > > +++ b/nptl/pthread_rwlock_timedrdlock.c > > @@ -20,8 +20,21 @@ > > =20 > > /* See pthread_rwlock_common.c. */ > > int > > -pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock, > > - const struct timespec *abstime) > > +__pthread_rwlock_timedrdlock64 (pthread_rwlock_t *rwlock, > > + const struct __timespec64 *abstime) > > { > > - return __pthread_rwlock_rdlock_full (rwlock, CLOCK_REALTIME, > > abstime); > > + return __pthread_rwlock_rdlock_full64 (rwlock, CLOCK_REALTIME, > > abstime); } > > + =20 >=20 > Ok. >=20 > > +#if __TIMESIZE !=3D 64 > > +libpthread_hidden_def (__pthread_rwlock_timedrdlock64) =20 >=20 > Maybe an extra space here? Ok. Fixed. >=20 > > +int > > +__pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock, > > + const struct timespec *abstime) > > +{ > > + struct __timespec64 ts64 =3D valid_timespec_to_timespec64 > > (*abstime); + > > + return __pthread_rwlock_timedrdlock64 (rwlock, &ts64); > > +} > > +#endif > > +weak_alias (__pthread_rwlock_timedrdlock, > > pthread_rwlock_timedrdlock) =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_timedwrlock.c > > b/nptl/pthread_rwlock_timedwrlock.c index ccee8b77d9..6c5284a172 > > 100644 --- a/nptl/pthread_rwlock_timedwrlock.c > > +++ b/nptl/pthread_rwlock_timedwrlock.c > > @@ -20,8 +20,21 @@ > > =20 > > /* See pthread_rwlock_common.c. */ > > int > > -pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock, > > - const struct timespec *abstime) > > +__pthread_rwlock_timedwrlock64 (pthread_rwlock_t *rwlock, > > + const struct __timespec64 *abstime) > > { > > - return __pthread_rwlock_wrlock_full (rwlock, CLOCK_REALTIME, > > abstime); > > + return __pthread_rwlock_wrlock_full64 (rwlock, CLOCK_REALTIME, > > abstime); } > > + > > +#if __TIMESIZE !=3D 64 > > +libpthread_hidden_def (__pthread_rwlock_timedwrlock64) =20 >=20 >=20 > Maybe an extra space here? Ok. Fixed. >=20 > > +int > > +__pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock, > > + const struct timespec *abstime) > > +{ > > + struct __timespec64 ts64 =3D valid_timespec_to_timespec64 > > (*abstime); + > > + return __pthread_rwlock_timedwrlock64 (rwlock, &ts64); > > +} > > +#endif > > +weak_alias (__pthread_rwlock_timedwrlock, > > pthread_rwlock_timedwrlock) =20 >=20 > Ok. >=20 > > diff --git a/nptl/pthread_rwlock_wrlock.c > > b/nptl/pthread_rwlock_wrlock.c index a42aa626f0..210e6cffdc 100644 > > --- a/nptl/pthread_rwlock_wrlock.c > > +++ b/nptl/pthread_rwlock_wrlock.c > > @@ -24,7 +24,7 @@ __pthread_rwlock_wrlock (pthread_rwlock_t *rwlock) > > { > > LIBC_PROBE (wrlock_entry, 1, rwlock); =20 >=20 > We need to move the systemtap probe to the 64-bit variant, so the > 64-bit time will still trigger it. The same comment as above apply here. >=20 > > =20 > > - int result =3D __pthread_rwlock_wrlock_full (rwlock, > > CLOCK_REALTIME, NULL); > > + int result =3D __pthread_rwlock_wrlock_full64 (rwlock, > > CLOCK_REALTIME, NULL); LIBC_PROBE (wrlock_acquire_write, 1, rwlock); > > return result; > > } > > =20 >=20 > Ok. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de --Sig_/nOG469cjsGYt024D/dWSMr3 Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAl90hCEACgkQAR8vZIA0 zr0ygAgAo/S/B9rZTSnNHMyc36Tio5d7B6Ne7aMfsZb3V6XK/IThwOrr+GFYsmhZ y8GeaoNUJ8nTln0YbSpyhbtt8nX1vAGzGgyrRDK8TUkFr27+Cr5eEAWrnvcLye/D W2cZcqB5CkyNgPWuwlHk18kVh5hbXnCnx6zcdlg2EUbUquZtJKa+mg85xDMvpXkO FILbz3lHAiNIYcPjaLLviNm2yTXr80co8qukhIAD+j837kRcvAmo8IhGcTPoNbNS CDkX1JQoXLMmYDFiE1px/VwfvEi7dVN12WvY9h1d5v37E74l6rl3rO2DbJE+FpeN oxNMeTnxig9TbhOB4rPxCPwK/KzIIg== =c4H5 -----END PGP SIGNATURE----- --Sig_/nOG469cjsGYt024D/dWSMr3--