Message ID | 1254410216-24444-1-git-send-email-aaro.koskinen@nokia.com |
---|---|
State | New, archived |
Headers | show |
I was working on the same fix :-) On Thu, 1 Oct 2009 18:16:55 +0300 Aaro Koskinen <aaro.koskinen@nokia.com> wrote: > diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c > index 1060337..ac67833 100644 > --- a/drivers/mtd/mtdoops.c > +++ b/drivers/mtd/mtdoops.c > @@ -335,7 +335,7 @@ static void mtdoops_console_sync(void) > cxt->ready = 0; > spin_unlock_irqrestore(&cxt->writecount_lock, flags); > > - if (mtd->panic_write && in_interrupt()) > + if (mtd->panic_write && (in_interrupt() || panic_on_oops)) > /* Interrupt context, we're going to panic so try and log */ > mtdoops_write(cxt, 1); I believe we'll also need to make this module in-kernel with this change, since panic_on_oops is not exported. I've sent another patch that does these two things together. I also get problems when mtd->read is called from mtdoops_inc_counter, so my patch also skips this if we have panic_on_oops set (there is also no point since the board will hang / be restarted after that). // Simon
Hello, Simon Kagstrom wrote: > I was working on the same fix :-) > > On Thu, 1 Oct 2009 18:16:55 +0300 > Aaro Koskinen <aaro.koskinen@nokia.com> wrote: > >> diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c >> index 1060337..ac67833 100644 >> --- a/drivers/mtd/mtdoops.c >> +++ b/drivers/mtd/mtdoops.c >> @@ -335,7 +335,7 @@ static void mtdoops_console_sync(void) >> cxt->ready = 0; >> spin_unlock_irqrestore(&cxt->writecount_lock, flags); >> >> - if (mtd->panic_write && in_interrupt()) >> + if (mtd->panic_write && (in_interrupt() || panic_on_oops)) >> /* Interrupt context, we're going to panic so try and log */ >> mtdoops_write(cxt, 1); > > I believe we'll also need to make this module in-kernel with this > change, since panic_on_oops is not exported. I've sent another patch > that does these two things together. Yes, I think you are right. > I also get problems when mtd->read is called from mtdoops_inc_counter, > so my patch also skips this if we have panic_on_oops set (there is also > no point since the board will hang / be restarted after that). I think you need to call it, otherwise the ready flag does not get set and you may loose some messages? Which driver you are using? The second patch I sent for OMAP addressed this problem, basically the driver should know we are in oops and rely on very minimal functionality in read/write. A.
On Fri, 02 Oct 2009 17:30:51 +0300 Aaro Koskinen <aaro.koskinen@nokia.com> wrote: > > I also get problems when mtd->read is called from mtdoops_inc_counter, > > so my patch also skips this if we have panic_on_oops set (there is also > > no point since the board will hang / be restarted after that). > > I think you need to call it, otherwise the ready flag does not get set and > you may loose some messages? Which driver you are using? The second patch > I sent for OMAP addressed this problem, basically the driver should know we > are in oops and rely on very minimal functionality in read/write. Well, the counter will be updated on the next boot anyway by find_next_position. mtdoops_inc_counter just positions it at the next block and (if needed) erases that. So not calling it will just delay the initialization to the next boot. I did consider putting mtdoops_inc_counter on a work queue, but I think it's overkill in this case (since it probably won't get called anyway by the panic). I'm using it on an OpenRD base board, with a NAND flash (using the functions in nand_base.c). I've patched the NAND driver with Edgars implementation of panic_write from here: http://lists.infradead.org/pipermail/linux-mtd/2009-October/027447.html I think your second patch is good to have anyway. // Simon
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c index 1060337..ac67833 100644 --- a/drivers/mtd/mtdoops.c +++ b/drivers/mtd/mtdoops.c @@ -335,7 +335,7 @@ static void mtdoops_console_sync(void) cxt->ready = 0; spin_unlock_irqrestore(&cxt->writecount_lock, flags); - if (mtd->panic_write && in_interrupt()) + if (mtd->panic_write && (in_interrupt() || panic_on_oops)) /* Interrupt context, we're going to panic so try and log */ mtdoops_write(cxt, 1); else
If panic_on_oops is set, the log should be written right away. Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com> --- drivers/mtd/mtdoops.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)