Re: [PATCH] sysrq: add debug message to reboot event

From: tina.yang
Date: Fri Aug 21 2009 - 01:14:43 EST


Andrew Morton wrote:
> On Wed, 19 Aug 2009 19:22:16 -0700
> "tina.yang" <tina.yang@xxxxxxxxxx> wrote:
>
>
>> Add debug message to detect keyboard vs non-keyboard triggered sysrq-b events.
>> This is to assist postmortem debugging on complicated computing setup with
>> large number of applications involved where reboot event had occurred, but
>> unclear of its origin.
>>
>
> I'm still struggling to understand the motivation for the change.
> There are a large number of ways in which a machine can be rebooted,
> all the way down to a triple-fault. So it seems fairly arbitrary to
> add additional information to discriminate between just two of those
> ways.
>
> I assume that somewhere in your setup you have a script which does
> `echo b /proc/sysrq-trigger' and it took ages to work out that this was
> happening and you felt that having this code in place would have
> helped you debug that problem, yes?
>
> If so, I wonder what is the likelihood that someone else will have the
> same problem and will find this change useful.
>
Exactly what you've described except we are debugging someone else's
(customers') setup remotely without familiarity of their environment
and often with a time constraint. The setup usually involves
large database clusters and who knows what else are running there.
I think people in a comparable environment can benefit from a little
more debug help from the system as we can.

> Perhaps we should do this for all sysrq events rather than just sysrq-b?
>
>
>
From a system perspective, I agree generalization of message handling is
better. The most critical one we have experienced is the reboot event.

>> --- linux-2.6.18.i686/drivers/char/sysrq.c.orig 2009-08-13 10:55:57.526459000 -0700
>> +++ linux-2.6.18.i686/drivers/char/sysrq.c 2009-08-13 10:58:10.798739000 -0700
>>
>
> 2.6.18 is truly ancient and this patch doesn't apply at all to
> current development kernels.
>
>
Sorry grabbed the wrong one.

--- linux-2.6.git/drivers/char/sysrq.c.orig 2009-08-13 15:43:34.000000000 -0700
+++ linux-2.6.git/drivers/char/sysrq.c 2009-08-20 20:40:50.287922000 -0700
@@ -135,17 +135,22 @@ static struct sysrq_key_op sysrq_crash_o
.enable_mask = SYSRQ_ENABLE_DUMP,
};

-static void sysrq_handle_reboot(int key, struct tty_struct *tty)
+static void sysrq_handle_reboot(int check_mask, int key, struct tty_struct *tty)
{
+ if (check_mask)
+ printk("<keyboard-invoked>\n");
+ else
+ printk("<non-keyboard-invoked: PID %d COMMAND %s>\n",
+ current->pid, current->comm);
lockdep_off();
local_irq_enable();
emergency_restart();
}
static struct sysrq_key_op sysrq_reboot_op = {
- .handler = sysrq_handle_reboot,
- .help_msg = "reBoot",
- .action_msg = "Resetting",
- .enable_mask = SYSRQ_ENABLE_BOOT,
+ .handler_with_mask = sysrq_handle_reboot,
+ .help_msg = "reBoot",
+ .action_msg = "Resetting",
+ .enable_mask = SYSRQ_ENABLE_BOOT,
};

static void sysrq_handle_sync(int key, struct tty_struct *tty)
@@ -509,7 +514,10 @@ void __handle_sysrq(int key, struct tty_
if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
printk("%s\n", op_p->action_msg);
console_loglevel = orig_log_level;
- op_p->handler(key, tty);
+ if (op_p->handler_with_mask)
+ op_p->handler_with_mask(check_mask, key, tty);
+ else
+ op_p->handler(key, tty);
} else {
printk("This sysrq operation is disabled.\n");
}
--- linux-2.6.git/include/linux/sysrq.h.orig 2009-08-13 20:53:48.085786000 -0700
+++ linux-2.6.git/include/linux/sysrq.h 2009-08-13 20:54:12.625756000 -0700
@@ -32,6 +32,7 @@ struct tty_struct;

struct sysrq_key_op {
void (*handler)(int, struct tty_struct *);
+ void (*handler_with_mask)(int, int, struct tty_struct *);
char *help_msg;
char *action_msg;
int enable_mask;

--
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/