All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Kagstrom <simon.kagstrom@netinsight.net>
To: "Shargorodsky Atal (EXT-Teleca/Helsinki)"
	<ext-atal.shargorodsky@nokia.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Koskinen Aaro \(Nokia-D/Helsinki\)" <aaro.koskinen@nokia.com>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Ingo Molnar <mingo@elte.hu>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH v11 4/5] core: Add kernel message dumper to call on oopses and panics
Date: Mon, 26 Oct 2009 08:41:58 +0100	[thread overview]
Message-ID: <20091026084158.0644ea85@marrow.netinsight.se> (raw)
In-Reply-To: <1256313202.5824.60.camel@atal-desktop>

On Fri, 23 Oct 2009 18:53:22 +0300
"Shargorodsky Atal (EXT-Teleca/Helsinki)" <ext-atal.shargorodsky@nokia.com> wrote:

> 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?
> 
> I can think of a couple of way to figure it out in the module
> itself, but I could not think of any clean way to do it.

This is correct, and the mtdoops driver has some provisions to handle
this. First, there is a parameter to the module to specify whether
oopses should be dumped at all - I added this for the particular case
that someone has panic_on_oops set.

Second, it does not dump oopses directly anyway, but puts it in a work
queue. That way, if panic_on_oops is set, it will store the panic but
the oops (called from the workqueue) will not get written anyway.

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

Well, it depends I think. The code currently looks like this:

	kmsg_dump(KMSG_DUMP_PANIC);
	/*
	 * If we have crashed and we have a crash kernel loaded let it handle
	 * everything else.
	 * Do we want to call this before we try to display a message?
	 */
	crash_kexec(NULL);
	[... Comments removed]
	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);

And moving kdump_msg() after crash_kexec() will make us miss the
message if we have a kexec crash kernel as well. I realise that these
two approaches might be complementary and are not likely to be used at
the same time, but it's still something to think about.

Then again, maybe it's possible to move the panic notifiers above
crash_kexec() as well, which would solve things nicely.

// Simon

WARNING: multiple messages have this Message-ID (diff)
From: Simon Kagstrom <simon.kagstrom@netinsight.net>
To: "Shargorodsky Atal (EXT-Teleca/Helsinki)" 
	<ext-atal.shargorodsky@nokia.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>,
	Artem Bityutskiy <dedekind1@gmail.com>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Koskinen Aaro (Nokia-D/Helsinki)" <aaro.koskinen@nokia.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH v11 4/5] core: Add kernel message dumper to call on oopses and panics
Date: Mon, 26 Oct 2009 08:41:58 +0100	[thread overview]
Message-ID: <20091026084158.0644ea85@marrow.netinsight.se> (raw)
In-Reply-To: <1256313202.5824.60.camel@atal-desktop>

On Fri, 23 Oct 2009 18:53:22 +0300
"Shargorodsky Atal (EXT-Teleca/Helsinki)" <ext-atal.shargorodsky@nokia.com> wrote:

> 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?
> 
> I can think of a couple of way to figure it out in the module
> itself, but I could not think of any clean way to do it.

This is correct, and the mtdoops driver has some provisions to handle
this. First, there is a parameter to the module to specify whether
oopses should be dumped at all - I added this for the particular case
that someone has panic_on_oops set.

Second, it does not dump oopses directly anyway, but puts it in a work
queue. That way, if panic_on_oops is set, it will store the panic but
the oops (called from the workqueue) will not get written anyway.

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

Well, it depends I think. The code currently looks like this:

	kmsg_dump(KMSG_DUMP_PANIC);
	/*
	 * If we have crashed and we have a crash kernel loaded let it handle
	 * everything else.
	 * Do we want to call this before we try to display a message?
	 */
	crash_kexec(NULL);
	[... Comments removed]
	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);

And moving kdump_msg() after crash_kexec() will make us miss the
message if we have a kexec crash kernel as well. I realise that these
two approaches might be complementary and are not likely to be used at
the same time, but it's still something to think about.

Then again, maybe it's possible to move the panic notifiers above
crash_kexec() as well, which would solve things nicely.

// Simon

  parent reply	other threads:[~2009-10-26  7:42 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-15  7:40 [PATCH v7 0/5]: mtdoops: fixes and improvements Simon Kagstrom
2009-10-15  7:47 ` [PATCH v7 1/5]: mtdoops: avoid erasing already empty areas Simon Kagstrom
2009-10-23  3:57   ` Artem Bityutskiy
2009-10-15  7:47 ` [PATCH v7 2/5]: mtdoops: Keep track of used/unused mtdoops pages in an array Simon Kagstrom
2009-10-23  4:08   ` Artem Bityutskiy
2009-10-15  7:47 ` [PATCH v7 3/5]: mtdoops: Make page (record) size configurable Simon Kagstrom
2009-10-23  4:13   ` Artem Bityutskiy
2009-10-23  6:58     ` Simon Kagstrom
2009-10-15  7:48 ` [PATCH v7 4/5]: core: Add kernel message dumper to call on oopses and panics Simon Kagstrom
2009-10-15  7:48   ` Simon Kagstrom
2009-10-15  9:31   ` Ingo Molnar
2009-10-15  9:31     ` Ingo Molnar
2009-10-15 14:10     ` [PATCH v8 4/5] " Simon Kagstrom
2009-10-15 14:10       ` Simon Kagstrom
2009-10-15 15:46       ` Ingo Molnar
2009-10-15 15:46         ` Ingo Molnar
2009-10-16  7:46         ` [PATCH v9 " Simon Kagstrom
2009-10-16  7:46           ` Simon Kagstrom
2009-10-16  8:09           ` Ingo Molnar
2009-10-16  8:09             ` Ingo Molnar
2009-10-16  8:24             ` Artem Bityutskiy
2009-10-16  8:24               ` Artem Bityutskiy
2009-10-16  9:25               ` [PATCH v10 " Simon Kagstrom
2009-10-16  9:25                 ` Simon Kagstrom
2009-10-16 10:10                 ` Ingo Molnar
2009-10-16 10:10                   ` Ingo Molnar
2009-10-16 11:00                   ` Artem Bityutskiy
2009-10-16 11:00                     ` Artem Bityutskiy
2009-10-16 12:57                     ` Ingo Molnar
2009-10-16 12:57                       ` Ingo Molnar
2009-10-16 12:09                   ` [PATCH v11 " Simon Kagstrom
2009-10-16 12:09                     ` Simon Kagstrom
2009-10-19 11:48                     ` Artem Bityutskiy
2009-10-19 11:48                       ` Artem Bityutskiy
2009-10-19 12:50                       ` Ingo Molnar
2009-10-19 12:50                         ` Ingo Molnar
2009-10-21 23:33                         ` Linus Torvalds
2009-10-21 23:33                           ` Linus Torvalds
2009-10-22  6:25                           ` Simon Kagstrom
2009-10-22  6:25                             ` Simon Kagstrom
2009-10-22  6:36                             ` Artem Bityutskiy
2009-10-22  6:36                               ` Artem Bityutskiy
2009-10-22  6:42                               ` Simon Kagstrom
2009-10-22  8:52                                 ` Artem Bityutskiy
2009-10-22  8:58                                   ` Simon Kagstrom
2009-10-23  7:22                               ` Ingo Molnar
2009-10-23  7:22                                 ` Ingo Molnar
2009-10-23 15:53                             ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-23 15:53                               ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-24 17:05                               ` Artem Bityutskiy
2009-10-24 17:05                                 ` Artem Bityutskiy
2009-10-26 10:40                                 ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26 10:40                                   ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26  7:41                               ` Simon Kagstrom [this message]
2009-10-26  7:41                                 ` Simon Kagstrom
2009-10-26 10:36                                 ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26 10:36                                   ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26 11:53                                   ` Simon Kagstrom
2009-10-26 11:53                                     ` Simon Kagstrom
2009-10-26 15:13                                     ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26 15:13                                       ` Shargorodsky Atal (EXT-Teleca/Helsinki)
2009-10-26 15:37                                       ` Simon Kagstrom
2009-10-26 15:37                                         ` Simon Kagstrom
2009-10-16 11:25                 ` [PATCH v10 " Aaro Koskinen
2009-10-16 11:25                   ` Aaro Koskinen
2009-10-15  7:48 ` [PATCH v7 5/5]: mtdoops: refactor as a kmsg_dumper Simon Kagstrom
2009-10-15 14:10   ` [PATCH v8 " Simon Kagstrom
2009-10-16  7:46     ` [PATCH v9 " Simon Kagstrom
2009-10-23  4:32   ` [PATCH v7 " Artem Bityutskiy
2009-10-23  6:34     ` Simon Kagstrom
2010-01-06 14:33   ` David Woodhouse
2010-01-07  6:47     ` Simon Kagstrom
2009-10-29 12:35 ` [PATCH v12 0/4]: mtdoops: fixes and improvements Simon Kagstrom
2009-10-29 12:41   ` [PATCH v12 1/4]: mtdoops: Keep track of used/unused mtdoops pages in an array Simon Kagstrom
2009-10-29 15:24     ` Artem Bityutskiy
2009-10-29 12:41   ` [PATCH v12 2/4]: mtdoops: Add a maximum MTD partition size Simon Kagstrom
2009-10-29 15:27     ` Artem Bityutskiy
2009-11-03  6:23     ` Artem Bityutskiy
2009-10-29 12:41   ` [PATCH v12 3/4]: mtdoops: Make page (record) size configurable Simon Kagstrom
2009-11-03  6:23     ` Artem Bityutskiy
2009-11-03  7:27       ` Simon Kagstrom
2009-11-03  7:45         ` Artem Bityutskiy
2009-10-29 12:41   ` [PATCH v12 4/4]: mtdoops: refactor as a kmsg_dumper Simon Kagstrom
2009-11-03  7:29     ` Artem Bityutskiy
2009-11-03 13:19       ` [PATCH v13 " Simon Kagstrom
2009-11-10  9:55         ` Simon Kagstrom
2009-11-10 11:53           ` Artem Bityutskiy
2009-11-10 11:58             ` Simon Kagstrom
2009-11-10 16:04         ` Artem Bityutskiy
2009-11-10 16:11         ` Artem Bityutskiy
2009-11-11  9:46           ` Simon Kagstrom
2009-11-11 10:29             ` Artem Bityutskiy
2009-11-11 11:27               ` Simon Kagstrom
2009-11-11 11:34                 ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091026084158.0644ea85@marrow.netinsight.se \
    --to=simon.kagstrom@netinsight.net \
    --cc=aaro.koskinen@nokia.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=ext-atal.shargorodsky@nokia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mingo@elte.hu \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.