Re: [PATCH v11 4/5] core: Add kernel message dumper to call on oopses and panics

[Artem Bityutskiy <dedekind1@gmail.com>]

On Fri, 2009-10-23 at 18:53 +0300, Shargorodsky Atal
(EXT-Teleca/Helsinki) wrote:
> Hi all,
> 
> On Thu, 2009-10-22 at 08:25 +0200, ext Simon Kagstrom wrote:
> 
> > Thanks to everyone for the reviewing and suggestions!
> > 
> 
> I have a couple of questions:
> 
> 1. If somebody writes a module that uses dumper for uploading the
> oopses/panics logs via some pay-per-byte medium,  since he has no way
> to know in a module if the panic_on_oops flag is set, he'll have
> to upload both oops and the following panic, because he does not
> know for sure that the panic was caused by the oops. Hence he
> pays twice for the same information, right?

Looks like a valid point to me. Indeed, in this case we will first call
'kmsg_dump(KMSG_DUMP_OOPS)' from 'oops_exit()', and then call it from
'panic()'. And the dumper may dump the same information, which is not
nice.

I've looked briefly and tried to figure out how to fix this. But I
cannot find an clean way. And I was confused by the die/oops/etc code.
My question is, why the code does not work the following way instead?

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 2d8a371..8f322c7 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -235,13 +235,12 @@ void __kprobes oops_end(unsigned long flags, struct pt_regs *regs, int signr)
        raw_local_irq_restore(flags);
        oops_exit();

-       if (!signr)
-               return;
        if (in_interrupt())
                panic("Fatal exception in interrupt");
        if (panic_on_oops)
                panic("Fatal exception");
-       do_exit(signr);
+       if (signr)
+               do_exit(signr);
 }

 int __kprobes __die(const char *str, struct pt_regs *regs, long err)

If the code worked like this, I think the problem indicated by Atal
could be easily fixed.

> 2. We tried to use panic notifiers mechanism to print additional
> information that we want to see in mtdoops logs and it worked well,
> but having the kmsg_dump(KMSG_DUMP_PANIC) before the
> atomic_notifier_call_chain() breaks this functionality.
> Can we the call kmsg_dump() after the notifiers had been invoked?

Atal, I think you should just attach your patch, it is easier to express
what you mean.

But for me it looks like atomic_notifier_call_chain() can be moved up.

Anyway, please, show your patch.

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