All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Tejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Kay Sievers <kay@vrfy.org>, Josh Triplett <josh@joshtriplett.org>,
	Linux Embedded <linux-embedded@vger.kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH v4.2-rc1] printk: make extended printk support conditional on netconsole
Date: Wed, 1 Jul 2015 18:05:13 +0200	[thread overview]
Message-ID: <20150701160513.GC8099@pathway.suse.cz> (raw)
In-Reply-To: <20150629233140.GA7711@mtj.duckdns.org>

On Mon 2015-06-29 19:31:40, Tejun Heo wrote:
> 6fe29354befe ("printk: implement support for extended console
> drivers") implemented extended printk support for extended netconsole.
> The code added was miniscule but it added static 8k buffer
> unconditionally unnecessarily bloating the kernel for cases where
> extended netconsole is not used.
> 
> This patch introduces CONFIG_PRINTK_CON_EXTENDED which is selected by
> CONFIG_NETCONSOLE.  If the config option is not set, extended printk
> support is compiled out along with the static buffer.
> 
> Verified 8k reduction in vmlinux bss when !CONFIG_NETCONSOLE.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-and-suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Linus, Andrew.
> 
> This removes an unnecessary 8k bss bloat introduced during v4.2-rc1
> merge window on certain configs.  The original patch was routed
> through -mm.  How should this be routed?
> 
> Thanks.
> 
>  drivers/net/Kconfig    |    1 +
>  init/Kconfig           |    3 +++
>  kernel/printk/printk.c |   33 +++++++++++++++++++++++++++++----
>  3 files changed, 33 insertions(+), 4 deletions(-)

[...]

> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
[...]
> @@ -2561,9 +2584,11 @@ void register_console(struct console *ne
>  		console_drivers->next = newcon;
>  	}
>  
> -	if (newcon->flags & CON_EXTENDED)
> -		if (!nr_ext_console_drivers++)
> +	if (newcon->flags & CON_EXTENDED) {
> +		if (!nr_ext_console_drivers)
>  			pr_info("printk: continuation disabled due to ext consoles, expect more fragments in /dev/kmsg\n");
> +		inc_nr_ext_console_drivers();

We should handle also the situation when CON_EXTENDED is set
and CONFIG_PRINTK_CON_EXTENDED is not set by mistake. Otherwise,
we will not increment nr_ext_console_drivers here => ext_text will
not be filled in console_unlock() => call_console_drivers()
will print nothing for the CON_EXTENDED console.

At least, I would print an error here. Something like.

#ifndef CONFIG_PRINTK_CON_EXTENDED
		pr_err("The registered extended console will print nothing because the kernel is not compiled with PRINTK_CON_EXTENDED\n");
#endif

I wonder if there is a good identification of the console that can be printed.

Otherwise, it looks fine to me.

Best Regards,
Petr

  reply	other threads:[~2015-07-01 16:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-29 14:45 [PATCHSET] printk: implement extended console support Tejun Heo
2015-04-29 14:45 ` [PATCH 1/3] printk: guard the amount written per line by devkmsg_read() Tejun Heo
2015-04-29 14:45 ` [PATCH 2/3] printk: factor out message formatting from devkmsg_read() Tejun Heo
2015-04-29 14:45 ` [PATCH 3/3] printk: implement support for extended console drivers Tejun Heo
2015-04-30 21:31   ` Andrew Morton
2015-04-30 22:12     ` Tejun Heo
2015-04-30 22:15       ` Andrew Morton
2015-05-01 16:23   ` [PATCH v3 " Tejun Heo
2015-06-29  9:20   ` [PATCH " Geert Uytterhoeven
2015-06-29 15:28     ` Tejun Heo
2015-06-29 15:47       ` Geert Uytterhoeven
2015-06-29 15:49         ` Tejun Heo
2015-06-29 16:11           ` josh
2015-06-29 16:11           ` Geert Uytterhoeven
2015-06-29 16:13             ` Tejun Heo
2015-06-29 16:50               ` josh
2015-06-29 23:31               ` [PATCH v4.2-rc1] printk: make extended printk support conditional on netconsole Tejun Heo
2015-07-01 16:05                 ` Petr Mladek [this message]
2015-07-02 16:21                 ` [PATCH v2 " Tejun Heo
2015-07-03 14:07                   ` Petr Mladek
2015-07-03 15:25                     ` Tejun Heo
2015-07-03 16:22                   ` [PATCH v3 " Tejun Heo

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=20150701160513.GC8099@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=josh@joshtriplett.org \
    --cc=kay@vrfy.org \
    --cc=linux-embedded@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tj@kernel.org \
    --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.