Re: [PATCH 3/3] printk: implement support for extended console drivers

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Geert Uytterhoeven]

Hi Tejun,

On Wed, Apr 29, 2015 at 4:45 PM, Tejun Heo <tj@kernel.org> wrote:
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c

> @@ -2196,6 +2227,7 @@ out:
>   */
>  void console_unlock(void)
>  {
> +       static char ext_text[CONSOLE_EXT_LOG_MAX];

Can you please
  a) make this feature optional,
  b) (de)allocate this buffer dynamically when the first/last console with
      CON_EXTENDED set is (un)registered?

Your patch is at the top of the bloat-o-meter output (against v4.1):

add/remove: 388/154 grow/shrink: 1309/269 up/down: 92366/-44878 (47488)
function                                     old     new   delta
ext_text                                       -    8192   +8192
do_con_trol                                    -    4824   +4824
path_openat                                 1416    4224   +2808
tvec_bases                                     4    2080   +2076
ip_do_fragment                                 -    1824   +1824
unix_stream_read_generic                       -    1452   +1452
ext4_ext_shift_extents                         -    1350   +1350
ext4_insert_range                              -    1174   +1174
bpf_prepare_filter                           264    1430   +1166
proc_pid_cmdline_read                          -    1020   +1020

and unlike the others, this one is not that difficult to fix.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Tejun Heo]

Hello,

On Mon, Jun 29, 2015 at 11:20:41AM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 29, 2015 at 4:45 PM, Tejun Heo <tj@kernel.org> wrote:
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> 
> > @@ -2196,6 +2227,7 @@ out:
> >   */
> >  void console_unlock(void)
> >  {
> > +       static char ext_text[CONSOLE_EXT_LOG_MAX];
> 
> Can you please
>   a) make this feature optional,

netconsole itself is optional & modular.  I'm not sure making further
splits is called for, especially given the use cases.

>   b) (de)allocate this buffer dynamically when the first/last console with
>       CON_EXTENDED set is (un)registered?

But yeah, making the buffer allocated on demand should be simple
enough.  Will get to it.

Thanks.

-- 
tejun

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Geert Uytterhoeven]

Hi Tejun,

On Mon, Jun 29, 2015 at 5:28 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Jun 29, 2015 at 11:20:41AM +0200, Geert Uytterhoeven wrote:
>> On Wed, Apr 29, 2015 at 4:45 PM, Tejun Heo <tj@kernel.org> wrote:
>> > --- a/kernel/printk/printk.c
>> > +++ b/kernel/printk/printk.c
>>
>> > @@ -2196,6 +2227,7 @@ out:
>> >   */
>> >  void console_unlock(void)
>> >  {
>> > +       static char ext_text[CONSOLE_EXT_LOG_MAX];
>>
>> Can you please
>>   a) make this feature optional,
>
> netconsole itself is optional & modular.  I'm not sure making further
> splits is called for, especially given the use cases.

It could be a hidden option, selected by its users (e.g. netconsole).

>>   b) (de)allocate this buffer dynamically when the first/last console with
>>       CON_EXTENDED set is (un)registered?
>
> But yeah, making the buffer allocated on demand should be simple
> enough.  Will get to it.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Tejun Heo]

On Mon, Jun 29, 2015 at 05:47:49PM +0200, Geert Uytterhoeven wrote:
> > netconsole itself is optional & modular.  I'm not sure making further
> > splits is called for, especially given the use cases.
> 
> It could be a hidden option, selected by its users (e.g. netconsole).

Hmmm... what do you mean?

Thanks.

-- 
tejun

Re: [PATCH 3/3] printk: implement support for extended console drivers

[josh]

On Mon, Jun 29, 2015 at 11:49:14AM -0400, Tejun Heo wrote:
> On Mon, Jun 29, 2015 at 05:47:49PM +0200, Geert Uytterhoeven wrote:
> > > netconsole itself is optional & modular.  I'm not sure making further
> > > splits is called for, especially given the use cases.
> > 
> > It could be a hidden option, selected by its users (e.g. netconsole).
> 
> Hmmm... what do you mean?

config PRINTK_BITS_THAT_NETCONSOLE_NEEDS

(no help text or prompt)

config NETCONSOLE
    select PRINTK_BITS_THAT_NETCONSOLE_NEEDS

That would ensure that the bits added to printk don't get compiled in
unless CONFIG_NETCONSOLE=y.

- Josh Triplett

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Geert Uytterhoeven]

On Mon, Jun 29, 2015 at 5:49 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Jun 29, 2015 at 05:47:49PM +0200, Geert Uytterhoeven wrote:
>> > netconsole itself is optional & modular.  I'm not sure making further
>> > splits is called for, especially given the use cases.
>>
>> It could be a hidden option, selected by its users (e.g. netconsole).
>
> Hmmm... what do you mean?

init/Kconfig:

        config PRINTK_EXT_LOG
                bool

drivers/net/Kconfig:

        config NETCONSOLE
                tristate "Network console logging support"
                select PRINTK_EXT_LOG

kernel/printk/printk.c:

        void console_unlock(void)
        {
#ifdef CONFIG_PRINTK_EXT_LOG
                static char ext_text[CONSOLE_EXT_LOG_MAX];
#endif

etc.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] printk: implement support for extended console drivers

[Tejun Heo]

On Mon, Jun 29, 2015 at 06:11:40PM +0200, Geert Uytterhoeven wrote:
> On Mon, Jun 29, 2015 at 5:49 PM, Tejun Heo <tj@kernel.org> wrote:
> > On Mon, Jun 29, 2015 at 05:47:49PM +0200, Geert Uytterhoeven wrote:
> >> > netconsole itself is optional & modular.  I'm not sure making further
> >> > splits is called for, especially given the use cases.
> >>
> >> It could be a hidden option, selected by its users (e.g. netconsole).
> >
> > Hmmm... what do you mean?
> 
> init/Kconfig:
> 
>         config PRINTK_EXT_LOG
>                 bool
> 
> drivers/net/Kconfig:
> 
>         config NETCONSOLE
>                 tristate "Network console logging support"
>                 select PRINTK_EXT_LOG
> 
> kernel/printk/printk.c:
> 
>         void console_unlock(void)
>         {
> #ifdef CONFIG_PRINTK_EXT_LOG
>                 static char ext_text[CONSOLE_EXT_LOG_MAX];
> #endif

OIC, hmmm... yeah, I think doing it on-demand would be better but will
try to find out which way is better.

Thanks!

-- 
tejun

Re: [PATCH 3/3] printk: implement support for extended console drivers

[josh]

On Mon, Jun 29, 2015 at 12:13:55PM -0400, Tejun Heo wrote:
> On Mon, Jun 29, 2015 at 06:11:40PM +0200, Geert Uytterhoeven wrote:
> > On Mon, Jun 29, 2015 at 5:49 PM, Tejun Heo <tj@kernel.org> wrote:
> > > On Mon, Jun 29, 2015 at 05:47:49PM +0200, Geert Uytterhoeven wrote:
> > >> > netconsole itself is optional & modular.  I'm not sure making further
> > >> > splits is called for, especially given the use cases.
> > >>
> > >> It could be a hidden option, selected by its users (e.g. netconsole).
> > >
> > > Hmmm... what do you mean?
> > 
> > init/Kconfig:
> > 
> >         config PRINTK_EXT_LOG
> >                 bool
> > 
> > drivers/net/Kconfig:
> > 
> >         config NETCONSOLE
> >                 tristate "Network console logging support"
> >                 select PRINTK_EXT_LOG
> > 
> > kernel/printk/printk.c:
> > 
> >         void console_unlock(void)
> >         {
> > #ifdef CONFIG_PRINTK_EXT_LOG
> >                 static char ext_text[CONSOLE_EXT_LOG_MAX];
> > #endif
> 
> OIC, hmmm... yeah, I think doing it on-demand would be better but will
> try to find out which way is better.

Allocating the buffer dynamically is fine, but in that case the code to
do so should ideally be compiled out.  Since printk is used by almost
*all* kernels, while netconsole is not, it's more critical to avoid
allowing printk's size to balloon.

- Josh Triplett

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

[Tejun Heo]

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/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.
--- 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
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -84,6 +84,10 @@ static struct lockdep_map console_lock_d
 };
 #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,25 @@ static struct lockdep_map console_lock_d
  */
 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) { }
+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 +2247,7 @@ out:
  */
 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 +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();
+	}
 
 	if (newcon->flags & CON_PRINTBUFFER) {
 		/*
@@ -2638,7 +2663,7 @@ int unregister_console(struct console *c
 	}
 
 	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

Re: [PATCH v4.2-rc1] printk: make extended printk support conditional on netconsole

[Petr Mladek]

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

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

[Tejun Heo]

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(-)

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

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

[Petr Mladek]

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

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

[Tejun Heo]

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.

-- 
tejun

[PATCH v3 v4.2-rc1] printk: make extended printk support conditional on netconsole

[Tejun Heo]

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.

v3: Moved ext console enable info messages into the inc function.

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

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..841ea4d 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,33 @@ static struct lockdep_map console_lock_dep_map = {
  */
 static int nr_ext_console_drivers;
 
+static void inc_nr_ext_console_drivers(void)
+{
+	if (!nr_ext_console_drivers++)
+		pr_info("printk: continuation disabled due to ext consoles, expect more fragments in /dev/kmsg\n");
+}
+
+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 +2255,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;
@@ -2562,8 +2593,7 @@ void register_console(struct console *newcon)
 	}
 
 	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 +2668,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