All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Liav Rehana <liavr@mellanox.com>
Cc: john.stultz@linaro.org, linux-kernel@vger.kernel.org,
	eladkan@mellanox.com, noamca@mellanox.com
Subject: Re: [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation.
Date: Mon, 26 Sep 2016 20:01:27 -0400 (EDT)	[thread overview]
Message-ID: <alpine.DEB.2.20.1609261956160.4915@nanos> (raw)
In-Reply-To: <1474868736-22724-1-git-send-email-liavr@mellanox.com>

On Mon, 26 Sep 2016, Liav Rehana wrote:
> 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.
> The following commit fixes that chance of sign extension,

Again. This does not fix anything, it papers over the underlying problem
that the calling code hands in a delta which is big enough to overflow the
multiplication into the negative space. You just extend the range of deltas
which are handled gracefully, but that does not fix the underlying problem
as we still can run into the multiplication overflow. It won't cause the
result to be negative, but it will be crap nevertheless.

> while maintaining the type of the nsec variable as signed for other
> functions that use this variable, for possible legit negative time
> intervals.

What is this maintaining? The type of this variable changes to u64 and
other functions cannot use this variable at all because it's local to that
function. This sentence makes no sense at all.

Thanks,

	tglx

  parent reply	other threads:[~2016-09-27  0:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-26  5:45 [PATCH] timekeeping: Change type of nsec variable to unsigned in its calculation Liav Rehana
2016-09-26  6:02 ` John Stultz
2016-09-27  0:01 ` Thomas Gleixner [this message]
2016-09-27  5:10   ` Liav Rehana
2016-09-27 14:18     ` Thomas Gleixner
2016-09-26  6:13 Liav Rehana
2016-11-19  4:53 John Stultz
2016-11-28 22:50 ` John Stultz
2016-11-29 14:22 ` Thomas Gleixner
2016-11-29 23:57   ` David Gibson
2016-11-30 23:21     ` Thomas Gleixner
2016-12-01  2:12       ` David Gibson
2016-12-01 11:59         ` Thomas Gleixner
2016-12-01 20:23           ` John Stultz
2016-12-01 20:46             ` Thomas Gleixner
2016-12-01 21:19               ` John Stultz
2016-12-01 22:44                 ` Thomas Gleixner
2016-12-01 23:03                   ` John Stultz
2016-12-01 23:08                     ` Thomas Gleixner
2016-12-01 23:32           ` David Gibson
2016-12-02  8:36             ` Thomas Gleixner
2016-12-03  0:33               ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1609261956160.4915@nanos \
    --to=tglx@linutronix.de \
    --cc=eladkan@mellanox.com \
    --cc=john.stultz@linaro.org \
    --cc=liavr@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=noamca@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.