[PATCH] console: prevent registered consoles from dumping old kernel message over again

[PATCH] console: prevent registered consoles from dumping old kernel message over again

[Feng Tang]

For a platform with many consoles like:
 "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"

Each time when the non "selected_console" (tty1 and ttyMFD2 here) get
registered, the existing kernel message will be printed out on registered
consoles again, the "mrst" early console will get some same message for
3 times, and "tty1" will get some for twice.

So try to temporarily disable registered console's printing when dump the
existing kernel messages to the new console.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 kernel/printk.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index f64b899..ba7186b 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1245,6 +1245,7 @@ void register_console(struct console *newcon)
 	int i;
 	unsigned long flags;
 	struct console *bcon = NULL;
+	struct console *con;
 
 	/*
 	 * before we register a new CON_BOOT console, make sure we don't
@@ -1357,8 +1358,20 @@ void register_console(struct console *newcon)
 		spin_lock_irqsave(&logbuf_lock, flags);
 		con_start = log_start;
 		spin_unlock_irqrestore(&logbuf_lock, flags);
+
+		/*
+		 * Disable the existing consoles temporarily to prevent it from
+		 * printing out kernel log again.
+		 */
+		for_each_console(con)
+			if (con != newcon)
+				con->flags &= ~CON_ENABLED;
 	}
 	release_console_sem();
+
+	for_each_console(con)
+		con->flags |= CON_ENABLED;
+
 	console_sysfs_notify();
 
 	/*
-- 
1.7.0.4

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Alan Cox]

On Wed, 12 Jan 2011 14:48:46 +0800
Feng Tang <feng.tang@intel.com> wrote:

> For a platform with many consoles like:
>  "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"
> 
> Each time when the non "selected_console" (tty1 and ttyMFD2 here) get
> registered, the existing kernel message will be printed out on
> registered consoles again, the "mrst" early console will get some
> same message for 3 times, and "tty1" will get some for twice.
> 
> So try to temporarily disable registered console's printing when dump
> the existing kernel messages to the new console.

I don't think the existing behaviour should be changed because other
systems and users rely on it. Perhaps we need to be able to do

console=xxx addconsole=yyy

or some such instead so the user can indicate if the console should get
all the messages so far or just be added as an extra console 

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Feng Tang]

Hi Alan,

On Wed, 12 Jan 2011 19:14:01 +0800
Alan Cox <alan@linux.intel.com> wrote:

> On Wed, 12 Jan 2011 14:48:46 +0800
> Feng Tang <feng.tang@intel.com> wrote:
> 
> > For a platform with many consoles like:
> >  "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"
> > 
> > Each time when the non "selected_console" (tty1 and ttyMFD2 here)
> > get registered, the existing kernel message will be printed out on
> > registered consoles again, the "mrst" early console will get some
> > same message for 3 times, and "tty1" will get some for twice.
> > 
> > So try to temporarily disable registered console's printing when
> > dump the existing kernel messages to the new console.
> 
> I don't think the existing behaviour should be changed because other
> systems and users rely on it. Perhaps we need to be able to do
> 
> console=xxx addconsole=yyy
> 
> or some such instead so the user can indicate if the console should
> get all the messages so far or just be added as an extra console 


Don't know if I understand correctly, but with this patch all the consoles
still will get the full version of kernel messages, it just save some
console from dumping some of the early messages several times.

Still use the old example, assume log0 is the kernel messages from boot start to
the point where tty1 get registered, log1 is from tty1's registering to
ttyMFD2's, log2 is the rest kernel message. Before this patch, on the
boot console, we will see: log0 + log0 + log1 + log0 + log1 + log2.

Thanks,
Feng

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Alan Cox]

> Don't know if I understand correctly, but with this patch all the consoles
> still will get the full version of kernel messages, it just save some
> console from dumping some of the early messages several times.
> 
> Still use the old example, assume log0 is the kernel messages from boot start to
> the point where tty1 get registered, log1 is from tty1's registering to
> ttyMFD2's, log2 is the rest kernel message. Before this patch, on the
> boot console, we will see: log0 + log0 + log1 + log0 + log1 + log2.

Sorry misunderstood the point of the patch - yes that makes sense !

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Andrew Morton]

On Wed, 12 Jan 2011 14:48:46 +0800
Feng Tang <feng.tang@intel.com> wrote:

> For a platform with many consoles like:
>  "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"
> 
> Each time when the non "selected_console" (tty1 and ttyMFD2 here) get
> registered, the existing kernel message will be printed out on registered
> consoles again, the "mrst" early console will get some same message for
> 3 times, and "tty1" will get some for twice.
> 
> So try to temporarily disable registered console's printing when dump the
> existing kernel messages to the new console.
> 
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  kernel/printk.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index f64b899..ba7186b 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -1245,6 +1245,7 @@ void register_console(struct console *newcon)
>  	int i;
>  	unsigned long flags;
>  	struct console *bcon = NULL;
> +	struct console *con;
>  
>  	/*
>  	 * before we register a new CON_BOOT console, make sure we don't
> @@ -1357,8 +1358,20 @@ void register_console(struct console *newcon)
>  		spin_lock_irqsave(&logbuf_lock, flags);
>  		con_start = log_start;
>  		spin_unlock_irqrestore(&logbuf_lock, flags);
> +
> +		/*
> +		 * Disable the existing consoles temporarily to prevent it from
> +		 * printing out kernel log again.
> +		 */
> +		for_each_console(con)
> +			if (con != newcon)
> +				con->flags &= ~CON_ENABLED;

Is it racy?  If some CPU does an unrelated printk in the middle of all
this, might that get lost?  Perhaps not, given the way we pass the
start/end indexes into log_buf[].

>  	}
>  	release_console_sem();
> +
> +	for_each_console(con)
> +		con->flags |= CON_ENABLED;

If a console previously had CON_ENABLED unset, this will bogusly set
it.  That will (at least) defeat the intent of a previous
console_stop().

How's about something like this?

--- a/kernel/printk.c~a
+++ a/kernel/printk.c
@@ -113,6 +113,11 @@ static unsigned con_start;	/* Index into
 static unsigned log_end;	/* Index into log_buf: most-recently-written-char + 1 */
 
 /*
+ * If exclusive_console is non-NULL then only this console is to be printed to.
+ */
+static struct console *exclusive_console;
+
+/*
  *	Array of consoles built from command line options (console=)
  */
 struct console_cmdline
@@ -460,6 +465,8 @@ static void __call_console_drivers(unsig
 	struct console *con;
 
 	for_each_console(con) {
+		if (exclusive_console && con != exclusive_console)
+			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||
 				(con->flags & CON_ANYTIME)))
@@ -1358,8 +1365,15 @@ void register_console(struct console *ne
 		spin_lock_irqsave(&logbuf_lock, flags);
 		con_start = log_start;
 		spin_unlock_irqrestore(&logbuf_lock, flags);
+		/*
+		 * We're about to replay the log buffer.  Only do this to the
+		 * just-registered console to avoid excessive message spam to
+		 * the already-registered consoles.
+		 */
+		exclusive_console = newcon;
 	}
 	release_console_sem();
+	exclusive_console = NULL;
 	console_sysfs_notify();
 
 	/*
_

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Feng Tang]

On Thu, 20 Jan 2011 08:11:35 +0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 12 Jan 2011 14:48:46 +0800
> Feng Tang <feng.tang@intel.com> wrote:
> 
> > For a platform with many consoles like:
> >  "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"
> > 
> > Each time when the non "selected_console" (tty1 and ttyMFD2 here)
> > get registered, the existing kernel message will be printed out on
> > registered consoles again, the "mrst" early console will get some
> > same message for 3 times, and "tty1" will get some for twice.
> > 
> > So try to temporarily disable registered console's printing when
> > dump the existing kernel messages to the new console.
> > 
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > ---
> >  kernel/printk.c |   13 +++++++++++++
> >  1 files changed, 13 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/printk.c b/kernel/printk.c
> > index f64b899..ba7186b 100644
> > --- a/kernel/printk.c
> > +++ b/kernel/printk.c
> > @@ -1245,6 +1245,7 @@ void register_console(struct console *newcon)
> >  	int i;
> >  	unsigned long flags;
> >  	struct console *bcon = NULL;
> > +	struct console *con;
> >  
> >  	/*
> >  	 * before we register a new CON_BOOT console, make sure we
> > don't @@ -1357,8 +1358,20 @@ void register_console(struct console
> > *newcon) spin_lock_irqsave(&logbuf_lock, flags);
> >  		con_start = log_start;
> >  		spin_unlock_irqrestore(&logbuf_lock, flags);
> > +
> > +		/*
> > +		 * Disable the existing consoles temporarily to
> > prevent it from
> > +		 * printing out kernel log again.
> > +		 */
> > +		for_each_console(con)
> > +			if (con != newcon)
> > +				con->flags &= ~CON_ENABLED;
> 
> Is it racy?  If some CPU does an unrelated printk in the middle of all
> this, might that get lost?  Perhaps not, given the way we pass the
> start/end indexes into log_buf[].

The printk message should be ok, as they will fall into the log_buf[] anyway.

> 
> >  	}
> >  	release_console_sem();
> > +
> > +	for_each_console(con)
> > +		con->flags |= CON_ENABLED;
> 
> If a console previously had CON_ENABLED unset, this will bogusly set
> it.  That will (at least) defeat the intent of a previous
> console_stop().
> 
> How's about something like this?

Yes, it makes sense, I should add some check. And this new
"exclusive_console" should be better as its name shows what it is for.

Thanks,
Feng

> 
> --- a/kernel/printk.c~a
> +++ a/kernel/printk.c
> @@ -113,6 +113,11 @@ static unsigned con_start;	/* Index into
>  static unsigned log_end;	/* Index into log_buf:
> most-recently-written-char + 1 */ 
>  /*
> + * If exclusive_console is non-NULL then only this console is to be
> printed to.
> + */
> +static struct console *exclusive_console;
> +
> +/*
>   *	Array of consoles built from command line options
> (console=) */
>  struct console_cmdline
> @@ -460,6 +465,8 @@ static void __call_console_drivers(unsig
>  	struct console *con;
>  
>  	for_each_console(con) {
> +		if (exclusive_console && con != exclusive_console)
> +			continue;
>  		if ((con->flags & CON_ENABLED) && con->write &&
>  				(cpu_online(smp_processor_id()) ||
>  				(con->flags & CON_ANYTIME)))
> @@ -1358,8 +1365,15 @@ void register_console(struct console *ne
>  		spin_lock_irqsave(&logbuf_lock, flags);
>  		con_start = log_start;
>  		spin_unlock_irqrestore(&logbuf_lock, flags);
> +		/*
> +		 * We're about to replay the log buffer.  Only do
> this to the
> +		 * just-registered console to avoid excessive
> message spam to
> +		 * the already-registered consoles.
> +		 */
> +		exclusive_console = newcon;
>  	}
>  	release_console_sem();
> +	exclusive_console = NULL;
>  	console_sysfs_notify();
>  
>  	/*
> _
> 

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Andrew Morton]

On Thu, 20 Jan 2011 09:44:58 +0800
Feng Tang <feng.tang@intel.com> wrote:

> > How's about something like this?
> 
> Yes, it makes sense, I should add some check. And this new
> "exclusive_console" should be better as its name shows what it is for.

Is it racy, or does console_sem coverage save us?

Anyway.  If you think that the exclusive_console approach is suitable,
please send a tested patch back at me sometime?

Re: [PATCH] console: prevent registered consoles from dumping old kernel message over again

[Feng Tang]

On Thu, 20 Jan 2011 09:51:34 +0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 20 Jan 2011 09:44:58 +0800
> Feng Tang <feng.tang@intel.com> wrote:
> 
> > > How's about something like this?
> > 
> > Yes, it makes sense, I should add some check. And this new
> > "exclusive_console" should be better as its name shows what it is
> > for.
> 
> Is it racy, or does console_sem coverage save us?
> 
> Anyway.  If you think that the exclusive_console approach is suitable,
> please send a tested patch back at me sometime?
> 

Actually before my last email, I tested your patch which works fine which
I just forgot to mention :)

looks a little racy, so I move the "exclusive_console=NULL" to some place
protected by console_sem, and it works fine on my platform, pls review it

Thanks,
Feng


-----------------------------
rom cf1fcf9942f1ac067081572c8dc1e238674441fd Mon Sep 17 00:00:00 2001
From: Feng Tang <feng.tang@intel.com>
Date: Thu, 20 Jan 2011 10:26:21 +0800
Subject: [PATCH] console: prevent registered consoles from dumping old kernel message over again

For a platform with many consoles like:
 "console=tty1 console=ttyMFD2 console=ttyS0 earlyprintk=mrst"

Each time when the non "selected_console" (tty1 and ttyMFD2 here) get
registered, the existing kernel message will be printed out on registered
consoles again, the "mrst" early console will get some same message for
3 times, and "tty1" will get some for twice.

As suggested by Andrew Morton, everytime a new console is registered, it
will be set as the "exclusive" console which will dump the already existing
kernel message.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 kernel/printk.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 53d9a9e..ee71cc4 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -113,6 +113,11 @@ static unsigned con_start;	/* Index into log_buf: next char to be sent to consol
 static unsigned log_end;	/* Index into log_buf: most-recently-written-char + 1 */
 
 /*
+ * If exclusive_console is non-NULL then only this console is to be printed to.
+ */
+static struct console *exclusive_console;
+
+/*
  *	Array of consoles built from command line options (console=)
  */
 struct console_cmdline
@@ -460,6 +465,8 @@ static void __call_console_drivers(unsigned start, unsigned end)
 	struct console *con;
 
 	for_each_console(con) {
+		if (exclusive_console && con != exclusive_console)
+			continue;
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||
 				(con->flags & CON_ANYTIME)))
@@ -1136,6 +1143,11 @@ void release_console_sem(void)
 		local_irq_restore(flags);
 	}
 	console_locked = 0;
+
+	/* Release the exclusive_console once it is used */
+	if (unlikely(exclusive_console))
+		exclusive_console = NULL;
+
 	up(&console_sem);
 	spin_unlock_irqrestore(&logbuf_lock, flags);
 	if (wake_klogd)
@@ -1358,6 +1370,12 @@ void register_console(struct console *newcon)
 		spin_lock_irqsave(&logbuf_lock, flags);
 		con_start = log_start;
 		spin_unlock_irqrestore(&logbuf_lock, flags);
+		/*
+		 * We're about to replay the log buffer.  Only do this to the
+		 * just-registered console to avoid excessive message spam to
+		 * the already-registered consoles.
+		 */
+		exclusive_console = newcon;
 	}
 	release_console_sem();
 	console_sysfs_notify();
--