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 A69273987930 for ; Wed, 30 Sep 2020 13:40:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A69273987930 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 4C1cnc4XCXz1qs3X; Wed, 30 Sep 2020 15:40:04 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 4C1cnc35jKz1sM8y; Wed, 30 Sep 2020 15:40:04 +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 Jr0ES2hN2Dt0; Wed, 30 Sep 2020 15:40:02 +0200 (CEST) X-Auth-Info: 2e7BgTLGh3/tlnvyoLf5w4sPTQ6VXKAW5oOPKn4rkQs= 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:40:02 +0200 (CEST) Date: Wed, 30 Sep 2020 15:39:53 +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: <20200930153953.424a46c2@jawa> In-Reply-To: References: <20200919130759.31916-1-lukma@denx.de> <20200919130759.31916-2-lukma@denx.de> <20200930144710.662d9d99@jawa> 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_/fVsog_eUmsr3Sn+lsFECI=_"; 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:40:07 -0000 --Sig_/fVsog_eUmsr3Sn+lsFECI=_ Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Adhemerval, > On 30/09/2020 09:47, Lukasz Majewski wrote: > > Hi Adhemerval, > > =20 > >> On 19/09/2020 10:07, Lukasz Majewski wrote: =20 > >>> This is the helper function, which uses struct __timespec64 > >>> to provide 64 bit absolute time to futex syscalls. > >>> > >>> 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 > >>> > >>> 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). > >>> > >>> Additional precautions for m68k port have been taken - the > >>> futex-internal.c file will be compiled with -fno-inline flag. =20 > >> > >> LGTM with a nit regarding style and a clarification about=20 > >> '-fno-inline'. =20 > >=20 > > 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(+) > >>> > >>> 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 > >> > >> 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 > >> > >> Space before parentheses. =20 > >=20 > > Ok. Fixed. > > =20 > >> =20 > >>> + > >>> + clockbit =3D (clockid =3D=3D CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIM= E : > >>> 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 > >> > >> 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 > >> > >> 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 > >> > >> Do we still need it? =20 > >=20 > > 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). > >=20 > > I did some investigation and it looks like static inline > > in_time_t_range() function affects the compiler capabilities to > > generate correct code. > >=20 > > It can be easily inlined on any architecture but m68k. > >=20 > > As m68k has issues with this code compilation - it is IMHO better to > > disable inlining (-fno-inline) on this particular port. > >=20 > > As a result we would have slower execution only on relevant > > functions, but 64 bit time support would be provided. =20 >=20 > I recall this was need on first patch for the cancellable 64-bit > futex_abstimed_wait where it embedded the 32-bit fallback in the=20 > __futex_abstimed_wait_cancelable64. And my suggestion to move it to=20 > an external function seems to avoid the extra compiler flag. >=20 > This patchset uses the same strategy and I checked both patches > and it seems that -fno-inline is not required to build m68k. As fair as I can tell (at the time I tested it) - it was necessary to have -fno-inline when this patch was applied on top of the one which is now in the -master branch (in futex-internal.c). I will double check it now with: glibc/glibc-many-build --keep failed glibcs 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_/fVsog_eUmsr3Sn+lsFECI=_ Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAl90iqoACgkQAR8vZIA0 zr3WrAf/WGKsG2RjfxScR6E8Q0W7VWMeJxZpOjV12FN2WD2oBO4n+jbR+d6M4ubg I2MiF2ZyydxFdobqgf7cob8mDxMueiBjtOVTC94PA+viKWdLG/sI6WZadodHpsvQ K3YbdHzG8eywbURDzqPNw6/Qsh2dFS4bMq0Qeh3AkviM7IzDhvc1KakJD1fIy/7h g358H0GuRRcqnvT3JuuzUIe6RuL2F9JfY917eCzjBQRvqkl2FfrQ4dA7ra05afEj mACRjkRlnU1ysnxyEGW3sbkMb8qn4+GeVoVp1hqlbVIWnlnzVeTj/y8ova1xwRBq mkUh7fhZHf0qBtBNAbr/ObKzhfABpg== =kVgo -----END PGP SIGNATURE----- --Sig_/fVsog_eUmsr3Sn+lsFECI=_--