Bug 14102

Hello - this issue has been discussed on the mtd mailing list but I wanted to raise it here as I believe it is the right thing to do, mostly because from what I can tell, the mtd oops code is not behaving correctly when executing on a non pre-emptive kernel (my system - linux 2.6.29 m68k coldfire with MMU, non preemptive kernel).

The full email thread can be found here:
http://lists.infradead.org/pipermail/linux-mtd/2009-August/027063.html

I'm not completely sure regarding the etiquette of pasting the email discussion into bugzilla, so if this is required I apologise. I appreciate that all the information should go into a bug entry so if lack of this is preventing the bug being addressed, please let me know.

Basically:

I'm trying to use the mtd oops feature on a non pre-emptible m68k
coldfire Linux kernel. Problem is, there is nothing being written to
flash upon a kernel panic. I'm passing the correct syntax on the command
line and I can see the kernel messages indicating that everything is ok, ie:

/ # dmesg | grep -i mtd
[    0.000000] Kernel command line: console=ttyS0,115200 ip=dhcp
root=/dev/nfs nfsroot=192.168.0.2:/home/matt/nfs/evb/rootfs/ rw
mtdparts=physmap-flash.0:4M@0x80000(kernel)ro,5M@0x480000(r
amdisk),-@0x980000(writeable) console=ttyMTD2
[    0.336920] console [ttyMTD2] enabled
[    0.933655] 3 cmdlinepart partitions found on MTD device physmap-flash.0
[    0.940745] Creating 3 MTD partitions on "physmap-flash.0":
[    0.951893] mtd: Giving out device 0 to kernel
[    0.965760] mtd: Giving out device 1 to ramdisk
[    0.979124] mtd: Giving out device 2 to writeable
[    0.993942] mtdoops: Ready 8, 9 (no erase)
[    0.993979] mtdoops: Attached to MTD device 2

The issue appears to be that the call to schedule_work() in
mtdoops_console_sync() does not result in mtdoops_workfunc_write() being
invoked to perform the flash write.

I understand that there is only a very small window in which to actually
flush the buffer to flash (involving a spinlock and testing
oops_in_progress). However, from what I can tell, on a non pre-emptible
kernel, after panic() does it's business of calling bust_spinlocks(),
which calls console_unblank(), which calls mtdoops_console_sync(), which
calls schedule_work(), there is no way [certainly that I can see] that
the system can context switch to the worker thread in order to invoke
the routine that actually writes to flash.

The only way I have been able to get the flash to be written to upon
panic is to make the following change:

diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 1a6b3be..2d734e2 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -335,7 +335,7 @@ static void mtdoops_console_sync(void)
                /* Interrupt context, we're going to panic so try and log */
                mtdoops_write(cxt, 1);
        else
-               schedule_work(&cxt->work_write);
+               mtdoops_write(cxt, 0);
 }

 static void

I can pull out the logs just fine in user space using oopslog, courtesy
of Mr Purdie.

I can replicate the a similar situation on my system with a simple kernel
module. The module_init routine uses INIT_WORK() to prepare a routine for execution in the work thread context. The routine simply invokes panic(). Following INIT_WORK(), schedule_work() is used to set up the routine for execution. Immediately after the call to schedule_work() the code then sits in an endless for loop with an mdelay(1).

Running on my non-preemptive kernel, when the module is insmod, the system hangs and does not panic. If schedule() is called after schedule_work() and before entry to the for loop, the system panics. Running on a preemptive kernel, the call to schedule() is not required in order for the system to panic.

In mtdoops.c, I have tried invoking schedule() immediately after schedule_work() in mtdoops_console_sync(). However this still results in flash not being written to when there is a panic. I realise that now there are of course other factors at play which could explain why the flash never gets
written to, (specifically related to scheduling, eg thread time slices,
quantums etc), but I wanted to point out that there certainly appears to
be a couple of cases in mtdoops.c where there should be calls to
schedule() after calls to schedule_work() - only there aren't.

Obviously calling schedule() in the context of a panic is to be avoided.

Richard (Purdie) suggested the following change for mtdoops.c:

if (mtd->panic_write && in_interrupt())
	/* Interrupt context, we're going to panic so try and log */
	mtdoops_write(cxt, 1);
else if (in_interrupt())
	/* Interrupt context but with no panic write function */
        /* We're going to crash anyway so we may as well try and log */
	mtdoops_write(cxt, 0);
else
	schedule_work(&cxt->work_write);

The mtd driver I'm using does not have a panic_write() function and from what I can tell the execution is never in interrupt context, so the call to schedule_work() is always invoked.

The only way I can get 100% reliable writes to flash upon panic in a non-preemptive system is to apply my patch above. This obviously negates the requirement for the async flash write but this mechanism was clearly implemented for a reason.