| Message ID | 1555733130-19804-1-git-send-email-mohankumar718@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/3] PCI: Cleanup printk logging | expand |
Hi Mohan, On Sat, Apr 20, 2019 at 07:05:30AM +0300, Mohan Kumar wrote: > Define a pr_fmt() macro that convert all of the explicit printk() calls > into corresponding pr_*(). I dropped the pr_fmt() parts of this because there's really no benefit: we move the prefix from the actual printk() to the pr_fmt definition, which is a little bit harder to understand for the reader, and it makes the string harder to find, e.g., if you see "PCI: can't add host bridge window" in the dmesg log and search for it in the kernel source, you'll no longer find it. I think pr_fmt does make sense if the prefix is used many times, but for these cases, where it's only used once or twice, it doesn't seem worthwhile. > Signed-off-by: Mohan Kumar <mohankumar718@gmail.com> > --- > drivers/pci/bus.c | 5 ++++- > drivers/pci/pci-stub.c | 11 +++++------ > drivers/pci/pci-sysfs.c | 3 ++- > drivers/pci/pci.c | 5 +++-- > drivers/pci/quirks.c | 9 +++++---- > drivers/pci/setup-bus.c | 5 +++-- > drivers/pci/slot.c | 4 +++- > 7 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 5cb40b2..a742ef5 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -6,6 +6,9 @@ > * David Miller (davem@redhat.com) > * Ivan Kokshaysky (ink@jurassic.park.msu.ru) > */ > + > +#define pr_fmt(fmt) "PCI: " fmt > + > #include <linux/module.h> > #include <linux/kernel.h> > #include <linux/pci.h> > @@ -23,7 +26,7 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res, > > entry = resource_list_create_entry(res, 0); > if (!entry) { > - printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res); > + pr_err("can't add host bridge window %pR\n", res); This is an example of doing two changes at once: 1) Converting "printk(KERN_ERR)" to "pr_err()" 2) Factoring out the "PCI: " prefix with pr_fmt() The first change is worthwhile and I pulled that out and squashed it into your first patch. This patch (if we were going to do it) should contain *only* the pr_fmt() factoring. So the first patch would look like this: - printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res); + pr_err("PCI: can't add host bridge window %pR\n", res); and the second patch would look like this: +#define pr_fmt(fmt) "PCI: " fmt - pr_err("PCI: can't add host bridge window %pR\n", res); + pr_err("can't add host bridge window %pR\n", res); > @@ -159,8 +161,7 @@ static int __init pci_apply_final_quirks(void) > u8 tmp; > > if (pci_cache_line_size) > - printk(KERN_DEBUG "PCI: CLS %u bytes\n", > - pci_cache_line_size << 2); > + pr_info("CLS %u bytes\n", pci_cache_line_size << 2); > > pci_apply_fixup_final_quirks = true; > for_each_pci_dev(dev) { > @@ -177,7 +178,7 @@ static int __init pci_apply_final_quirks(void) > if (!tmp || cls == tmp) > continue; > > - printk(KERN_DEBUG "PCI: CLS mismatch (%u != %u), using %u bytes\n", > + pr_info("CLS mismatch (%u != %u), using %u bytes\n", > cls << 2, tmp << 2, > pci_dfl_cache_line_size << 2); Here's a case where we should actually be using pci_printk() since this message is related to a specific device. I added a new patch before your first patch to do this: - printk(KERN_DEBUG "PCI: CLS mismatch ..." + pci_printk(KERN_DEBUG, dev, "CLS mismatch ..." There's a similar case in pci_create_legacy_files(), so the new patch converts both of those. Bjorn
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 5cb40b2..a742ef5 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -6,6 +6,9 @@ * David Miller (davem@redhat.com) * Ivan Kokshaysky (ink@jurassic.park.msu.ru) */ + +#define pr_fmt(fmt) "PCI: " fmt + #include <linux/module.h> #include <linux/kernel.h> #include <linux/pci.h> @@ -23,7 +26,7 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res, entry = resource_list_create_entry(res, 0); if (!entry) { - printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res); + pr_err("can't add host bridge window %pR\n", res); return; } diff --git a/drivers/pci/pci-stub.c b/drivers/pci/pci-stub.c index 66f8a59..f946bf9 100644 --- a/drivers/pci/pci-stub.c +++ b/drivers/pci/pci-stub.c @@ -16,6 +16,8 @@ * .../0000:00:19.0/driver -> ../../../bus/pci/drivers/pci-stub */ +#define pr_fmt(fmt) "pci-stub: " fmt + #include <linux/module.h> #include <linux/pci.h> @@ -66,20 +68,17 @@ static int __init pci_stub_init(void) &class, &class_mask); if (fields < 2) { - printk(KERN_WARNING - "pci-stub: invalid id string \"%s\"\n", id); + pr_warn("invalid id string \"%s\"\n", id); continue; } - printk(KERN_INFO - "pci-stub: add %04X:%04X sub=%04X:%04X cls=%08X/%08X\n", + pr_info("add %04X:%04X sub=%04X:%04X cls=%08X/%08X\n", vendor, device, subvendor, subdevice, class, class_mask); rc = pci_add_dynid(&stub_driver, vendor, device, subvendor, subdevice, class, class_mask, 0); if (rc) - printk(KERN_WARNING - "pci-stub: failed to add dynamic id (%d)\n", rc); + pr_warn("failed to add dynamic id (%d)\n", rc); } return 0; diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 25794c2..455bf29 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -12,6 +12,7 @@ * Modeled after usb's driverfs.c */ +#define pr_fmt(fmt) "pci: warning: " fmt #include <linux/kernel.h> #include <linux/sched.h> @@ -1111,7 +1112,7 @@ void pci_create_legacy_files(struct pci_bus *b) kfree(b->legacy_io); b->legacy_io = NULL; kzalloc_err: - printk(KERN_WARNING "pci: warning: could not create legacy I/O port and ISA memory resources to sysfs\n"); + pr_warn("could not create legacy I/O port and ISA memory resources to sysfs\n"); return; } diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index f5ff01d..a24a8bc 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -8,6 +8,8 @@ * Copyright 1997 -- 2000 Martin Mares <mj@ucw.cz> */ +#define pr_fmt(fmt) "PCI: " fmt + #include <linux/acpi.h> #include <linux/kernel.h> #include <linux/delay.h> @@ -6276,8 +6278,7 @@ static int __init pci_setup(char *str) } else if (!strncmp(str, "disable_acs_redir=", 18)) { disable_acs_redir_param = str + 18; } else { - printk(KERN_ERR "PCI: Unknown option `%s'\n", - str); + pr_err("Unknown option `%s'\n", str); } } str = k; diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 06af0c3..fb802d1 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -12,6 +12,8 @@ * file, where their drivers can use them. */ +#define pr_fmt(fmt) "PCI: " fmt + #include <linux/types.h> #include <linux/kernel.h> #include <linux/export.h> @@ -159,8 +161,7 @@ static int __init pci_apply_final_quirks(void) u8 tmp; if (pci_cache_line_size) - printk(KERN_DEBUG "PCI: CLS %u bytes\n", - pci_cache_line_size << 2); + pr_info("CLS %u bytes\n", pci_cache_line_size << 2); pci_apply_fixup_final_quirks = true; for_each_pci_dev(dev) { @@ -177,7 +178,7 @@ static int __init pci_apply_final_quirks(void) if (!tmp || cls == tmp) continue; - printk(KERN_DEBUG "PCI: CLS mismatch (%u != %u), using %u bytes\n", + pr_info("CLS mismatch (%u != %u), using %u bytes\n", cls << 2, tmp << 2, pci_dfl_cache_line_size << 2); pci_cache_line_size = pci_dfl_cache_line_size; @@ -185,7 +186,7 @@ static int __init pci_apply_final_quirks(void) } if (!pci_cache_line_size) { - printk(KERN_DEBUG "PCI: CLS %u bytes, default %u\n", + pr_info("CLS %u bytes, default %u\n", cls << 2, pci_dfl_cache_line_size << 2); pci_cache_line_size = cls ? cls : pci_dfl_cache_line_size; } diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index ec44a0f..418023a 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -14,6 +14,8 @@ * tighter packing. Prefetchable range support. */ +#define pr_fmt(fmt) "PCI: " fmt + #include <linux/init.h> #include <linux/kernel.h> #include <linux/module.h> @@ -2016,8 +2018,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) goto enable_all; } - printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n", - tried_times + 1); + pr_info("No. %d try to assign unassigned res\n", tried_times + 1); /* * Try to release leaf bridge's resources that doesn't fit resource of diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index c46d5e1..9f0463e 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -5,6 +5,8 @@ * Alex Chiang <achiang@hp.com> */ +#define pr_fmt(fmt) "PCI: " fmt + #include <linux/kobject.h> #include <linux/slab.h> #include <linux/module.h> @@ -403,7 +405,7 @@ static int pci_slot_init(void) pci_slots_kset = kset_create_and_add("slots", NULL, &pci_bus_kset->kobj); if (!pci_slots_kset) { - printk(KERN_ERR "PCI: Slot initialization failure\n"); + pr_err("Slot initialization failure\n"); return -ENOMEM; } return 0;
Define a pr_fmt() macro that convert all of the explicit printk() calls into corresponding pr_*(). Signed-off-by: Mohan Kumar <mohankumar718@gmail.com> --- drivers/pci/bus.c | 5 ++++- drivers/pci/pci-stub.c | 11 +++++------ drivers/pci/pci-sysfs.c | 3 ++- drivers/pci/pci.c | 5 +++-- drivers/pci/quirks.c | 9 +++++---- drivers/pci/setup-bus.c | 5 +++-- drivers/pci/slot.c | 4 +++- 7 files changed, 25 insertions(+), 17 deletions(-)