From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751456AbdFZGwT (ORCPT ); Mon, 26 Jun 2017 02:52:19 -0400 Received: from mail-wr0-f196.google.com ([209.85.128.196]:35123 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751092AbdFZGwN (ORCPT ); Mon, 26 Jun 2017 02:52:13 -0400 Date: Mon, 26 Jun 2017 08:52:04 +0200 From: Thierry Reding To: Mariusz Skamra Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de Subject: Re: [PATCH v2] ktime: Simplify ktime_compare implementation Message-ID: <20170626065204.GA31609@ulmo.fritz.box> References: <1495803647-9504-1-git-send-email-mariuszx.skamra@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="FCuugMFkClbJLl1L" Content-Disposition: inline In-Reply-To: <1495803647-9504-1-git-send-email-mariuszx.skamra@intel.com> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --FCuugMFkClbJLl1L Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 26, 2017 at 03:00:47PM +0200, Mariusz Skamra wrote: > ktime_sub can be used here instread of two conditional checks. >=20 > Signed-off-by: Mariusz Skamra > Acked-by: Kuppuswamy Sathyanarayanan > --- > include/linux/ktime.h | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) >=20 > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > index 0c8bd45..04817b1 100644 > --- a/include/linux/ktime.h > +++ b/include/linux/ktime.h > @@ -108,11 +108,7 @@ static inline ktime_t timeval_to_ktime(struct timeva= l tv) > */ > static inline int ktime_compare(const ktime_t cmp1, const ktime_t cmp2) > { > - if (cmp1 < cmp2) > - return -1; > - if (cmp1 > cmp2) > - return 1; > - return 0; > + return ktime_sub(cmp1, cmp2); > } > =20 > /** > --=20 > 2.1.4 Boot is broken on Tegra186 on next-20170623. Bisection points at this commit and I've confirmed that reverting it also fixes the problem. Another fix is to change the return value of ktime_compare() to ktime_t, though I'm not sure that'd be a good alternative because it breaks with the traditional return type of int (-1, 0, +1) for comparison functions. It looks like the issue is with the truncation from s64 to int, as demonstrated by this minimal test case: --- >8 --- #include #include #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) typedef int64_t s64; typedef s64 ktime_t; #define ktime_sub(lhs, rhs) ((lhs) - (rhs)) static inline int ktime_compare(const ktime_t cmp1, const ktime_t cmp2) { return ktime_sub(cmp1, cmp2); } int main(int argc, char *argv[]) { static const ktime_t values[][2] =3D { { 0, 1 }, { 1, 0 }, { 0, INT64_MAX }, { INT64_MAX, 0 }, }; unsigned int i; for (i =3D 0; i < ARRAY_SIZE(values); i++) printf("%ld - %ld =3D %ld (%d)\n", values[i][0], values[i][1], ktime_sub(values[i][0], values[i][1]), ktime_compare(values[i][0], values[i][1])); return 0; } --- >8 --- and the following results on x86_64: $ gcc -O2 -g -Wall -Werror -o test test.c && ./test 0 - 1 =3D -1 (-1) 1 - 0 =3D 1 (1) 0 - 9223372036854775807 =3D -9223372036854775807 (1) 9223372036854775807 - 0 =3D 9223372036854775807 (-1) and AArch64: $ aarch64-unknown-linux-gnu-gcc -O2 -g -Wall -Werror -static -o test test.= c && qemu-aarch64 ./test 0 - 1 =3D -1 (-1) 1 - 0 =3D 1 (1) 0 - 9223372036854775807 =3D -9223372036854775807 (1) 9223372036854775807 - 0 =3D 9223372036854775807 (-1) Perhaps it would be best to drop this patch? Thierry --FCuugMFkClbJLl1L Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAllQrxEACgkQ3SOs138+ s6En7Q//S2ERPq4HiyTcQxoMsG+vTcs0ttu6nc69rZcNxBdoqcH1sa/Y+gnJzge+ f5ZyY0nfUedug48iNIZTG1QMhLZAb+Itn88ShNxN/0O0vXyAPShwG12a+gf9d9Yf 34JTH8FKVoTgUGYua5qmQdZZMoFxXaQaxd4eQP/qpekK9iOzo24rHd1q5q+iu036 qyIzU4DFegH/cQP3OWfekn/tiP+w5xceApVakJRl6GwL5HZHpHFaVwLkdjLI0YMI /g8pa8aIDGInxRtGCygB0gx6tLHUgMbD8MDh7gk5UYoq82e4+W/flvMxdmsHzw5x fqRNPTDN0ufAOmNWetUApJ7V+YIfxjrb7luw+MMyihU4/B7iFnRdyt4iUBdSd664 NalmKLNqgD1swkK8eo4/JkeFGpAogqxlTiBLgUPpvjDy38LHva+AEeYikW8NIUjE D8Ix8U1iSzJibLKuGy+n6t9zo1O8lQfnoXtVr0Op6lkHTt9klXH8mgvGY0ZDAz8/ jPgidL7Axnxmz7QtEw1mL2LQdiXpA5RZAVlp+XIrSCTBQ82Y63lgEWE7CyAR48uR vvkl5zzKzDNFqNznITbDPOg2Bg2fN/elSVZY2AB1Rs6a96sYvrzJYdz/SH2/o6BE 4Am907rI59A8+/vu5fGmM/obP2wRFpZtNS6BNWcOgFvTvSMPcnE= =bDGN -----END PGP SIGNATURE----- --FCuugMFkClbJLl1L--