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

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, May 05, 2009 at 10:49:37AM -0400, Neil Horman wrote:
> On Tue, May 05, 2009 at 10:23:59AM -0400, Vivek Goyal wrote:
> > 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
> > 
> I think a good argument could be made for doing that, but I kind of like
> traversing the entire page fault path, specifically to make sure that an oops
> works the way they expect it too.  If nothing else, going through the entire
> oops path like I do below will (hopefully) prevent people from complaining that
> sysrq-c works, but when their custom module gets an OOPS, then never get a
> vmcore :).

Ya, traversing an OOPs path makes sense because of complaint above you
pointed out.

> 
> I'll defer to an opposing consensus of course, but I think going through the
> entire oops path is a better solution.

I am fine with traversing OOps path.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Thanks
Vivek