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 ACD563857C6B for ; Wed, 30 Sep 2020 12:47:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ACD563857C6B 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 4C1bcl5Jhyz1qsjn; Wed, 30 Sep 2020 14:47:19 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 4C1bcl3ggvz1sM8y; Wed, 30 Sep 2020 14:47:19 +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 2Y1JKSi3kpKx; Wed, 30 Sep 2020 14:47:17 +0200 (CEST) X-Auth-Info: tb1HPJngFNdVoXf8je+Ps+RB2CRg5Vidy0XU4+vJbtg= 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 14:47:17 +0200 (CEST) Date: Wed, 30 Sep 2020 14:47:10 +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 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time Message-ID: <20200930144710.662d9d99@jawa> In-Reply-To: References: <20200919130759.31916-1-lukma@denx.de> <20200919130759.31916-2-lukma@denx.de> 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_/NyAPwIuLqVklGgIW=lsD0pr"; protocol="application/pgp-signature" X-Spam-Status: No, score=-16.5 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 12:47:24 -0000 --Sig_/NyAPwIuLqVklGgIW=lsD0pr Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Adhemerval, > On 19/09/2020 10:07, Lukasz Majewski wrote: > > This is the helper function, which uses struct __timespec64 > > to provide 64 bit absolute time to futex syscalls. > >=20 > > The aim of this function is to move convoluted pre-processor > > macro code from sysdeps/nptl/lowlevellock-futex.h to C > > function in futex-internal.c > >=20 > > The futex_abstimed_wait64 function has been put into a separate > > file on the purpose - to avoid issues apparent on the m68k > > architecture related to small number of available registers (there > > is not enough registers to put all necessary arguments in them if > > the above function would be added to futex-internal.h with > > __always_inline attribute). > >=20 > > Additional precautions for m68k port have been taken - the > > futex-internal.c file will be compiled with -fno-inline flag. =20 >=20 > LGTM with a nit regarding style and a clarification about=20 > '-fno-inline'. Please find the explanation below. >=20 > >=20 > > --- > > Changes for v2: > > - Handle the case when *abstime pointer is NULL > > --- > > sysdeps/nptl/futex-internal.c | 70 > > +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h | > > 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile | 2 + > > 3 files changed, 78 insertions(+) > >=20 > > diff --git a/sysdeps/nptl/futex-internal.c > > b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644 > > --- a/sysdeps/nptl/futex-internal.c > > +++ b/sysdeps/nptl/futex-internal.c > > @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned > > int* futex_word, abstime !=3D NULL ? &ts32 : NULL, > > NULL /* Unused. */, > > FUTEX_BITSET_MATCH_ANY); } > > + > > +static int > > +__futex_abstimed_wait32 (unsigned int* futex_word, > > + unsigned int expected, clockid_t clockid, > > + const struct __timespec64* abstime, > > + int private) > > +{ > > + struct timespec ts32; > > + > > + if (abstime !=3D NULL && ! in_time_t_range (abstime->tv_sec)) > > + return -EOVERFLOW; > > + > > + unsigned int clockbit =3D (clockid =3D=3D CLOCK_REALTIME) ? > > + FUTEX_CLOCK_REALTIME : 0; > > + int op =3D __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, > > private); + > > + if (abstime !=3D NULL) > > + ts32 =3D valid_timespec64_to_timespec (*abstime); > > + > > + return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected, > > + abstime !=3D NULL ? &ts32 : NULL, > > + NULL /* Unused. */, > > FUTEX_BITSET_MATCH_ANY); +} > > #endif > > =20 > > int =20 >=20 > Ok. >=20 > > @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned > > int* futex_word, futex_fatal_error (); > > } > > } > > + > > +int > > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int > > expected, > > + clockid_t clockid, > > + const struct __timespec64* abstime, int > > private) +{ > > + unsigned int clockbit; > > + int err; > > + > > + /* Work around the fact that the kernel rejects negative timeout > > values > > + despite them being valid. */ > > + if (__glibc_unlikely ((abstime !=3D NULL) && (abstime->tv_sec < > > 0))) > > + return ETIMEDOUT; > > + > > + if (! lll_futex_supported_clockid(clockid)) > > + return EINVAL; =20 >=20 > Space before parentheses.=20 Ok. Fixed. >=20 > > + > > + clockbit =3D (clockid =3D=3D CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : > > 0; > > + int op =3D __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, > > private); + > > + err =3D INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, > > expected, > > + abstime, NULL /* Unused. */, > > + FUTEX_BITSET_MATCH_ANY); > > +#ifndef __ASSUME_TIME64_SYSCALLS > > + if (err =3D=3D -ENOSYS) > > + err =3D __futex_abstimed_wait32 (futex_word, expected, > > + clockid, abstime, private); > > +#endif > > + switch (err) > > + { > > + case 0: > > + case -EAGAIN: > > + case -EINTR: > > + case -ETIMEDOUT: > > + return -err; > > + > > + case -EFAULT: /* Must have been caused by a glibc or > > application bug. */ > > + case -EINVAL: /* Either due to wrong alignment, unsupported > > + clockid or due to the timeout not being > > + normalized. Must have been caused by a glibc > > or > > + application bug. */ > > + case -ENOSYS: /* Must have been caused by a glibc bug. */ > > + /* No other errors are documented at this time. */ > > + default: > > + futex_fatal_error (); > > + } > > +} =20 >=20 > Ok. >=20 > > diff --git a/sysdeps/nptl/futex-internal.h > > b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644 > > --- a/sysdeps/nptl/futex-internal.h > > +++ b/sysdeps/nptl/futex-internal.h > > @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned > > int* futex_word, const struct __timespec64* abstime, > > int private) attribute_hidden; > > =20 > > +int > > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int > > expected, > > + clockid_t clockid, > > + const struct __timespec64* abstime, > > + int private) attribute_hidden; > > + > > #endif /* futex-internal.h */ =20 >=20 > Ok. >=20 > > diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile > > b/sysdeps/unix/sysv/linux/m68k/Makefile index > > be40fae68a..65164c5752 100644 --- > > a/sysdeps/unix/sysv/linux/m68k/Makefile +++ > > b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@ > > sysdep-dl-routines +=3D dl-static sysdep-others +=3D lddlibc4 > > install-bin +=3D lddlibc4 > > endif > > + > > +CFLAGS-futex-internal.c +=3D -fno-inline > > =20 >=20 > Do we still need it? Unfortunately, yes. The m68k architecture has the issue with properly compiling this code (in futex-internal.c) as it has very limited number of registers (8 x 32 bit IIRC). I did some investigation and it looks like static inline in_time_t_range() function affects the compiler capabilities to generate correct code. It can be easily inlined on any architecture but m68k. As m68k has issues with this code compilation - it is IMHO better to disable inlining (-fno-inline) on this particular port. As a result we would have slower execution only on relevant functions, but 64 bit time support would be provided.=20 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_/NyAPwIuLqVklGgIW=lsD0pr Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAl90fk4ACgkQAR8vZIA0 zr35rQgAxI1S1Hs/cLO9R7dyMptaP13V5XSYXXlcI30FNSz1mi7deEwSlbEeAB7e kNaUYUOSXM/lIh1MlmKxSEntIeVveFKhR+wTfMMNgh1gK6rYkj6L7/WDmJJxW7f/ 0m8Tf2QL138yufWpWNRn1LIcZBy4a5MCR7flbIfEMF2M9uyM4WHoRDaWC8XPp/eP 2khqlWQXWaoC56lAhXdhiIt7YnkF+1jFbmPxqzL/AGHva1Pyg2pBmv9it2DmERJ9 wKrTp+Rovftwx2pYLbfbBrRr/qTDoapTKhc22vXDlzFZutbuElM7v8Y0UG/H16yW oGPypKE3K2070Z+76y+yjW1Or3riGA== =0GCf -----END PGP SIGNATURE----- --Sig_/NyAPwIuLqVklGgIW=lsD0pr--