Hi,
On Tue, Dec 18, 2018 at 9:05 AM Daniel Thompson
<daniel.thomp...@linaro.org> wrote:
>
> On Sun, Dec 09, 2018 at 06:36:49PM -0800, Douglas Anderson wrote:
> > Ever since commit 5516fd7b92a7 ("debug: prevent entering debug mode on
> > panic/exception.") (yes, years ago) my kgdb workflow has been broken.
> > On Chrome OS we have 'kernel.panic = -1' in
> > '/etc/sysctl.d/00-sysctl.conf'. That means that when userspace starts
> > up it will tell the kernel "please reboot on panic". ...and so when I
> > get a panic then the system reboots instead of letting me debug it.
> >
> > While I could go in an change the 'sysctl.conf' and I could go in and
> > hack the kernel myself, these things are inconvenient. I either need
> > to keep a private kernel patch or or remember to edit a file every
> > time I install an updated version of Chrome OS. What is convenient
> > (for me) is to have a CONFIG option that makes kgdb override the panic
> > request. This is because the Chrome OS build system makes it very
> > easy for me to add some extra CONFIG "fragments" to my debug kernels.
> >
> > Hopefully having this extra config option is OK and useful to others
> > who would also prefer to make sure that kgdb is always entered on a
> > panic no matter what userspace might request.
> >
> > Signed-off-by: Douglas Anderson <diand...@chromium.org>
>
> Sorry to be late with this review. I forgot to search for "debug:" when
> I was checking for missed patches earlier.
>
> Mind you... one of the reasons I deferred review when you first sent it
> in was that my gut reaction was "I don't like it" so I decided to wait
> until I could offer a head reaction instead.
>
> Ultimately I'm not sure this should be solved within kgdb. Perhaps best
> phrased as: is the problem that kgdb *misinterprets* panic_timeout or is
> the problem that Doug wants to *override* panic_timeout?
>
> I think the answer to this question is the later meaning I'm interested
> in what happens if you introduce a CONFIG_PANIC_TIMEOUT_FORCE (c.f.
> CONFIG_CMDLINE_FORCE) to prevent the userspace changing the
> panic_timeout (either by avoiding registering the panic sysctl or, if
> that is a huge ABI problem attaching it to a different variable).
>
> TBH I'm not sure if such a patch would be accepted but I think it makes
> more semantic sense!
>
> (there is a small review comment below but the above is more important)
Thanks for the review. Yeah, it definitely was a bit of a
questionable patch but I figured I'd throw it out there to see what
folks thought. I think we should just drop it. I talked with Brian
about this offline and we agree that it actually should be OK to just
drop the setting from '00-sysctl.conf'. I have my patch at
crrev.com/c/1382879 and it looks like folks are happy with it. :-)
-Doug
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
