[PATCH 1/2] mtdoops: do not schedule work if we are going to die

[PATCH 1/2] mtdoops: do not schedule work if we are going to die

[Aaro Koskinen]

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(-)

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
-- 
1.5.4.3

[PATCH 2/2] OneNAND: OMAP: do not use DMA if oops in progress

[Aaro Koskinen]

Otherwise we may hang if we are called from panic() through mtdoops.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 drivers/mtd/onenand/omap2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 0108ed4..b1e04ff 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -294,7 +294,7 @@ static int omap3_onenand_read_bufferram(struct mtd_info *mtd, int area,
 		goto out_copy;
 
 	/* panic_write() may be in an interrupt context */
-	if (in_interrupt())
+	if (in_interrupt() || oops_in_progress)
 		goto out_copy;
 
 	if (buf >= high_memory) {
@@ -371,7 +371,7 @@ static int omap3_onenand_write_bufferram(struct mtd_info *mtd, int area,
 		goto out_copy;
 
 	/* panic_write() may be in an interrupt context */
-	if (in_interrupt())
+	if (in_interrupt() || oops_in_progress)
 		goto out_copy;
 
 	if (buf >= high_memory) {
-- 
1.5.4.3

Re: [PATCH 1/2] mtdoops: do not schedule work if we are going to die

[Simon Kagstrom]

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

Re: [PATCH 1/2] mtdoops: do not schedule work if we are going to die

[Aaro Koskinen]

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.

Re: [PATCH 1/2] mtdoops: do not schedule work if we are going to die

[Simon Kagstrom]

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