Dumb question: BKL on reboot ?
Quoted 2 lines · Hannes Reinecke
I've got a dumb question: Why is the BKL held on entering
sys_reboot()inkernel/sys.c:405?
Interesting. I have a few SMP deadlocks on x86-64 in reboot too and it's possible that it is the same problem.
I would hold it during exection of the notifiers, but drop it before calling into machine_*
-Andi
kernel/sys.c renamed from kernel/sys.c-o +4 -2
@@ -409,6 +409,7 @@ system_running =0 ;device_shutdown ();printk (KERN_EMERG "Restarting system.\n" ); +unlock_kernel ();machine_restart (NULL );break ;
@@ -425,8 +426,8 @@ system_running =0 ;device_shutdown ();printk (KERN_EMERG "System halted.\n" ); -machine_halt ();unlock_kernel (); +machine_halt ();do_exit (0 );break ;
@@ -435,8 +436,8 @@ system_running =0 ;device_shutdown ();printk (KERN_EMERG "Power down.\n" ); -machine_power_off ();unlock_kernel (); +machine_power_off ();do_exit (0 );break ;
@@ -451,6 +452,7 @@ system_running =0 ;device_shutdown ();printk (KERN_EMERG "Restarting system with command '%s'.\n" , buffer); +unlock_kernel ();machine_restart (buffer);break ;
Quoted 12 lines · Andi Kleen
Quoted 2 lines · Hannes Reinecke
I've got a dumb question: Why is the BKL held on entering
sys_reboot()inkernel/sys.c:405?Interesting. I have a few SMP deadlocks on x86-64 in reboot too and it's possible that it is the same problem.
I would hold it during exection of the notifiers, but drop it before calling into machine_*
Indeed. Works on s390 and survived several reboots (test still continuing). I moved lock_kernel() into the sections that actually _do_ something; looks much cleaner IMHO.
THX for your help.
Is it worth to try to push it into mainstream kernel?
Cheers,
Hannes
--
kernel/sys.c 2003-08-20 10:52:39.000000000 +0200 renamed from kernel/sys.c.orig 2003-08-20 10:52:39.000000000 +0200 +1 -1
@@ -340,11 +340,12 @@ magic2 != LINUX_REBOOT_MAGIC2B)) return -EINVAL; - lock_kernel(); switch (cmd) { case LINUX_REBOOT_CMD_RESTART: + lock_kernel(); notifier_call_chain(&reboot_notifier_list, SYS_RESTART,
NULL);
printk(KERN_EMERG "Restarting system.\n");
+ unlock_kernel();
machine_restart(NULL);
break;@@ -357,20 +358,25 @@
break;
case LINUX_REBOOT_CMD_HALT:
+ lock_kernel();
notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
printk(KERN_EMERG "System halted.\n");
+ unlock_kernel();
machine_halt();
do_exit(0);
break;
case LINUX_REBOOT_CMD_POWER_OFF:
+ lock_kernel();
notifier_call_chain(&reboot_notifier_list,
SYS_POWER_OFF, NULL);
printk(KERN_EMERG "Power down.\n");
+ unlock_kernel();
machine_power_off();
do_exit(0);
break;
case LINUX_REBOOT_CMD_RESTART2:
+ lock_kernel();
if (strncpy_from_user(&buffer[0], (char *)arg,
sizeof(buffer) -
1) < 0) {
unlock_kernel();
return -EFAULT;@@ -379,14 +385,13 @@
notifier_call_chain(&reboot_notifier_list, SYS_RESTART,
buffer);
printk(KERN_EMERG "Restarting system with command
'%s'.\n", buff
er);
+ unlock_kernel();
machine_restart(buffer);
break;
default:
- unlock_kernel();
return -EINVAL;
}
- unlock_kernel();
return 0;}
Quoted 1 line
Is it worth to try to push it into mainstream kernel?
Yes definitely. My patch was for 2.6.0test3.
-Andi