public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <anton.vorontsov@linaro.org>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Colin Cross <ccross@android.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kees Cook <keescook@chromium.org>,
	Tony Luck <tony.luck@intel.com>, Arnd Bergmann <arnd@arndb.de>,
	John Stultz <john.stultz@linaro.org>,
	Shuah Khan <shuahkhan@gmail.com>,
	arve@android.com, Rebecca Schultz Zavin <rebecca@android.com>,
	Jesper Juhl <jj@chaosbits.net>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Thomas Meyer <thomas@m3y3r.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marco Stornelli <marco.stornelli@gmail.com>,
	WANG Cong <xiyou.wangcong@gmail.com>,
	linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	linaro-kernel@lists.linaro.org, patches@linaro.org,
	kernel-team@android.com, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: [PATCH/RFC] Make sure linux_banner is the first message in log_buf
Date: Tue, 22 May 2012 19:35:16 -0700	[thread overview]
Message-ID: <20120523023513.GA22235@lizard> (raw)
In-Reply-To: <CAMbhsRQPYzXregvvgQZD1dV_m4tgx0hQqnYPyaO3mV0hRpc5JQ@mail.gmail.com>

For scripting it is important to have a consistent header that tells
where the kernel log starts. At least to me it always seemed that
linux_banner served this purpose.

Though, an early kernel code may print things before the Linux banner.
When we parse logs from multiple boots grabbed from serial or pstore
consoles, we might miss these messages.

The only bulletproof, arch-independent and whatnot way to ensure that
we have the banner printed first is to print it from the printk itself.
The resulting code looks quite obvious.

Sure, this doesn't address bootloader or low-level very early arch-
dependent console output that goes to the HW directly, but there's
nothing we can do about it (in pstore console we don't see them anyway).

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---

On Tue, May 22, 2012 at 11:06:29AM -0700, Colin Cross wrote:
> >
> > Now to get the only most recent messages we can do:
> >
> >        tac ramoops-console | sed '/^Linux version.*(.*@.*)/ q' | tac
> 
> Lots of problems with this.
> "Linux version ..." is not the first line in the console log on my
> devices, there are messages before it that shouldn't be dropped by
> automated logs collectors using this regexp.

Ouch. This seems not right. And the same issue exist if we collect
logs from e.g. serial console, there (unlike to just reading /proc/kmsg)
we don't know where current kernel's messages start, and more than
that, in serial console case we don't know where the messages end
(unlike to pstore).

So it's a general problem, not only related to pstore.

But we can fix this. How about the patch below?

> There is a timestamp before "Linux version", so the regexp never matches.

This is also fixable.

tac ramoops-console | sed '/\(^\|] \)Linux version.*(.*@.*)/ q' | tac

> There is often no newline at the end of the old log, so if "Linux
> version" was the first line in the log, it would still not get
> matched.

Yeah, true. We will have to add at least a new line in pstore.

> Relying on the first line in the log to not change seems likely to
> cause problems for scripts in the future.  Why not separate them where
> the code knows for sure that the old log is ending and the new log is
> starting?

I'm not opposed to a header (or a footer) at all, and you're right,
one of them is obviously needed.

I just don't like introducing yet another ad-hoc header format that
we will only use for pstore/console. We have "start of the Linux
kernel log" header, so let's try to use it?..

If it is not suitable for us today, let's think how to fix that, and
if anything, we can fall-back to your plan, i.e. adding our own footer
(or zapping prz) at boot time.

But so far I can see this solution:

1. Make sure we printk linux_banner as the first string in the
   log_buf (this patch). This is an improvement not only for pstore;

2. In pstore, add a newline to previous' boot log (we'll add it
   unconditinally, so that it we won't loose the information about
   the last new line :-).

 init/main.c     |    1 -
 kernel/printk.c |    3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/init/main.c b/init/main.c
index 44b2433..df3711d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -492,7 +492,6 @@ asmlinkage void __init start_kernel(void)
 	tick_init();
 	boot_cpu_init();
 	page_address_init();
-	printk(KERN_NOTICE "%s", linux_banner);
 	setup_arch(&command_line);
 	mm_init_owner(&init_mm, &init_task);
 	mm_init_cpumask(&init_mm);
diff --git a/kernel/printk.c b/kernel/printk.c
index b663c2c..1e1461b 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -749,6 +749,9 @@ asmlinkage int printk(const char *fmt, ...)
 	va_list args;
 	int r;
 
+	/* Make sure linux_banner is kernel's first message. */
+	printk_once(KERN_NOTICE "%s", linux_banner);
+
 #ifdef CONFIG_KGDB_KDB
 	if (unlikely(kdb_trap_printk)) {
 		va_start(args, fmt);
-- 
1.7.9.2


  reply	other threads:[~2012-05-23  2:37 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-22 14:17 [PATCH v4 0/16] Merge ram_console into pstore, and more Anton Vorontsov
2012-05-22 14:17 ` [PATCH 01/16] pstore/inode: Make pstore_fill_super() static Anton Vorontsov
2012-05-22 15:24   ` Kees Cook
2012-05-22 14:17 ` [PATCH 02/16] pstore/ram: Should update old dmesg buffer before reading Anton Vorontsov
2012-05-22 15:25   ` Kees Cook
2012-05-22 14:17 ` [PATCH 03/16] pstore/ram_core: Do not reset restored zone's position and size Anton Vorontsov
2012-05-22 18:06   ` Colin Cross
2012-05-23  2:35     ` Anton Vorontsov [this message]
2012-05-23  2:50       ` [PATCH/RFC] Make sure linux_banner is the first message in log_buf Linus Torvalds
2012-05-23  2:55       ` David Miller
2012-05-23  3:59         ` Anton Vorontsov
2012-05-22 14:17 ` [PATCH 04/16] pstore/ram_core: Factor persistent_ram_zap() out of post_init() Anton Vorontsov
2012-05-22 15:26   ` Kees Cook
2012-05-22 14:17 ` [PATCH 05/16] pstore/ram: Should zap persistent zone on unlink Anton Vorontsov
2012-05-22 15:27   ` Kees Cook
2012-05-22 14:17 ` [PATCH 06/16] pstore: Add console log messages support Anton Vorontsov
2012-05-22 14:17 ` [PATCH 07/16] pstore/ram: Introduce ramoops_context.max_dump_count Anton Vorontsov
2012-05-22 15:27   ` Kees Cook
2012-05-22 14:17 ` [PATCH 08/16] pstore/ram: Factor dmesg przs initialization out of probe() Anton Vorontsov
2012-05-22 15:29   ` Kees Cook
2012-05-22 14:17 ` [PATCH 09/16] pstore/ram: Factor ramoops_get_dump_prz() out of ramoops_pstore_read() Anton Vorontsov
2012-05-22 15:32   ` Kees Cook
2012-05-22 14:17 ` [PATCH 10/16] pstore/ram: Add console messages handling Anton Vorontsov
2012-05-22 15:37   ` Kees Cook
2012-05-22 14:17 ` [PATCH 11/16] pstore/ram_core: Silence some printks Anton Vorontsov
2012-05-22 14:17 ` [PATCH 12/16] pstore/ram: Add some more documentation and examples Anton Vorontsov
2012-05-22 14:17 ` [PATCH 13/16] staging/android: Remove ram_console driver Anton Vorontsov
2012-05-22 14:17 ` [PATCH 14/16] pstore/ram_core: Remove now unused code Anton Vorontsov
2012-05-22 14:17 ` [PATCH 15/16] pstore/platform: Make automatic updates interval configurable Anton Vorontsov
2012-05-22 15:39   ` Kees Cook
2012-05-22 14:17 ` [PATCH 16/16] pstore/platform: Disable automatic updates by default Anton Vorontsov
2012-05-22 15:42   ` Kees Cook
2012-05-22 18:19 ` [PATCH v4 0/16] Merge ram_console into pstore, and more Greg Kroah-Hartman
2012-05-22 18:33   ` Kees Cook
2012-05-22 19:04     ` Greg Kroah-Hartman
2012-05-22 19:09       ` Kees Cook

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=20120523023513.GA22235@lizard \
    --to=anton.vorontsov@linaro.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=arve@android.com \
    --cc=ccross@android.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jj@chaosbits.net \
    --cc=john.stultz@linaro.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marco.stornelli@gmail.com \
    --cc=mingo@elte.hu \
    --cc=patches@linaro.org \
    --cc=rdunlap@xenotime.net \
    --cc=rebecca@android.com \
    --cc=rusty@rustcorp.com.au \
    --cc=sboyd@codeaurora.org \
    --cc=shuahkhan@gmail.com \
    --cc=thomas@m3y3r.de \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xiyou.wangcong@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox