Re: [PATCH] powerpc/xmon: add debugfs entry for xmon

["Guilherme G. Piccoli" <gpiccoli@linux.vnet.ibm.com>]

On 14/02/2017 09:37, Michael Ellerman wrote:
> "Guilherme G. Piccoli" <gpiccoli@linux.vnet.ibm.com> writes:
> 
>> Currently the xmon debugger is set only via kernel boot command-line.
>> It's disabled by default, and can be enabled with "xmon=on" on the
>> command-line. Also, xmon may be accessed via sysrq mechanism, but once
>> we enter xmon via sysrq,  it's  kept enabled until system is rebooted,
>> even if we exit the debugger. A kernel crash will then lead to xmon
>> instance, instead of triggering a kdump procedure (if configured), for
>> example.
>>
>> This patch introduces a debugfs entry for xmon, allowing user to query
>> its current state and change it if desired. Basically, the "xmon" file
>> to read from/write to is under the debugfs mount point, on powerpc
>> directory. Reading this file will provide the current state of the
>> debugger, one of the following: "on", "off", "early" or "nobt". Writing
>> one of these states to the file will take immediate effect on the debugger.
> 
> I like this in general.
> 
> But I think we can simplify it a bit.
> 
> I don't think we need the nobt state anymore. As far as I can see it was
> added as a way to reinstate the old behaviour when the auto backtrace
> mode was added, but I've never heard of anyone using it.
> 
> If anyone hits a crash where they really need that feature they can
> always just hack the code to disable the backtrace.
> 
> So I think step 1 is a patch to drop the xmon_no_auto_backtrace stuff.
> 
> Also I'm not sure EARLY needs to be a separate state. It's just a
> one-off invocation at boot, all we really need is just a single bit of
> state communicated from early_parse_xmon() to xmon_setup(), ie. a static
> bool would do.
> 
> If we agree with that, then there's only two states left, on and off, in
> which case it can probably just be an int - and we can use a simple
> attribute file rather than custom parsing.
> 

Thanks for the good suggestions Michael, I'll change the code and submit
a new version.


>> * I had this patch partially done for some time, and after a discussion
>> at the kernel slack channel latest week, I decided to rebase and fix
>> some remaining bugs. I'd change 'x' option to always disable the debugger,
> 
> Not quite.
> 
> 'x' should exit and leave xmon in whatever state it was previously in.

Agreed!

Thanks,


Guilherme

> 
> cheers
>