[PATCH] introduce boot_printk()

[PATCH] introduce boot_printk()

[Yinghai Lu]

could be enabled via "boot=verbose" to get more debug info

will use it to convert some printk(KERN_DEBUG ...)

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 include/linux/kernel.h |   19 +++++++++++++++++++
 kernel/printk.c        |   21 +++++++++++++++++++++
 2 files changed, 40 insertions(+)

Index: linux-2.6/include/linux/kernel.h
===================================================================
--- linux-2.6.orig/include/linux/kernel.h
+++ linux-2.6/include/linux/kernel.h
@@ -344,6 +344,25 @@ static inline char *pack_hex_byte(char *
 #endif
 
 /*
+ * Debugging macros
+ */
+#define BOOT_QUIET   0
+#define BOOT_VERBOSE 1
+#define BOOT_SPEW    2
+
+extern int boot_verbosity;
+/*
+ * Define the default level of output to be very little
+ * This can be turned up by using boot=verbose for more
+ * information and boot=spew for _lots_ of information.
+ * boot_verbosity is defined in printk.c
+ */
+#define boot_printk(v, s, a...) do {        \
+		if ((v) <= boot_verbosity)  \
+			printk(s, ##a);    \
+	} while (0)
+
+/*
  *      Display an IP address in readable format.
  */
 
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c
+++ linux-2.6/kernel/printk.c
@@ -604,6 +604,27 @@ asmlinkage int printk(const char *fmt, .
 	return r;
 }
 
+int boot_verbosity;
+
+static int __init boot_set_verbosity(char *arg)
+{
+	if (!arg)
+		return -EINVAL;
+
+	if (strcmp("spew", arg) == 0)
+		boot_verbosity = BOOT_SPEW;
+	else if (strcmp("verbose", arg) == 0)
+		boot_verbosity = BOOT_VERBOSE;
+	else {
+		printk(KERN_WARNING "boot Verbosity level %s not recognised"
+			" use boot=verbose or boot=spew\n", arg);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+early_param("boot", boot_set_verbosity);
+
 /* cpu currently holding logbuf_lock */
 static volatile unsigned int printk_cpu = UINT_MAX;
 

Re: [PATCH] introduce boot_printk()

[Pavel Machek]

On Mon 2008-09-15 01:05:16, Yinghai Lu wrote:
> could be enabled via "boot=verbose" to get more debug info
> 
> will use it to convert some printk(KERN_DEBUG ...)
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

boot= is a bit too generic name, I guess...
								Pavel

> ---
>  include/linux/kernel.h |   19 +++++++++++++++++++
>  kernel/printk.c        |   21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> Index: linux-2.6/include/linux/kernel.h
> ===================================================================
> --- linux-2.6.orig/include/linux/kernel.h
> +++ linux-2.6/include/linux/kernel.h
> @@ -344,6 +344,25 @@ static inline char *pack_hex_byte(char *
>  #endif
>  
>  /*
> + * Debugging macros
> + */
> +#define BOOT_QUIET   0
> +#define BOOT_VERBOSE 1
> +#define BOOT_SPEW    2
> +
> +extern int boot_verbosity;
> +/*
> + * Define the default level of output to be very little
> + * This can be turned up by using boot=verbose for more
> + * information and boot=spew for _lots_ of information.
> + * boot_verbosity is defined in printk.c
> + */
> +#define boot_printk(v, s, a...) do {        \
> +		if ((v) <= boot_verbosity)  \
> +			printk(s, ##a);    \
> +	} while (0)
> +
> +/*
>   *      Display an IP address in readable format.
>   */
>  
> Index: linux-2.6/kernel/printk.c
> ===================================================================
> --- linux-2.6.orig/kernel/printk.c
> +++ linux-2.6/kernel/printk.c
> @@ -604,6 +604,27 @@ asmlinkage int printk(const char *fmt, .
>  	return r;
>  }
>  
> +int boot_verbosity;
> +
> +static int __init boot_set_verbosity(char *arg)
> +{
> +	if (!arg)
> +		return -EINVAL;
> +
> +	if (strcmp("spew", arg) == 0)
> +		boot_verbosity = BOOT_SPEW;
> +	else if (strcmp("verbose", arg) == 0)
> +		boot_verbosity = BOOT_VERBOSE;
> +	else {
> +		printk(KERN_WARNING "boot Verbosity level %s not recognised"
> +			" use boot=verbose or boot=spew\n", arg);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +early_param("boot", boot_set_verbosity);
> +
>  /* cpu currently holding logbuf_lock */
>  static volatile unsigned int printk_cpu = UINT_MAX;
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] introduce boot_printk()

[Randy Dunlap]

On Mon, 15 Sep 2008 01:05:16 -0700 Yinghai Lu wrote:

> could be enabled via "boot=verbose" to get more debug info
> 
> will use it to convert some printk(KERN_DEBUG ...)
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> ---
>  include/linux/kernel.h |   19 +++++++++++++++++++
>  kernel/printk.c        |   21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> Index: linux-2.6/include/linux/kernel.h
> ===================================================================
> --- linux-2.6.orig/include/linux/kernel.h
> +++ linux-2.6/include/linux/kernel.h
> @@ -344,6 +344,25 @@ static inline char *pack_hex_byte(char *
>  #endif
>  
>  /*
> + * Debugging macros
> + */
> +#define BOOT_QUIET   0
> +#define BOOT_VERBOSE 1
> +#define BOOT_SPEW    2
> +
> +extern int boot_verbosity;
> +/*
> + * Define the default level of output to be very little
> + * This can be turned up by using boot=verbose for more
> + * information and boot=spew for _lots_ of information.
> + * boot_verbosity is defined in printk.c
> + */
> +#define boot_printk(v, s, a...) do {        \
> +		if ((v) <= boot_verbosity)  \
> +			printk(s, ##a);    \
> +	} while (0)
> +
> +/*
>   *      Display an IP address in readable format.
>   */
>  
> Index: linux-2.6/kernel/printk.c
> ===================================================================
> --- linux-2.6.orig/kernel/printk.c
> +++ linux-2.6/kernel/printk.c
> @@ -604,6 +604,27 @@ asmlinkage int printk(const char *fmt, .
>  	return r;
>  }
>  
> +int boot_verbosity;
> +
> +static int __init boot_set_verbosity(char *arg)
> +{
> +	if (!arg)
> +		return -EINVAL;
> +
> +	if (strcmp("spew", arg) == 0)
> +		boot_verbosity = BOOT_SPEW;
> +	else if (strcmp("verbose", arg) == 0)
> +		boot_verbosity = BOOT_VERBOSE;
> +	else {
> +		printk(KERN_WARNING "boot Verbosity level %s not recognised"

		                                                       ised; "

and please add an entry to Documentation/kernel-parameters.txt for this.


> +			" use boot=verbose or boot=spew\n", arg);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +early_param("boot", boot_set_verbosity);
> +
>  /* cpu currently holding logbuf_lock */
>  static volatile unsigned int printk_cpu = UINT_MAX;


---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Mon, Sep 15, 2008 at 2:58 AM, Pavel Machek <pavel@suse.cz> wrote:
> On Mon 2008-09-15 01:05:16, Yinghai Lu wrote:
>> could be enabled via "boot=verbose" to get more debug info
>>
>> will use it to convert some printk(KERN_DEBUG ...)
>>
>> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>
> boot= is a bit too generic name, I guess...

Ingo suggest to use x86=verbose

but some users are in driver/acpi etc

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
> could be enabled via "boot=verbose" to get more debug info
> 
> will use it to convert some printk(KERN_DEBUG ...)

This seems odd, why not just use KERN_DEBUG?

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Mon, Sep 15, 2008 at 10:15 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>
>> could be enabled via "boot=verbose" to get more debug info
>>
>> will use it to convert some printk(KERN_DEBUG ...)
>
> This seems odd, why not just use KERN_DEBUG?
>

some guys complained that it is even too verbose with KERN_DEBUG

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> some guys complained that it is even too verbose with KERN_DEBUG
> 

I can understand the desire to enable filtering on a 
subsystem-by-subsystem basis, but I think rather than doing a 
subsystem-specific hack we should do something that any subsystem can use.

Perhaps we could extend the current <level> hack to include a subsystem, 
something like:

<7><x86>Your blort driver seems to be befudged, trying blarfing.

... and something like "loglevel=x86:7,acpi:3,..."

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Mon, Sep 15, 2008 at 10:24 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>
>> some guys complained that it is even too verbose with KERN_DEBUG
>>
>
> I can understand the desire to enable filtering on a subsystem-by-subsystem
> basis, but I think rather than doing a subsystem-specific hack we should do
> something that any subsystem can use.
>
> Perhaps we could extend the current <level> hack to include a subsystem,
> something like:
>
> <7><x86>Your blort driver seems to be befudged, trying blarfing.
>
> ... and something like "loglevel=x86:7,acpi:3,..."
>
sounds good.

every subsystem will have
x86_printk
acpi_printk
pci_printk

YH

Re: [PATCH] introduce boot_printk()

[Peter Zijlstra]

On Mon, 2008-09-15 at 10:34 -0700, Yinghai Lu wrote:
> On Mon, Sep 15, 2008 at 10:24 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> > Yinghai Lu wrote:
> >>
> >> some guys complained that it is even too verbose with KERN_DEBUG
> >>
> >
> > I can understand the desire to enable filtering on a subsystem-by-subsystem
> > basis, but I think rather than doing a subsystem-specific hack we should do
> > something that any subsystem can use.
> >
> > Perhaps we could extend the current <level> hack to include a subsystem,
> > something like:
> >
> > <7><x86>Your blort driver seems to be befudged, trying blarfing.
> >
> > ... and something like "loglevel=x86:7,acpi:3,..."
> >
> sounds good.
> 
> every subsystem will have
> x86_printk
> acpi_printk
> pci_printk

doesn't sound like an easily extensible interface, better would be
extending printk with a KERN_subsys tag like proposed.

Perhaps Jason has better ideas - iirc he worked on something similar.

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Peter Zijlstra wrote:
>>
>> every subsystem will have
>> x86_printk
>> acpi_printk
>> pci_printk
> 
> doesn't sound like an easily extensible interface, better would be
> extending printk with a KERN_subsys tag like proposed.
> 

That would be my thinking too.

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 9:06 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Peter Zijlstra wrote:
>>>
>>> every subsystem will have
>>> x86_printk
>>> acpi_printk
>>> pci_printk
>>
>> doesn't sound like an easily extensible interface, better would be
>> extending printk with a KERN_subsys tag like proposed.
>>
>
> That would be my thinking too.

will be printk(KERN_PCI, KERN_DEBUG ".....") ?
and it will be ...KERN_PCI will be int?


YH

Re: [PATCH] introduce boot_printk()

[Jeremy Fitzhardinge]

Yinghai Lu wrote:
> On Tue, Sep 16, 2008 at 9:06 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>   
>> Peter Zijlstra wrote:
>>     
>>>> every subsystem will have
>>>> x86_printk
>>>> acpi_printk
>>>> pci_printk
>>>>         
>>> doesn't sound like an easily extensible interface, better would be
>>> extending printk with a KERN_subsys tag like proposed.
>>>
>>>       
>> That would be my thinking too.
>>     
>
> will be printk(KERN_PCI, KERN_DEBUG ".....") ?
> and it will be ...KERN_PCI will be int?
>   

#define X86_DEBUG    KERN_DEBUG SUBSYS_X86
...
    printk(X86_DEBUG "woozle failed the wibble\n);


    J

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 9:06 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Peter Zijlstra wrote:
>>>
>>> every subsystem will have
>>> x86_printk
>>> acpi_printk
>>> pci_printk
>>
>> doesn't sound like an easily extensible interface, better would be
>> extending printk with a KERN_subsys tag like proposed.
>>
>
> That would be my thinking too.

it seem using pci_printk or acpi_printk.. could be more flexible.

otherwise will need to keep update linux/kernel.h to add so call subsys tags...

use only need to
have DEFINE_LOGLEVEL_SETUP(xxx) in c
and have DECLARE_LOGLEVEL(xxx) in .h
then just use xxx_printk

YH

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 10:45 AM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On Tue, Sep 16, 2008 at 9:06 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>> Peter Zijlstra wrote:
>>>>
>>>> every subsystem will have
>>>> x86_printk
>>>> acpi_printk
>>>> pci_printk
>>>
>>> doesn't sound like an easily extensible interface, better would be
>>> extending printk with a KERN_subsys tag like proposed.
>>>
>>
>> That would be my thinking too.
>
> it seem using pci_printk or acpi_printk.. could be more flexible.
>
> otherwise will need to keep update linux/kernel.h to add so call subsys tags...
>
> use only need to
> have DEFINE_LOGLEVEL_SETUP(xxx) in c
> and have DECLARE_LOGLEVEL(xxx) in .h
> then just use xxx_printk
>
and in real subsys could have as many the xxx_printk that user want...

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> it seem using pci_printk or acpi_printk.. could be more flexible.
> 
> otherwise will need to keep update linux/kernel.h to add so call subsys tags...
> 
> use only need to
> have DEFINE_LOGLEVEL_SETUP(xxx) in c
> and have DECLARE_LOGLEVEL(xxx) in .h
> then just use xxx_printk
> 

Uh?  You can add the subsys tags elsewhere if you prefer too.

However, there are a few important differences to keep in mind.  Tags 
get logged in the dmesg buffer even if not printed to the kernel, 
whereas a jump-based architecture will not just suppress the output, but 
even the formatting of the output.  Either is a good thing for certain 
things, and a bad thing for other things.

I personally think the kind of things discussed belong in the category 
of "always put in the dmesg buffer", and like the idea of 
subsystem-tagging them.

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 10:53 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>
>> it seem using pci_printk or acpi_printk.. could be more flexible.
>>
>> otherwise will need to keep update linux/kernel.h to add so call subsys
>> tags...
>>
>> use only need to
>> have DEFINE_LOGLEVEL_SETUP(xxx) in c
>> and have DECLARE_LOGLEVEL(xxx) in .h
>> then just use xxx_printk
>>
>
> Uh?  You can add the subsys tags elsewhere if you prefer too.
>
> However, there are a few important differences to keep in mind.  Tags get
> logged in the dmesg buffer even if not printed to the kernel, whereas a
> jump-based architecture will not just suppress the output, but even the
> formatting of the output.  Either is a good thing for certain things, and a
> bad thing for other things.
>
> I personally think the kind of things discussed belong in the category of
> "always put in the dmesg buffer", and like the idea of subsystem-tagging
> them.
>

you still can use
pci_printk(v, KERN_PCI KERN_DEBUG "...\n");

if vprintk is expanded to handle KERN_PCI and other tags...
or even you could use
pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> you still can use
> pci_printk(v, KERN_PCI KERN_DEBUG "...\n");
> 
> if vprintk is expanded to handle KERN_PCI and other tags...
> or even you could use
> pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
> 

And the point of this, other than sheer verbosity?

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 11:15 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>
>> you still can use
>> pci_printk(v, KERN_PCI KERN_DEBUG "...\n");
>>
>> if vprintk is expanded to handle KERN_PCI and other tags...
>> or even you could use
>> pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
>>
>
> And the point of this, other than sheer verbosity?

let every file or componet of subsystem could control the print out
via loglevel=pci_any_name:7

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
> On Tue, Sep 16, 2008 at 11:15 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>> Yinghai Lu wrote:
>>> you still can use
>>> pci_printk(v, KERN_PCI KERN_DEBUG "...\n");
>>>
>>> if vprintk is expanded to handle KERN_PCI and other tags...
>>> or even you could use
>>> pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
>>>
>> And the point of this, other than sheer verbosity?
> 
> let every file or componet of subsystem could control the print out
> via loglevel=pci_any_name:7
> 

pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
^^^^          ^^^^^^^^^

Not only do you have duplication, here, but inconsistency...

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 11:31 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>
>> On Tue, Sep 16, 2008 at 11:15 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>
>>> Yinghai Lu wrote:
>>>>
>>>> you still can use
>>>> pci_printk(v, KERN_PCI KERN_DEBUG "...\n");
>>>>
>>>> if vprintk is expanded to handle KERN_PCI and other tags...
>>>> or even you could use
>>>> pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
>>>>
>>> And the point of this, other than sheer verbosity?
>>
>> let every file or componet of subsystem could control the print out
>> via loglevel=pci_any_name:7
>>
>
> pci_printk(v, KERN_ACPI KERN_DEBUG "...\n");
> ^^^^          ^^^^^^^^^
>
> Not only do you have duplication, here, but inconsistency...

how about
pci_printk(v, KERN_DEBUG "...\n");

will put
<pci> <7> ...\n
in dmesg bug,

and let vprintk get rid of <pci> like <7>

is that what you want?

YH

Re: [PATCH] introduce boot_printk()

[H. Peter Anvin]

Yinghai Lu wrote:
>>
>> Not only do you have duplication, here, but inconsistency...
> 
> how about
> pci_printk(v, KERN_DEBUG "...\n");
> 
> will put
> <pci> <7> ...\n
> in dmesg bug,
> 
> and let vprintk get rid of <pci> like <7>
> 
> is that what you want?
> 

First of all, what is the "v" here, and why not just have
printk(KERN_PCI KERN_DEBUG "...\n");

... and we can do #define PCI_DEBUG KERN_PCI KERN_DEBUG even.

We do have a need for special macros when we're doing device-specific 
prefixes, of course.  If that is what your "v" is meant to be, then 
there was an implicit topic shift in the discussion thread.

	-hpa

Re: [PATCH] introduce boot_printk()

[Yinghai Lu]

On Tue, Sep 16, 2008 at 11:53 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> Yinghai Lu wrote:
>>>
>>> Not only do you have duplication, here, but inconsistency...
>>
>> how about
>> pci_printk(v, KERN_DEBUG "...\n");
>>
>> will put
>> <pci> <7> ...\n
>> in dmesg bug,
>>
>> and let vprintk get rid of <pci> like <7>
>>
>> is that what you want?
>>
>
> First of all, what is the "v" here, and why not just have
> printk(KERN_PCI KERN_DEBUG "...\n");
>
> ... and we can do #define PCI_DEBUG KERN_PCI KERN_DEBUG even.
>
> We do have a need for special macros when we're doing device-specific
> prefixes, of course.  If that is what your "v" is meant to be, then there
> was an implicit topic shift in the discussion thread.

will try.

YH