lkml.org 
[lkml]   [2011]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH 2/2] watchdog: Softlockup has regular windows where it is not armed

The softlockup watchdog has a two stage sync - touch_softlockup_watchdog
simply sets the timestamp to 0 and later on the timer routine notices
this and sets the timestamp.

The problem is this timer goes off every 4 seconds by default, so
each time we call touch_softlockup_watchdog there is a period
of up to 4 seconds where the softlockup watchdog is not armed.

We call touch_softlockup_watchdog very often in the NO_HZ code and
end up hitting this issue every time we go in and out of idle.

I wrote a simple test case:

http://ozlabs.org/~anton/junkcode/badguy.tar.gz

That disables interrupts on selected CPUs for a period of time. Don't
run it on a machine you care about. When I disable interrupts for 30
seconds on a previously idle CPU I get no warning:

insmod ./badguy.ko timeout=30 cpus=4

However if I keep the CPU busy so we don't switch in and out of NO_HZ
mode I get a warning as expected:

taskset -c 4 yes > /dev/null &
insmod ./badguy.ko timeout=30 cpus=4

With the following patch I get a warning even on a previously idle
CPU.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

There might be a reason for this two stage sync but I haven't been
able to find it yet. Perhaps the unsynced versions of cpu_clock() and
sched_clock_tick() are not safe to call from all contexts?

Index: linux-build/kernel/watchdog.c
===================================================================
--- linux-build.orig/kernel/watchdog.c 2011-11-16 08:04:56.274478516 +1100
+++ linux-build/kernel/watchdog.c 2011-11-16 08:04:59.278533261 +1100
@@ -33,7 +33,6 @@ int __read_mostly watchdog_thresh = 10;
static DEFINE_PER_CPU(unsigned long, watchdog_touch_ts);
static DEFINE_PER_CPU(struct task_struct *, softlockup_watchdog);
static DEFINE_PER_CPU(struct hrtimer, watchdog_hrtimer);
-static DEFINE_PER_CPU(bool, softlockup_touch_sync);
static DEFINE_PER_CPU(bool, soft_watchdog_warn);
#ifdef CONFIG_HARDLOCKUP_DETECTOR
static DEFINE_PER_CPU(bool, hard_watchdog_warn);
@@ -134,7 +133,7 @@ static void __touch_watchdog(void)

void touch_softlockup_watchdog(void)
{
- __this_cpu_write(watchdog_touch_ts, 0);
+ __touch_watchdog();
}
EXPORT_SYMBOL(touch_softlockup_watchdog);

@@ -157,8 +156,8 @@ EXPORT_SYMBOL(touch_nmi_watchdog);

void touch_softlockup_watchdog_sync(void)
{
- __raw_get_cpu_var(softlockup_touch_sync) = true;
- __raw_get_cpu_var(watchdog_touch_ts) = 0;
+ sched_clock_tick();
+ __touch_watchdog();
}

#ifdef CONFIG_HARDLOCKUP_DETECTOR
@@ -258,19 +257,6 @@ static enum hrtimer_restart watchdog_tim
/* .. and repeat */
hrtimer_forward_now(hrtimer, ns_to_ktime(get_sample_period()));

- if (touch_ts == 0) {
- if (unlikely(__this_cpu_read(softlockup_touch_sync))) {
- /*
- * If the time stamp was touched atomically
- * make sure the scheduler tick is up to date.
- */
- __this_cpu_write(softlockup_touch_sync, false);
- sched_clock_tick();
- }
- __touch_watchdog();
- return HRTIMER_RESTART;
- }
-
/* check for a softlockup
* This is done by making sure a high priority task is
* being scheduled. The task touches the watchdog to
@@ -438,7 +424,7 @@ static int watchdog_enable(int cpu)
goto out;
}
kthread_bind(p, cpu);
- per_cpu(watchdog_touch_ts, cpu) = 0;
+ __touch_watchdog();
per_cpu(softlockup_watchdog, cpu) = p;
wake_up_process(p);
}

\
 
 \ /
  Last update: 2011-11-24 04:57    [W:0.051 / U:1.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site