linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Don Zickus <dzickus@redhat.com>
To: Michal Hocko <mhocko@suse.cz>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>
Subject: Re: [RFC 2/2] watchdog: update watchdog_tresh properly
Date: Fri, 19 Jul 2013 12:08:52 -0400	[thread overview]
Message-ID: <20130719160852.GM126784@redhat.com> (raw)
In-Reply-To: <1374224699-13255-2-git-send-email-mhocko@suse.cz>

On Fri, Jul 19, 2013 at 11:04:59AM +0200, Michal Hocko wrote:
> watchdog_tresh controls how often nmi perf event counter checks per-cpu
> hrtimer_interrupts counter and blows up if the counter hasn't changed
> since the last check. The counter is updated by per-cpu watchdog_hrtimer
> hrtimer which is scheduled with 2/5 watchdog_thresh period which
> guarantees that hrtimer is scheduled 2 times per the main period. Both
> hrtimer and perf event are started together when the watchdog is
> enabled.
> 
> So far so good. But...
> 
> But what happens when watchdog_thresh is updated from sysctl handler?
> 
> proc_dowatchdog will set a new sampling period and hrtimer callback
> (watchdog_timer_fn) will use the new value in the next round.
> The problem, however, is that nobody tells the perf event that the
> sampling period has changed so it is ticking with the period configured
> when it has been set up.
> 
> This might result in an ear riping dissonance between perf and hrtimer
> parts if the watchdog_thresh is increased. And even worse it might lead
> to KABOOM if the watchdog is configured to panic on such a spurious
> lockup.

Heh.  Good point.

What if we keep it simpler.

if (old_thresh != watchdog_thresh)
  watchdog_disable_all_cpus()
  wathcdog_enable_all_cpus()

The idea is that we are not changing thresholds that often and if we do, we
should probably sync up all the timers and threads again.  Safer to start
from scratch.

Thoughts?

Cheers,
Don

  reply	other threads:[~2013-07-19 16:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-19  9:04 [RFC 1/2] watchdog: update watchdog attributes atomically Michal Hocko
2013-07-19  9:04 ` [RFC 2/2] watchdog: update watchdog_tresh properly Michal Hocko
2013-07-19 16:08   ` Don Zickus [this message]
2013-07-19 16:37     ` Michal Hocko
2013-07-19 18:05       ` Don Zickus
2013-07-20  8:42         ` Michal Hocko
2013-07-22 11:45   ` [RFC -v2 " Michal Hocko
2013-07-22 12:47     ` Michal Hocko
2013-07-22 14:32     ` [RFC -v3 " Michal Hocko
2013-07-23 13:53       ` Don Zickus
2013-07-23 14:07         ` Michal Hocko
2013-07-23 14:44           ` Don Zickus
2013-07-23 14:51             ` Michal Hocko
2013-07-19 16:10 ` [RFC 1/2] watchdog: update watchdog attributes atomically Don Zickus
2013-07-19 16:33   ` Michal Hocko
2013-07-23 13:56     ` Don Zickus

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=20130719160852.GM126784@redhat.com \
    --to=dzickus@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.cz \
    --cc=mingo@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).