Re: broken suspend (sched related) [Was: 2.6.24-rc4-mm1]

[Gautham R Shenoy]

On Sun, Dec 09, 2007 at 08:46:47AM +0100, Ingo Molnar wrote:
> 
> * Jiri Slaby <jirislaby@xxxxxxxxx> wrote:
> 
> > On 12/08/2007 04:24 PM, Ingo Molnar wrote:
> > > i'm wondering why it had no effect now
> > 
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index e0d3a4f..a46c252 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -47,15 +47,21 @@ void __init cpu_hotplug_init(void)
> >  }
> > 
> >  #ifdef CONFIG_HOTPLUG_CPU
> > -
> > +#include <asm/io.h>
> >  void get_online_cpus(void)
> >  {
> > +       outb(0x20, 0x80);
> >         might_sleep();
> > +       outb(0x21, 0x80);
> 
> ah. If you comment out get_online_cpus()/put_online_cpus() from 
> kernel/softlockup.c, does it start working?
> 
> Gautham, any ideas?
Hi Ingo, 

>From the code I fail to see how get_online_cpus() can help us. 

+		/*
+		 * Only do the hung-tasks check on one CPU:
+		 */
+		get_online_cpus();
+		check_cpu = any_online_cpu(cpu_online_map);
+		put_online_cpus();

check_cpu can go offline here, no?

+
+		if (this_cpu != check_cpu)
+			continue;
+
+		if (sysctl_hung_task_timeout_secs)
+			check_hung_uninterruptible_tasks(this_cpu);

Further more this can cause a deadlock since we're calling 
get_online_cpus() from the watchdog thread's context, 
which is going to be kthread_stop'ed from a cpu-hotplug context.
This is what I think was happening in the case reported by Jiri.

Please find the patch below.

Thanks and Regards
gautham.

commit 15bfb662b35c609490185fba2fd4713d230b9374
Author: Gautham R Shenoy <ego@xxxxxxxxxx>
Date:   Mon Dec 10 13:41:45 2007 +0530

softlockup: remove get_online_cpus() which doesn't help here.

The get_online_cpus() protection seems to be bogus
in kernel/softlockup.c as cpu cached in check_cpu can go offline
once we do a put_online_cpus().

This can also cause deadlock during a cpu offline as follows:

WATCHDOG_THREAD:		OFFLINE_CPU:
				mutex_down(&cpu_hotplug.lock);
				/* All subsequent get_online_cpus
				 * will be blocked till we're
				 * done with this cpu-hotplug
				 * operation.
				 */

get_online_cpus();
/* watchdog is blocked
   Thus we cannot
   go further until
   the cpu-hotplug
   operation completes
 */
 				CPU_DEAD:
    				kthread_stop(watchdog_thread);

    				/* we're trying to stop a
    				 * thread which is blocked
    				 * waiting for us to finish.
    				 *
    				 * Since we cannot finish until
    				 * the thread stops, we deadlock here!
    				 */
    
Signed-off-by: Gautham R Shenoy <ego@xxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxxx>
Cc: Arjan van de Van <arjan@xxxxxxxxxxxxxxx>
Cc: Jiri Slaby <jirislaby@xxxxxxxxx>

diff --git a/kernel/softlockup.c b/kernel/softlockup.c
index e50b44a..576eb9c 100644
--- a/kernel/softlockup.c
+++ b/kernel/softlockup.c
@@ -219,9 +219,7 @@ static int watchdog(void *__bind_cpu)
 		/*
 		 * Only do the hung-tasks check on one CPU:
 		 */
-		get_online_cpus();
 		check_cpu = any_online_cpu(cpu_online_map);
-		put_online_cpus();
 
 		if (this_cpu != check_cpu)
 			continue;

-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/