Re: [PATCH] ANDROID: staging: add userpanic-dev driver

[Woody Lin <woodylin@google.com>]

On Thu, Aug 26, 2021 at 6:54 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 26, 2021 at 06:23:53PM +0800, Woody Lin wrote:
> > On Thu, Aug 26, 2021 at 5:48 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Aug 26, 2021 at 05:28:54PM +0800, Woody Lin wrote:
> > > > Add char device driver 'userpanic-dev' that exposes an interface to
> > > > userspace processes to request a system panic with customized panic
> > > > message.
> > > >
> > > > Signed-off-by: Woody Lin <woodylin@google.com>
> > > > ---
> > > >  drivers/staging/android/Kconfig         |  12 +++
> > > >  drivers/staging/android/Makefile        |   1 +
> > > >  drivers/staging/android/userpanic-dev.c | 110 ++++++++++++++++++++++++
> > >
> > > Why is this in staging?  What is wrong with it that it can not just go
> > > into the real part of the kernel?  A TODO file is needed explaining what
> > > needs to be done here in order for it to be accepted.
> >
> > Got it. No more TODO for this driver and I will move it to drivers/android/.
> >
> > >
> > > But why is this really needed at all?  Why would userspace want to panic
> > > the kernel in yet-another-way?
> >
> > The idea is to panic the kernel with a panic message specified by the userspace
> > process requesting the panic. Without this the panic handler can only collect
> > panic message "sysrq triggered crash" for a panic triggered by user processes.
> > Using this driver, user processes can put an informative description -
> > process name,
> > reason ...etc. - to the panic message.
>
> What custom userspace tool is going to use this new user/kernel api and
> again, why is it needed?  Who needs to panic the kernel with a custom
> message and where is that used?

It's for Android's services. Currently there are usages like these:

* init requests panic in InitFatalReboot (abort handler).
https://android.googlesource.com/platform/system/core/+/master/init/reboot_utils.cpp#170
  android::base::WriteStringToFile("c", PROC_SYSRQ);

* llkd requests panic to recover kernel live-lock.
https://android.googlesource.com/platform/system/core/+/master/llkd/libllkd.cpp#564
  android::base::WriteStringToFd("c", sysrqTriggerFd);

* Watchdog requests panic to recover timeout-loop.
https://android.googlesource.com/platform/frameworks/base/+/master/services/core/java/com/android/server/Watchdog.java#847
  doSysRq('c');

So to improve the panic message from "sysrq triggered crash" to a more
informative one (e.g.: "Watchdog break timeout-loop", "llkd panic
live-lock"), we'd like to add this driver to expose a dedicated
interface for userspace to panic the kernel with a custom message. Later
the panic handler implemented per platform can collect the message and
use it to build the crash report. A crash report with a more readable
title (compared to the generic "sysrq triggered crash") will be easier
to categorize, triage, etc.

And the reason to submit this to upstream, instead of making it a vendor
module, is that we'd like to enable it for the early stage of "init", where
none of the kernel module has been mounted.

>
> thanks,
>
> greg k-h

Regards,
Woody