Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755009AbcK3Adi (ORCPT ); Tue, 29 Nov 2016 19:33:38 -0500 Received: from ozlabs.org ([103.22.144.67]:52655 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752078AbcK3Adh (ORCPT ); Tue, 29 Nov 2016 19:33:37 -0500 Date: Wed, 30 Nov 2016 10:57:27 +1100 From: David Gibson To: Thomas Gleixner Cc: John Stultz , lkml , Liav Rehana , Chris Metcalf , Richard Cochran , Ingo Molnar , Prarit Bhargava , Laurent Vivier , "Christopher S . Hall" , "4.6+" Subject: Re: [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation. Message-ID: <20161129235727.GA19891@umbus> References: <1479531216-25361-1-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xHFwDpU9dbj6ez1V" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3397 Lines: 76 --xHFwDpU9dbj6ez1V Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 29, 2016 at 03:22:17PM +0100, Thomas Gleixner wrote: > On Fri, 18 Nov 2016, John Stultz wrote: > > From: Liav Rehana > >=20 > > During the calculation of the nsec variable in the inline function > > timekeeping_delta_to_ns, it may undergo a sign extension if its msb > > is set just before the shift. The sign extension may, in some cases, > > gain it a value near the maximum value of the 64-bit range. This is > > bad when it is later used in a division function, such as > > __iter_div_u64_rem, where the amount of loops it will go through to > > calculate the division will be too large. One can encounter such a > > problem, for example, when trying to connect through ftp from an > > outside host to the operation system. When the OS is too overloaded, > > delta will get a high enough value for the msb of the sum > > delta * tkr->mult + tkr->xtime_nsec to be set, and so after the > > shift the nsec variable will gain a value similar to > > 0xffffffffff000000. Using a variable with such a value in the > > inline function __iter_div_u64_rem will take too long, making the > > ftp connection attempt seem to get stuck. > > The following commit fixes that chance of sign extension, while > > maintaining the type of the nsec variable as signed for other > > functions that use this variable, for possible legit negative > > time intervals. > > > > Thomas/Ingo: This is for tip:timers/urgent. >=20 > Certainly not! My objections against this still stand. See: >=20 > http://lkml.kernel.org/r/alpine.DEB.2.20.1609261956160.4915@nanos >=20 > http://lkml.kernel.org/r/alpine.DEB.2.20.1609270929170.4891@nanos >=20 > If we have legitimate use cases with a negative delta, then this patch > breaks them no matter what. See the basic C course section in the second > link. So, fwiw, when I first wrote a variant on this, I wasn't trying to fix every case - just to make the consequences less bad if something goes wrong. An overflow here can still mess up timekeeping, it's true, but time going backwards tends to cause things to go horribly, horribly wrong - which was why I spotted this in the first place. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --xHFwDpU9dbj6ez1V Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYPhXnAAoJEGw4ysog2bOSSNIP+gLiR4w/7pY6pQy7mwAzomkY nSdZeS2ux7wHHKHSTJD7bWOYjqMgsPN3s9W8aL8w0GvPAFDKWJCYiQ2BjWmUV6BR lbfphZSXjQG8T4lzoKYwP6jS4emDlWFy3oa0nxEgQySY8eJ7+6GBS1tK6C09SwDd ibimA/L6MQzc8JfmJH1C9OcUKnYTmg0O21/xmWb2/MDp4JQw4BK8UEtOJS/m7Bdp RFG0JV+7ceYx8Fe9JKZLh5x1FPBkuz82Zs1gnjrPMSJxugUcrceyZattaqXI8qg9 ZafFvjTxBfP9JhKZU2kJ1MisilGc3qKjzWgKlZogP3bo6wfQ5mC60DL3Xt+++tgH XidHOGdZ3dhnxJhWcxp7trl4iAUY8dxcrXaioAGlapLB5u9nV+Dgu1ch6A9cVUIt oAijB4DBxnw4ft1cX21SpQIgrGC1k365Z73gE8lea0P7Yceu0E1HqjRvw8JHUGLb xZ0lbbPOIcC7066+lo+ropR9KDxjv+2wKqrVyP0649V9cLJDh7DNogs0uSiOtVfG T2fVnmPrAGI6v+dZD4eN5OvUdgVDFUA2QeTcOK7CAUsn+EU7UopdR9fkSpiX/O75 gHFhszcpg7UorxzlEw7+dwTzcld2Hbmguxbx/Y2Jav06nGbDvNyRKzMt9PEBDC5r JCHHBPEAyU2NynJUZ65+ =4MZf -----END PGP SIGNATURE----- --xHFwDpU9dbj6ez1V--