Skip to contentSkip to conversation

Dumb question: BKL on reboot ?

DISCUSSION Andi Kleen 2 participants 3 messages Aug 20, 2003 1 message not in the archive

lore
Share & tools
lore

Andi Kleen

author
#1 lore raw
In reply to a message not in the archive
Quoted 2 lines · Hannes Reinecke

I've got a dumb question: Why is the BKL held on entering sys_reboot() in kernel/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;
 
#2 lore raw
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() in kernel/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;

}

author
#3 lore raw
Quoted 1 line

Is it worth to try to push it into mainstream kernel?

Yes definitely. My patch was for 2.6.0test3.

-Andi