Re: [PATCH] panic.c: export panic_on_oops

[Artem Bityutskiy <dedekind1@gmail.com>]

On Mon, 2009-10-12 at 08:44 -0700, Linus Torvalds wrote:
> 
> On Mon, 12 Oct 2009, Linus Torvalds wrote:
> > 
> > Don't. kfifo's aren't going to help. You're doing this at all the wrong 
> > levels ENTIRELY, and we already have the buffer you want to flush.
> 
> Btw, a few simple rules:
> 
>  - if you need to make your device look like a "console device" for 
>    dumping at oops time, you're doing things wrong. You don't want line 
>    buffered output to begin with, and you don't want to see each line, you 
>    only want this at exceptional points.
> 
>  - if you need to look at "in_interrupt()" or "panic_on_oops", you're 
>    doing things wrong.
> 
>  - if you add your own buffers, you're doing things wrong. I have 
>    CONFIG_LOG_BUF_SHIFT=18 in my kernel, so I've already got 256kB worth 
>    of memory allocated for kernel messages. Sometimes I increase that 
>    further, just because I do some silly printk debugging.
> 
> IOW, just add a very simple "flush the dmesg buffer on oops" callback to 
> the end of the oops printout code (just a single call after the oops thing 
> is now known to be in the dmesg buffers!)
> 
> It's not just oopses, btw. Maybe people would like to do this as the last 
> stage of a reboot/shutdown too. Because some of the final printouts from 
> the shutdown will never make it to disk, because 'ksyslogd' has been 
> killed, and the root filesystem has been turned read-only.

Yes, it is difficult to disagree with this. As Ingo also pointed,
pretending to be a console driver is ugly, although I think it
was a clever way to avoid touching generic code.

But mtdoops tries to solves the following problem. What if we are
oopsing in an interrupt, which interrupted the mtd driver, so we have
all the locks held, and the mtd driver is in a unexpected stage ATM? Or
what if we are oopsing in the mtd driver, or in something which was
called by the MTD driver.?

This is what all that "workqueue vs. mtd->panic_write()" logic is about,
which Ingo dislikes.

The logic of mtdoops (my interpretation):

1. OK, there was an oops, we have it in a buffer and we need to write
   it to flash now.
2. If we are in interrupt context, we have to write right now, because
   the system is about to die very soon. And must use special
   'mtd->panic_write()' call.
3. If we have 'panic_on_oops' set, the system will be stopped soon. So
   we also have to write now, because this is our last chance. And we
   also have to use 'mtd->panic_write()'.
4. Otherwise, it is probable that system will be somewhat alive, and we
   may schedule the write instead of using 'mtd->panic_write()', because
   'mtd->panic_write()' abuses locking and chip state, and will screw up
   the mtd flash driver. So we prefer to schedule the write instead.

I guess similar could be done in 'panic()' instead. Or you have a more
elegant solution?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)