diff mbox

[v2,v4.2-rc1] printk: make extended printk support conditional on netconsole

Message ID 20150702162141.GC30677@mtj.duckdns.org
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Tejun Heo July 2, 2015, 4:21 p.m. UTC
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.

v2: Added a warning for cases where CON_EXTENDED is requested while
    CONFIG_PRINTK_CON_EXTENDED is disabled as suggested by Petr.

Cc: Petr Mladek <pmladek@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-and-suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/Kconfig    |    1 +
 init/Kconfig           |    3 +++
 kernel/printk/printk.c |   40 ++++++++++++++++++++++++++++++++++++----
 3 files changed, 40 insertions(+), 4 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Petr Mladek July 3, 2015, 2:07 p.m. UTC | #1
On Thu 2015-07-02 12:21:41, 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.
> 
> v2: Added a warning for cases where CON_EXTENDED is requested while
>     CONFIG_PRINTK_CON_EXTENDED is disabled as suggested by Petr.
> 
> Cc: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-and-suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reviewed-by: Petr Mladek <pmladek@suse.cz>

There is still one small thing that might get improved, see below.
But it is not that important and might be solved later. I am sorry
for not noticing it last time.


> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index cf8c242..f719118 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -84,6 +84,10 @@ static struct lockdep_map console_lock_dep_map = {
>  };
>  #endif
>  
> +#ifdef CONFIG_PRINTK_CON_EXTENDED
> +
> +#define CONSOLE_EXT_LOG_BUF_LEN		CONSOLE_EXT_LOG_MAX
> +
>  /*
>   * Number of registered extended console drivers.
>   *
> @@ -96,6 +100,32 @@ static struct lockdep_map console_lock_dep_map = {
>   */
>  static int nr_ext_console_drivers;
>  
> +static void inc_nr_ext_console_drivers(void)
> +{
> +	nr_ext_console_drivers++;
> +}
> +
> +static void dec_nr_ext_console_drivers(void)
> +{
> +	nr_ext_console_drivers--;
> +}
> +
> +#else	/* CONFIG_PRINTK_CON_EXTENDED */
> +
> +#define CONSOLE_EXT_LOG_BUF_LEN		0
> +#define nr_ext_console_drivers		0
> +
> +static void inc_nr_ext_console_drivers(void)
> +{
> +	WARN_ONCE(true, "printk: CON_EXTENDED requested when !CONFIG_PRINTK_CON_EXTENDED\n");
> +}
> +
> +static void dec_nr_ext_console_drivers(void)
> +{
> +}
> +
> +#endif	/* CONFIG_PRINTK_CON_EXTENDED */
> +
>  /*
>   * Helper macros to handle lockdep when locking/unlocking console_sem. We use
>   * macros instead of functions so that _RET_IP_ contains useful information.
> @@ -2561,9 +2591,11 @@ void register_console(struct console *newcon)
>  		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");

I would move the check and the message into
inc_nr_ext_console_drivers() when CONFIG_PRINTK_CON_EXTENDED is
defined. It does not make sense if we do not increment the counter.

Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo July 3, 2015, 3:25 p.m. UTC | #2
On Fri, Jul 03, 2015 at 04:07:34PM +0200, Petr Mladek wrote:
> > @@ -2561,9 +2591,11 @@ void register_console(struct console *newcon)
> >  		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");
> 
> I would move the check and the message into
> inc_nr_ext_console_drivers() when CONFIG_PRINTK_CON_EXTENDED is
> defined. It does not make sense if we do not increment the counter.

It doesn't make any difference as it gets compiled out anyway but yeah
moving it into the inc function makes more sense.  Updating the patch.

Thanks.
diff mbox

Patch

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 019fcef..39587f1 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -195,6 +195,7 @@  config GENEVE
 
 config NETCONSOLE
 	tristate "Network console logging support"
+	select PRINTK_CON_EXTENDED
 	---help---
 	If you want to log kernel messages over the network, enable this.
 	See <file:Documentation/networking/netconsole.txt> for details.
diff --git a/init/Kconfig b/init/Kconfig
index bcc41bd..cd281ab 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1438,6 +1438,9 @@  config PRINTK
 	  very difficult to diagnose system problems, saying N here is
 	  strongly discouraged.
 
+config PRINTK_CON_EXTENDED
+	bool
+
 config BUG
 	bool "BUG() support" if EXPERT
 	default y
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index cf8c242..f719118 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -84,6 +84,10 @@  static struct lockdep_map console_lock_dep_map = {
 };
 #endif
 
+#ifdef CONFIG_PRINTK_CON_EXTENDED
+
+#define CONSOLE_EXT_LOG_BUF_LEN		CONSOLE_EXT_LOG_MAX
+
 /*
  * Number of registered extended console drivers.
  *
@@ -96,6 +100,32 @@  static struct lockdep_map console_lock_dep_map = {
  */
 static int nr_ext_console_drivers;
 
+static void inc_nr_ext_console_drivers(void)
+{
+	nr_ext_console_drivers++;
+}
+
+static void dec_nr_ext_console_drivers(void)
+{
+	nr_ext_console_drivers--;
+}
+
+#else	/* CONFIG_PRINTK_CON_EXTENDED */
+
+#define CONSOLE_EXT_LOG_BUF_LEN		0
+#define nr_ext_console_drivers		0
+
+static void inc_nr_ext_console_drivers(void)
+{
+	WARN_ONCE(true, "printk: CON_EXTENDED requested when !CONFIG_PRINTK_CON_EXTENDED\n");
+}
+
+static void dec_nr_ext_console_drivers(void)
+{
+}
+
+#endif	/* CONFIG_PRINTK_CON_EXTENDED */
+
 /*
  * Helper macros to handle lockdep when locking/unlocking console_sem. We use
  * macros instead of functions so that _RET_IP_ contains useful information.
@@ -2224,7 +2254,7 @@  static void console_cont_flush(char *text, size_t size)
  */
 void console_unlock(void)
 {
-	static char ext_text[CONSOLE_EXT_LOG_MAX];
+	static char ext_text[CONSOLE_EXT_LOG_BUF_LEN];
 	static char text[LOG_LINE_MAX + PREFIX_MAX];
 	static u64 seen_seq;
 	unsigned long flags;
@@ -2561,9 +2591,11 @@  void register_console(struct console *newcon)
 		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();
+	}
 
 	if (newcon->flags & CON_PRINTBUFFER) {
 		/*
@@ -2638,7 +2670,7 @@  int unregister_console(struct console *console)
 	}
 
 	if (!res && (console->flags & CON_EXTENDED))
-		nr_ext_console_drivers--;
+		dec_nr_ext_console_drivers();
 
 	/*
 	 * If this isn't the last console and it has CON_CONSDEV set, we