[patch] softlockup: do the wakeup from a hrtimer

[patch] softlockup: do the wakeup from a hrtimer

[Ingo Molnar]

Subject: softlockup: do the wakeup from a hrtimer
From: Ingo Molnar <mingo@elte.hu>

David Miller reported soft lockup false-positives that trigger on NOHZ 
due to CPUs idling for more than 10 seconds.

The solution is to drive the wakeup of the watchdog threads not from the 
timer tick (which has no guaranteed frequency), but from the watchdog 
tasks themselves.

http://bugzilla.kernel.org/show_bug.cgi?id=9409

Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/softlockup.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Index: linux/kernel/softlockup.c
===================================================================
--- linux.orig/kernel/softlockup.c
+++ linux/kernel/softlockup.c
@@ -100,10 +100,6 @@ void softlockup_tick(void)
 
 	now = get_timestamp(this_cpu);
 
-	/* Wake up the high-prio watchdog task every second: */
-	if (now > (touch_timestamp + 1))
-		wake_up_process(per_cpu(watchdog_task, this_cpu));
-
 	/* Warn about unreasonable 10+ seconds delays: */
 	if (now <= (touch_timestamp + softlockup_thresh))
 		return;
@@ -141,7 +137,7 @@ static int watchdog(void *__bind_cpu)
 	while (!kthread_should_stop()) {
 		set_current_state(TASK_INTERRUPTIBLE);
 		touch_softlockup_watchdog();
-		schedule();
+		msleep(1000);
 	}
 
 	return 0;

Re: [patch] softlockup: do the wakeup from a hrtimer

[Andrew Morton]

On Tue, 20 Nov 2007 09:46:11 +0100
Ingo Molnar <mingo@elte.hu> wrote:

> Subject: softlockup: do the wakeup from a hrtimer
> From: Ingo Molnar <mingo@elte.hu>
> 
> David Miller reported soft lockup false-positives that trigger on NOHZ 
> due to CPUs idling for more than 10 seconds.
> 
> The solution is to drive the wakeup of the watchdog threads not from the 
> timer tick (which has no guaranteed frequency), but from the watchdog 
> tasks themselves.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=9409
> 
> Reported-by: David Miller <davem@davemloft.net>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  kernel/softlockup.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> Index: linux/kernel/softlockup.c
> ===================================================================
> --- linux.orig/kernel/softlockup.c
> +++ linux/kernel/softlockup.c
> @@ -100,10 +100,6 @@ void softlockup_tick(void)
>  
>  	now = get_timestamp(this_cpu);
>  
> -	/* Wake up the high-prio watchdog task every second: */
> -	if (now > (touch_timestamp + 1))
> -		wake_up_process(per_cpu(watchdog_task, this_cpu));
> -
>  	/* Warn about unreasonable 10+ seconds delays: */
>  	if (now <= (touch_timestamp + softlockup_thresh))
>  		return;
> @@ -141,7 +137,7 @@ static int watchdog(void *__bind_cpu)
>  	while (!kthread_should_stop()) {
>  		set_current_state(TASK_INTERRUPTIBLE);
>  		touch_softlockup_watchdog();
> -		schedule();
> +		msleep(1000);
>  	}
>  
>  	return 0;

I think you wanted msleep_interruptible() there to avoid contributing to
load average?

The set_current_state() can go away.

This will introduce an up-to-one-second delay in responding to
kthread_should_stop().  Is that bad?

Re: [patch] softlockup: do the wakeup from a hrtimer

[Ingo Molnar]

* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 20 Nov 2007 09:46:11 +0100
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > Subject: softlockup: do the wakeup from a hrtimer
> > From: Ingo Molnar <mingo@elte.hu>
> > 
> > David Miller reported soft lockup false-positives that trigger on NOHZ 
> > due to CPUs idling for more than 10 seconds.
> > 
> > The solution is to drive the wakeup of the watchdog threads not from the 
> > timer tick (which has no guaranteed frequency), but from the watchdog 
> > tasks themselves.
> > 
> > http://bugzilla.kernel.org/show_bug.cgi?id=9409
> > 
> > Reported-by: David Miller <davem@davemloft.net>
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > ---
> >  kernel/softlockup.c |    6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > Index: linux/kernel/softlockup.c
> > ===================================================================
> > --- linux.orig/kernel/softlockup.c
> > +++ linux/kernel/softlockup.c
> > @@ -100,10 +100,6 @@ void softlockup_tick(void)
> >  
> >  	now = get_timestamp(this_cpu);
> >  
> > -	/* Wake up the high-prio watchdog task every second: */
> > -	if (now > (touch_timestamp + 1))
> > -		wake_up_process(per_cpu(watchdog_task, this_cpu));
> > -
> >  	/* Warn about unreasonable 10+ seconds delays: */
> >  	if (now <= (touch_timestamp + softlockup_thresh))
> >  		return;
> > @@ -141,7 +137,7 @@ static int watchdog(void *__bind_cpu)
> >  	while (!kthread_should_stop()) {
> >  		set_current_state(TASK_INTERRUPTIBLE);
> >  		touch_softlockup_watchdog();
> > -		schedule();
> > +		msleep(1000);
> >  	}
> >  
> >  	return 0;
> 
> I think you wanted msleep_interruptible() there to avoid contributing to
> load average?
> 
> The set_current_state() can go away.

these can be fixed, but:

> This will introduce an up-to-one-second delay in responding to 
> kthread_should_stop().  Is that bad?

grumble, it's bad. I guess David is right that this should be fixed the 
right way ;-) So the above patch cannot go in.

	Ingo

[patch] softlockup: fix false positives on CONFIG_NOHZ

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> these can be fixed, but:
> 
> > This will introduce an up-to-one-second delay in responding to 
> > kthread_should_stop().  Is that bad?
> 
> grumble, it's bad. I guess David is right that this should be fixed 
> the right way ;-) So the above patch cannot go in.

Thomas found the right fix. David, could you try the fix below, does it 
fix those false positives on your nohz Niagara cores?

	Ingo

---------------->
Subject: softlockup: fix false positives on CONFIG_NOHZ
From: Thomas Gleixner <tglx@linutronix.de>

David Miller reported soft lockup false-positives that trigger
on NOHZ due to CPUs idling for more than 10 seconds.

The solution is touch the softlockup watchdog when we return from
idle. (by definition we are not 'locked up' when we were idle)

 http://bugzilla.kernel.org/show_bug.cgi?id=9409

Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/time/tick-sched.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/kernel/time/tick-sched.c
===================================================================
--- linux.orig/kernel/time/tick-sched.c
+++ linux/kernel/time/tick-sched.c
@@ -133,6 +133,8 @@ void tick_nohz_update_jiffies(void)
 	if (!ts->tick_stopped)
 		return;
 
+	touch_softlockup_watchdog();
+
 	cpu_clear(cpu, nohz_cpu_mask);
 	now = ktime_get();