linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Rik van Riel <riel@fb.com>
Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com,
	John Stultz <john.stultz@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Stephen Boyd <sboyd@kernel.org>, Jonathan Corbet <corbet@lwn.net>,
	Mark Rutland <Mark.Rutland@arm.com>,
	Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH RFC clocksource 2/5] clocksource: Retry clock read if long delays detected
Date: Wed, 6 Jan 2021 11:53:21 -0800	[thread overview]
Message-ID: <20210106195321.GC2743@paulmck-ThinkPad-P72> (raw)
In-Reply-To: <edbe9ac5fc3f76601f752ce2c5a8052dc05fd4f6.camel@fb.com>

On Wed, Jan 06, 2021 at 11:28:00AM -0500, Rik van Riel wrote:
> On Tue, 2021-01-05 at 16:41 -0800, paulmck@kernel.org wrote:
> > 
> > @@ -203,7 +204,6 @@ static void
> > clocksource_watchdog_inject_delay(void)
> >  		injectfail = inject_delay_run;
> >  	if (!(++injectfail / inject_delay_run % inject_delay_freq)) {
> >  		printk("%s(): Injecting delay.\n", __func__);
> > -		injectfail = 0;
> >  		for (i = 0; i < 2 * WATCHDOG_THRESHOLD / NSEC_PER_MSEC;
> > i++)
> >  			udelay(1000);
> 
> Wait, patch 1 just added that line?
> 
> Should patch 1 not add it and this
> patch go without
> this removal? :)

Good catch, will fix.  ;-)

> +               wdagain_nsec = clocksource_cyc2ns(delta, watchdog-
> >mult, watchdog->shift);
> +               if (wdagain_nsec < 0 || wdagain_nsec >
> WATCHDOG_MAX_SKEW) {
> +                       wderr_nsec = wdagain_nsec;
> +                       if (nretries++ < max_read_retries)
> +                               goto retry;
> +               }
> 
> Given that clocksource_cyc2ns uses unsigned multiplication
> followed by a right shift, do we need to test for <0?

I am worried about the possibility of the "shift" argument to
clocksource_cyc2ns() being zero.  For example, unless I am missing
something, clocksource_tsc has a zero .shift field.

							Thanx, Paul

  reply	other threads:[~2021-01-06 19:54 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-06  0:40 [PATCH RFC clocksource] Do not mark clocks unstable due to delays Paul E. McKenney
2021-01-06  0:41 ` [PATCH RFC clocksource 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-01-06  0:41 ` [PATCH RFC clocksource 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-01-06 16:28   ` Rik van Riel
2021-01-06 19:53     ` Paul E. McKenney [this message]
2021-01-06 20:59       ` Rik van Riel
2021-01-06  0:41 ` [PATCH RFC clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-01-06  0:41 ` [PATCH RFC clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-01-06  0:41 ` [PATCH RFC clocksource 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-01-12  0:42 ` [PATCH v2 clocksource] Do not mark clocks unstable due to delays Paul E. McKenney
2021-01-12  0:45   ` [PATCH v2 clocksource 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-01-12  0:45   ` [PATCH v2 clocksource 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-01-12  0:45   ` [PATCH v2 clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-01-12  0:45   ` [PATCH v2 clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-01-12  0:45   ` [PATCH v2 clocksource 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-02-02 17:04   ` [PATCH v3 clocksource] Do not mark clocks unstable due to delays Paul E. McKenney
2021-02-02 17:06     ` [PATCH clocksource 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-02-02 17:06     ` [PATCH clocksource 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-02-02 17:06     ` [PATCH clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-02-02 17:06     ` [PATCH clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-02-02 19:51       ` Randy Dunlap
2021-02-03  0:50         ` Paul E. McKenney
2021-02-03  1:31           ` Randy Dunlap
2021-02-03  1:40             ` Paul E. McKenney
2021-02-02 17:06     ` [PATCH clocksource 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-02-17 21:28     ` [PATCH v3 clocksource] Do not mark clocks unstable due to delays Paul E. McKenney
2021-02-17 21:29       ` [PATCH clocksource 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-02-17 21:29       ` [PATCH clocksource 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-02-17 21:29       ` [PATCH clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-02-17 21:29       ` [PATCH clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-02-17 21:29       ` [PATCH clocksource 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-03-04  0:49       ` [PATCH v5 clocksource] Do not mark clocks unstable due to delays for v5.13 Paul E. McKenney
2021-03-04  0:53         ` [PATCH kernel/time 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-03-04  0:53         ` [PATCH kernel/time 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-03-04  0:53         ` [PATCH kernel/time 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-03-04  0:53         ` [PATCH kernel/time 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-03-04  0:53         ` [PATCH kernel/time 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-04-02 20:29         ` [PATCH v5 clocksource] Do not mark clocks unstable due to delays for v5.13 Paul E. McKenney
2021-04-02 20:31           ` [PATCH v6 clocksource] Do not mark clocks unstable dueclocksource: Provide module parameters to inject delays in watchdog paulmck
2021-04-02 22:22             ` Thomas Gleixner
2021-04-02 22:37               ` Paul E. McKenney
2021-04-02 22:48               ` [PATCH v7 clocksource] Do not mark clocks unstable due to delays for v5.13 Paul E. McKenney
2021-04-02 22:49                 ` [PATCH v7 clocksource 1/5] clocksource: Provide module parameters to inject delays in watchdog paulmck
2021-04-02 22:49                 ` [PATCH v7 clocksource 2/5] clocksource: Retry clock read if long delays detected paulmck
2021-04-10  8:41                   ` Thomas Gleixner
2021-04-10 23:50                     ` Paul E. McKenney
2021-04-02 22:49                 ` [PATCH v7 clocksource 3/5] clocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-04-10  9:00                   ` Thomas Gleixner
2021-04-11  0:20                     ` Paul E. McKenney
2021-04-11 10:33                       ` Thomas Gleixner
2021-04-11 16:46                         ` Paul E. McKenney
2021-04-12  4:21                           ` Paul E. McKenney
2021-04-12 13:08                             ` Thomas Gleixner
2021-04-12 18:20                               ` Paul E. McKenney
2021-04-12 18:54                                 ` Thomas Gleixner
2021-04-12 19:57                                   ` Paul E. McKenney
2021-04-12 20:37                                     ` Thomas Gleixner
2021-04-12 23:18                                       ` Paul E. McKenney
2021-04-13 20:49                                         ` Thomas Gleixner
2021-04-14  4:48                                           ` Paul E. McKenney
2021-04-02 22:49                 ` [PATCH v7 clocksource 4/5] clocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-04-02 22:49                 ` [PATCH v7 clocksource 5/5] clocksource: Do pairwise clock-desynchronization checking paulmck
2021-04-10  9:04                   ` Thomas Gleixner
2021-04-11  0:21                     ` Paul E. McKenney
2021-04-10  8:01                 ` [PATCH v7 clocksource] Do not mark clocks unstable due to delays for v5.13 Thomas Gleixner
2021-04-10 23:26                   ` Paul E. McKenney
2021-04-11 10:58                     ` Thomas Gleixner
2021-04-11 16:50                       ` Paul E. McKenney
2021-04-02 20:31           ` [PATCH v6 clocksource] Do not mark clocks unstable dueclocksource: Retry clock read if long delays detected paulmck
2021-04-02 20:31           ` [PATCH v6 clocksource] Do not mark clocks unstable dueclocksource: Check per-CPU clock synchronization when marked unstable paulmck
2021-04-02 20:31           ` [PATCH v6 clocksource] Do not mark clocks unstable dueclocksource: Provide a module parameter to fuzz per-CPU clock checking paulmck
2021-04-02 20:31           ` [PATCH v6 clocksource] Do not mark clocks unstable dueclocksource: Do pairwise clock-desynchronization checking paulmck

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=20210106195321.GC2743@paulmck-ThinkPad-P72 \
    --to=paulmck@kernel.org \
    --cc=Mark.Rutland@arm.com \
    --cc=corbet@lwn.net \
    --cc=john.stultz@linaro.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=riel@fb.com \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).