login
Search
KERN_PCI
KERN_ACPI
v4: fix some checkpatch error and warning
v5: add default with DEFINE_LOGLEVE_SETUP_DEF
KERN_APIC
v6: set the default only one time
usage:
in .h to have
#define KERN_PCI "<pci>"
in .c to have
DEFINE_LOGLEVEL_SETUP(pci, KERN_PCI, "pci:");
then could use
printk(KERN_DEBUG KERN_PCI fmt, ...);
and command line
loglevel=3,pci:8
you can add different printk to different files of one subsys if you like
not just one susbsys one tag, and don't need to update kernel.h to add more tags
YH
--
so could make subsys easy to add loglevel and xxx_printk v2: make it more genric, so subsys user only need to two line macro v3: add back nameStr, so could find out iommu: and iommu_gart: and etc v4: use printk intead of pci_printk v5: fix checkpatch error and warning v6: add DEFINE_LOGLEVEL_SETUP_DEF to take default v7: call tag_loglevel_setup only one time, so could take several loglevel like loglevel=4 loglevel=acpi:8 loglevel=pci:8 loglevel=apic:8 is the same as loglevel=4,acpi:8,pci:8,apic:8 usage: in .h to have #define KERN_PCI "<pci>" in .c to have DEFINE_LOGLEVEL_SETUP(pci, KERN_PCI, "pci:"); then could use printk(KERN_DEBUG KERN_PCI fmt, ...); and command line loglevel=pci:8 you can add different printk to different files of one subsys if you like Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com> --- arch/x86/kernel/vmlinux_32.lds.S | 1 arch/x86/kernel/vmlinux_64.lds.S | 2 include/asm-generic/vmlinux.lds.h | 8 +++ include/linux/init.h | 22 ++++++++++ include/linux/kernel.h | 9 ++++ init/main.c | 77 +++++++++++++++++++++++++++++++++++++- kernel/printk.c | 28 +++++++++++-- 7 files changed, 142 insertions(+), 5 deletions(-) Index: linux-2.6/arch/x86/kernel/vmlinux_32.lds.S =================================================================== --- linux-2.6.orig/arch/x86/kernel/vmlinux_32.lds.S +++ linux-2.6/arch/x86/kernel/vmlinux_32.lds.S @@ -145,6 +145,7 @@ SECTIONS *(.x86_cpu_dev.init) __x86_cpu_dev_end = .; } + LOGLEVEL_SETUP_INIT(8) DYN_ARRAY_INIT(8) SECURITY_INIT . = ALIGN(4); Index: linux-2.6/arch/x86/kernel/vmlinux_64.lds.S =================================================================== --- linux-2.6.orig/arch/x86/kernel/vmlinux_64.lds.S +++ linux-2.6/arch/x86/kernel/vmlinux_64.lds.S @@ -174,6 +174,8 @@ SECTIONS } __x86_cpu_dev_end = .; + LOGLEVEL_SETUP_INIT(8) + DYN_ARRAY_INIT(8) ...
use DEFINE_LOGLEVEL_SETUP to set loglevel for pci
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
drivers/pci/pci.c | 2 ++
include/linux/pci.h | 2 ++
2 files changed, 4 insertions(+)
Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -1953,6 +1953,8 @@ static int __devinit pci_setup(char *str
}
early_param("pci", pci_setup);
+DEFINE_LOGLEVEL_SETUP(pci, KERN_PCI, "pci:");
+
device_initcall_sync(pci_init);
EXPORT_SYMBOL(pci_reenable_device);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -55,6 +55,8 @@
/* Include the ID list */
#include <linux/pci_ids.h>
+#define KERN_PCI "<pci>"
+
/* pci_slot represents a physical slot */
struct pci_slot {
struct pci_bus *bus; /* The bus this slot is on */
--
v2: use printk(KERN_DEBUG KERN_PCI ...
v3: fix checkpatch error and warning
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
drivers/pci/probe.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -304,7 +304,8 @@ static int __pci_read_base(struct pci_de
} else {
res->start = l64;
res->end = l64 + sz64;
- printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
+ printk(KERN_DEBUG KERN_PCI
+ "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
pci_name(dev), pos, res->start, res->end);
}
} else {
@@ -315,9 +316,10 @@ static int __pci_read_base(struct pci_de
res->start = l;
res->end = l + sz;
- printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
- pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
- res->start, res->end);
+ printk(KERN_DEBUG KERN_PCI "PCI: %s reg %x %s: [%llx, %llx]\n",
+ pci_name(dev), pos,
+ (res->flags & IORESOURCE_IO) ? "io port" : "32bit mmio",
+ res->start, res->end);
}
out:
@@ -388,8 +390,9 @@ void __devinit pci_read_bridge_bases(str
res->start = base;
if (!res->end)
res->end = limit + 0xfff;
- printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
- pci_name(dev), res->start, res->end);
+ printk(KERN_DEBUG KERN_PCI
+ "PCI: bridge %s io port: [%llx, %llx]\n",
+ pci_name(dev), res->start, res->end);
}
res = child->resource[1];
@@ -401,8 +404,9 @@ void __devinit pci_read_bridge_bases(str
res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
res->start = base;
res->end = limit + 0xfffff;
- printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
- pci_name(dev), res->start, res->end);
+ printk(KERN_DEBUG KERN_PCI
+ "PCI: bridge %s 32bit mmio: [%llx, ...i think we should aim to make the common usage of this facility as easy and short as possible. That means that i think the right line is: + printk(KERN_PCI + "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n", as that is the shortest form. KERN_PCI should imply whatever common-sense default. It should still make sense to specify KERN_FOO KERN_PCI, to override that default. Ingo --
vprintk will add default before that like like print(KERN_INFO KERN_PCI ....) YH --
What about a pci_dbg() macro similiar to dev_dbg()? Same could be used for ACPI, acpi_dbg(). -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com --
according to different users ... YH --
I really don't understand the point of this series. I think the user experience is too confusing. But if you're going to change printks like the one above, please at least make it use dev_printk() like most of the rest of the PCI core. --
use DEFINE_LOGLEVEL_SETUP to set loglevel for acpi
v2: use <acpi, "acpi:"> instead
v3: use KERN_ACPI
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
drivers/acpi/osl.c | 2 ++
include/linux/acpi.h | 1 +
2 files changed, 3 insertions(+)
Index: linux-2.6/drivers/acpi/osl.c
===================================================================
--- linux-2.6.orig/drivers/acpi/osl.c
+++ linux-2.6/drivers/acpi/osl.c
@@ -75,6 +75,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
extern char line_buf[80];
#endif /*ENABLE_DEBUGGER */
+DEFINE_LOGLEVEL_SETUP(acpi, KERN_ACPI, "acpi:");
+
static unsigned int acpi_irq_irq;
static acpi_osd_handler acpi_irq_handler;
static void *acpi_irq_context;
Index: linux-2.6/include/linux/acpi.h
===================================================================
--- linux-2.6.orig/include/linux/acpi.h
+++ linux-2.6/include/linux/acpi.h
@@ -43,6 +43,7 @@
#include <asm/acpi.h>
#include <linux/dmi.h>
+#define KERN_ACPI "<acpi>"
enum acpi_irq_model_id {
ACPI_IRQ_MODEL_PIC = 0,
--
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
drivers/acpi/numa.c | 9 +++++++++
1 file changed, 9 insertions(+)
Index: linux-2.6/drivers/acpi/numa.c
===================================================================
--- linux-2.6.orig/drivers/acpi/numa.c
+++ linux-2.6/drivers/acpi/numa.c
@@ -150,6 +150,15 @@ static __init int slit_valid(struct acpi
{
int i, j;
int d = slit->locality_count;
+ printk(KERN_DEBUG KERN_ACPI "ACPI: SLIT: nodes = %d\n", d);
+ for (i = 0; i < d; i++) {
+ printk(KERN_DEBUG KERN_ACPI " ");
+ for (j = 0; j < d; j++) {
+ u8 val = slit->entry[d*i + j];
+ printk(KERN_CONT KERN_ACPI " %d", val);
+ }
+ printk(KERN_CONT KERN_ACPI "\n");
+ }
for (i = 0; i < d; i++) {
for (j = 0; j < d; j++) {
u8 val = slit->entry[d*i + j];
--
It will produce something like this: <7><acpi>ACPI: SLIT: nodes = %d\n <7><acpi> <acpi> %d<acpi> %d<acpi> %d<acpi> %d<acpi>\n bit wrong... ;) Marcin --
On Wed, Sep 17, 2008 at 11:19 AM, Marcin Slusarz did you check v6...? YH --
No, I didn't test it. Sorry for the noise. Marcin --
and kill apic_printk
using loglevel=apic:8 intead
Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
---
Documentation/kernel-parameters.txt | 6
arch/x86/kernel/apic.c | 108 +++++-------
arch/x86/kernel/io_apic.c | 235 +++++++++++++---------------
arch/x86/kernel/mpparse.c | 10 -
arch/x86/kernel/smpboot.c | 11 -
include/asm-x86/apic.h | 20 --
include/asm-x86/es7000/wakecpu.h | 6
include/asm-x86/mach-default/mach_wakecpu.h | 6
8 files changed, 178 insertions(+), 224 deletions(-)
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -306,12 +306,6 @@ and is between 256 and 4096 characters.
not play well with APC CPU idle - disable it if you have
APC and your system crashes randomly.
- apic= [APIC,i386] Advanced Programmable Interrupt Controller
- Change the output verbosity whilst booting
- Format: { quiet (default) | verbose | debug }
- Change the amount of debugging information output
- when initialising the APIC and IO-APIC components.
-
apm= [APM] Advanced Power Management
See header of arch/x86/kernel/apm_32.c.
Index: linux-2.6/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic.c
+++ linux-2.6/arch/x86/kernel/apic.c
@@ -58,6 +58,8 @@
# error SPURIOUS_APIC_VECTOR definition error
#endif
+DEFINE_LOGLEVEL_SETUP_DEF(apic, KERN_APIC, "apic:", 6);
+
#ifdef CONFIG_X86_32
/*
* Knob to control our willingness to enable the local APIC.
@@ -123,8 +125,6 @@ char system_vectors[NR_VECTORS] = { [0 .
/*
* Debug level, exported for io_apic.c
*/
-unsigned int apic_verbosity;
-
int pic_mode;
/* Have we found an MP table */
@@ -549,7 ...here too the question arises: what should the semantics of the 'mixing' of such subsystem printk tags with the classic priority tags be. I think in this particular case we dont want the KERN_APIC tag, as that would prevent this failure message to be printed by default. I.e. this line to make sure this warning always shows up in the logs. Agreed? Ingo --
with DEFINE_LOGLEVEL_SETUP_DEF(apic, KERN_APIC, "apic:", 6); the KERN_WARNING <5> will be showing up that warning, even without loglevl=apic:8 or change that to DEFINE_LOGLEVEL_SETUP_DEF(apic, KERN_APIC, "apic:", 7); ? YH --
ok, but if that printk will show up no matter what, why is there any need to add an extra tag? my main worry is the duplication of the tag. People have a hard enough time adding a _single_ printk tag - adding multiple ones is clearly too much. Ingo --
Are you suggesting that each subsystem define its own default log level?
That sounds truly horrible. It would mean that when reading code or
adding new messages you'd always have to check what the default loglevel
is for a particular subsystem.
I really like the idea of being able to easily increase the verbosity for
a particular subsystem, but please make sure that by default all messages
with the same level are treated the same, irrespective of which subsystem
they belong to.
So IMO
printk(KERN_WARNING "APIC calibration not consistent "
and
printk(KERN_WARNING KERN_APIC "APIC calibration not consistent "
should do exactly the same thing if no 'loglevel=apic:X' is used and that
should be guaranteed for all subsystems.
Cheers,
FJP
--
could be done. SPEW: 8 ???:9 current msg_loglevel is limted to one digital YH --
This all looks dreadfully similar to Jason Baron's "dynamic debug v2". Similar enough that we couldn't justify merging both. --
(Jason Cc:-ed) agreed. Here's the summary as i see it: Jason's "dynamic debug" mainly relies on automatically creating a section of tags associated with each pr_debug() callsite. The unit of filtering is the .c file - which lends itself well to things like individual drivers but is not quite good for more generic entities like full subsystems. I think a better model is what hpa suggested: add string tags at the call site and make it all a sub-case of printk, via ASCII space tags. OTOH, a pr_debug() variant could be offered that just inserts a __FILE__ tag into the output - this would enable most of the "dynamic debug" functionality that Jason's patch series implements IMO. There's also a wider robustness argument why this is better: we want to keep printk simple by default, hence doing these things via the ASCII space is good design. "dynamic debug" does it all programmatically via extra sections, etc., which is eventually a source of bugs and unrobustness. Ingo --
hi,
indeed both proposals seem to be addressing a similar core issue - being
able to dynamically (as opposed to re-compiling) change the amount of
verbosity that the kernel spews. I would liken it to adding a -verbose
flag to the kernel.
Beyond that the 2 approaches differ in a number of ways as I see:
1)
Filtering Unit. 'Boot printk', or Yinghai's approach explicitly defines
filtering units via tags. 'dynamic debug' is implicitly based
around "KBUILD_MODNAME", not .c files, which seems to be a good unit for
most parts of the kernel. For full subsystems, it is not appropriate.
There, i have introduced the conecpt, of 'logical modname', which is very
roughly similar to the 'boot printk' tag concept. Basically, subsystems
could be grouped together using this 'logical modname'.
2)
kernel impact. Part of the design of 'dynamic debug' was to be very low
impact. in fact, when its disabled, we basically jump around the printk
with a test and a jump. So, there is only a 2 instruction overhead per
call-site. I believe (and please correct me if i'm wrong) that 'boot
printk' is filtering after printk has been called...So you have at least
the overhead of a fucntion call plus the overhead of the printk()
function.
3)
usefullness beyond just doing 'printk'. The basic building block of
dynamic debug is simply: 'dynamic_debug_enabled()'. So blocks of code are built
up as:
if (dynamic_debug_enabled()) {
do debugging work
}
printk is then simply:
if (dynamic_debug_enabled()) {
printk();
}
or:
define pr_debug()
(dynamic_debug_enabled()) {
printk();
}
The intent here was to dynamically be able to turn 'printk' or other
debugging code dynamically off and on.
4)
ability to compile out the code. 'dynamic debug' has 3 modes of
compilation. If 'DEBUG' is set then then dynamic_debug_enabled() simply
is defined to 1. If 'DYNAMIC_DEBUG' is set dynamic_debug_enabled()
becomes the test and jump i desscribe in ...8) The ability to accessed filtered messages a posteori (via dmesg), which is something we currently take for granted. This *is* the fundamental difference between what Yinghai has now and both your stuff and Yinghai's original proposal. Not producing the additional messages at all is inherently cheaper, sometimes *much* cheaper, but it obviously means the information is not accessible at all. At the moment, I would argue that the fact that dmesg is, in effect, more verbose than the kernel itself is a good thing; it makes dmesg dumps more useful. -hpa --
which is what we really want. If a bootup fails, the user has to repeat the bootup at least once with a verbosity level increased and with (hopefully) some sort of log capture facility attached. So the point would be, if the user specified loglevel=all, we would get yes - but log messages not showing up on the physical console is already a task that is solved via regular loglevels. printks are being removed because even printing to the dmesg can be expensive (vsprintf is not the cheapest of functions, and most of dmesg shows up in the syslog and people want a clear, default, not too verbose output for that). so the main feature of these patches is that we can cut out too verbose information by default, and we can reactivate it with a single central switch (and a repeated bootup - which has to be done anyway). Everything else is already solved via either traditional loglevels or pr_debug() based, build-time facilities. Ingo --
That is true if the bootup fails, but it's fairly common that we get the machine up (at least to the point when we can dmesg), but the user wants to report a problem. For that case, it's very nice if the dmesg log contains as much information as possible. So I don't think it's clear that pre-filtering is what we want, at all. -hpa --
hi, if we take this argument to its extreme, then we end up spending all of our time verifying that the kernel is working correctly and no time actually doing work. I think 'printk_ratelimit' captures this. Thus, the line has to be drawn somewhere. If you want the messages in 'dmesg' use, printk(KERN_DEBUG), and 'grep'. For the rest, I propose pre-filtering, which is what 'dynamic debug' uses. thanks, -Jason --
Taking any argument to its extreme and you come up with something ridiculous. One could equally argue that if you have so many debugging messages that you have to prefilter for performance, you're so bloating your kernel that you need to stop. I find it highly questionable that it makes sense to put even skipped messages into hot paths in the production kernel. Skipped prints are NOT free, even if they are lot cheaper than actually rendering the string. -hpa --
in my testing there was no significant difference between pre-filtering vs. not built in. However, there was a measureable affect of having them If you feel that way, you simply don't have to turn the single CONFIG_ on. Futher, the patchset allows fine-grained control over the modules that you would want to enable/disable. thanks, -Jason --
<somewhat irrelevant rant> Okay, I'm so bloody sick of hearing people justifying bloat by saying "no measurable difference." That only means that you don't have GROSS bloat. Unless you have real statistics to the contrary, most people's performance testing is at the very best accurate to 5-10%. This is hardly "no bloat". </somewhat irrelevant rant> -hpa --
about performance difference:
1. will have printk(KERN_level KERN_tag "...\n");
2. will have tag_printk like
#define dev_printk(level, dev, format, arg...) \
printk(level KERN_DEV "%s %s: " format , dev_driver_string(dev) , \
dev_name(dev) , ## arg)
3. still can use MACRO for compiling time
#ifdef DEBUG
#define dev_dbg(dev, format, arg...) \
dev_printk(KERN_DEBUG , dev , format , ## arg)
#else
#define dev_dbg(dev, format, arg...) \
({ if (0) dev_printk(KERN_DEBUG, dev, format, ##arg); 0; })
#endif
4. with /proc/sys/kernel/printk_tag/dev could modify run-time level.
5. for big chunk dump or spew, caller could use
level = get_tag_level(KERN_DEV, &len) and compare level to
KERN_SPEW_LEVEL or etc
to decide if need to dump it.
6. other overhead to put tag like <dev> should be ok just like
loglevel aready in the buffer
current only have one struct {
char *tag;
char *name; /* with extra : */
int level;
}
the level is some kind of console_loglevel for that tag.
if want to go further, let every dev_printk check that. then will not
have them in log buffer
YH
--
