Re: [PATCH] sysrq: Simplify sysrq-c handler

[Vivek Goyal]

On Tue, May 05, 2009 at 09:45:47AM -0400, Neil Horman wrote:
> Currently the sysrq-c handler is bit over-engineered.  Its behavior is dependent
> on a few compile time and run time factors that alter its behavior which is
> really unnecessecary.  If CONFIG_KEXEC is not configured, sysrq-c, crashes the
> system with a NULL pointer dereference.  If CONFIG_KEXEC is configured, it calls
> crash_kexec directly, which implies that the kexec kernel will either be booted
> (if its been previously loaded), or it will simply do nothing (the no kexec
> kernel has been loaded).  It would be much easier to just simplify the whole
> thing to dereference a NULL pointer all the time regardless of configuration.
> That way, it will always try to crash the system, and if a kexec kernel has been
> loaded into reserved space, it will still boot from the page fault trap handler
> (assuming panic_on_oops is set appropriately).
> 

Neil,

Would it make sense to call panic() directly so that we are not dependent
on panic_on_oops being set?

Thanks
Vivek

> Neil
> 
> Signed-off-by: Neil Horman <nhorman@xxxxxxxxxxxxx>
> 
> 
>  sysrq.c |   15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> 
> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> index b0a6a3e..9319e75 100644
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/sysrq.c
> @@ -120,20 +120,17 @@ static struct sysrq_key_op sysrq_unraw_op = {
>  #define sysrq_unraw_op (*(struct sysrq_key_op *)0)
>  #endif /* CONFIG_VT */
>  
> -#ifdef CONFIG_KEXEC
> -static void sysrq_handle_crashdump(int key, struct tty_struct *tty)
> +static void sysrq_handle_crash(int key, struct tty_struct *tty)
>  {
> -	crash_kexec(get_irq_regs());
> +	void *killer = NULL;
> +	*killer = 1;
>  }
>  static struct sysrq_key_op sysrq_crashdump_op = {
> -	.handler	= sysrq_handle_crashdump,
> -	.help_msg	= "Crashdump",
> -	.action_msg	= "Trigger a crashdump",
> +	.handler	= sysrq_handle_crash,
> +	.help_msg	= "Crash",
> +	.action_msg	= "Trigger a crash",
>  	.enable_mask	= SYSRQ_ENABLE_DUMP,
>  };
> -#else
> -#define sysrq_crashdump_op (*(struct sysrq_key_op *)0)
> -#endif
>  
>  static void sysrq_handle_reboot(int key, struct tty_struct *tty)
>  {
--
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/