* [PATCH v8 1/3] aerdrv: Trace Event for AER
@ 2013-01-02 23:27 Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 2/3] aerdrv: Enhanced AER logging Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 3/3] aerdrv: Cleanup log output for AER Lance Ortiz
0 siblings, 2 replies; 12+ messages in thread
From: Lance Ortiz @ 2013-01-02 23:27 UTC (permalink / raw)
To: bhelgaas, lance_ortiz, jiang.liu, tony.luck, bp, rostedt, mchehab,
linux-acpi, linux-pci, linux-kernel
This header file will define a new trace event that will be triggered when
a AER event occurs. The following data will be provided to the trace
event.
char * dev_name - The name of the slot where the device resides
([domain:]bus:device.function).
u32 status - Either the correctable or uncorrectable register
indicating what error or errors have been see.
u8 severity - error severity 0:NONFATAL 1:FATAL 2:CORRECTED
The trace event will also provide a trace string that may look like:
"0000:05:00.0 PCIe Bus Error:severity=Uncorrected (Non-Fatal), Poisoned
TLP"
v1-v2 Move header from include/ras/aer_event.h to
include/trace/events/ras.h
v3-v4 Cleaned up comments and commit header
v4-v5 More cleanup remove () from if statement in print.
Renamed string define to be more specific.
v5-v6 change TRACE_SYSTEM define to be ras and not aer.
Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
include/trace/events/ras.h | 77 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 77 insertions(+), 0 deletions(-)
create mode 100644 include/trace/events/ras.h
diff --git a/include/trace/events/ras.h b/include/trace/events/ras.h
new file mode 100644
index 0000000..88b8783
--- /dev/null
+++ b/include/trace/events/ras.h
@@ -0,0 +1,77 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ras
+
+#if !defined(_TRACE_AER_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_AER_H
+
+#include <linux/tracepoint.h>
+#include <linux/edac.h>
+
+
+/*
+ * PCIe AER Trace event
+ *
+ * These events are generated when hardware detects a corrected or
+ * uncorrected event on a PCIe device. The event report has
+ * the following structure:
+ *
+ * char * dev_name - The name of the slot where the device resides
+ * ([domain:]bus:device.function).
+ * u32 status - Either the correctable or uncorrectable register
+ * indicating what error or errors have been seen
+ * u8 severity - error severity 0:NONFATAL 1:FATAL 2:CORRECTED
+ */
+
+#define aer_correctable_errors \
+ {BIT(0), "Receiver Error"}, \
+ {BIT(6), "Bad TLP"}, \
+ {BIT(7), "Bad DLLP"}, \
+ {BIT(8), "RELAY_NUM Rollover"}, \
+ {BIT(12), "Replay Timer Timeout"}, \
+ {BIT(13), "Advisory Non-Fatal"}
+
+#define aer_uncorrectable_errors \
+ {BIT(4), "Data Link Protocol"}, \
+ {BIT(12), "Poisoned TLP"}, \
+ {BIT(13), "Flow Control Protocol"}, \
+ {BIT(14), "Completion Timeout"}, \
+ {BIT(15), "Completer Abort"}, \
+ {BIT(16), "Unexpected Completion"}, \
+ {BIT(17), "Receiver Overflow"}, \
+ {BIT(18), "Malformed TLP"}, \
+ {BIT(19), "ECRC"}, \
+ {BIT(20), "Unsupported Request"}
+
+TRACE_EVENT(aer_event,
+ TP_PROTO(const char *dev_name,
+ const u32 status,
+ const u8 severity),
+
+ TP_ARGS(dev_name, status, severity),
+
+ TP_STRUCT__entry(
+ __string( dev_name, dev_name )
+ __field( u32, status )
+ __field( u8, severity )
+ ),
+
+ TP_fast_assign(
+ __assign_str(dev_name, dev_name);
+ __entry->status = status;
+ __entry->severity = severity;
+ ),
+
+ TP_printk("%s PCIe Bus Error: severity=%s, %s\n",
+ __get_str(dev_name),
+ __entry->severity == HW_EVENT_ERR_CORRECTED ? "Corrected" :
+ __entry->severity == HW_EVENT_ERR_FATAL ?
+ "Fatal" : "Uncorrected",
+ __entry->severity == HW_EVENT_ERR_CORRECTED ?
+ __print_flags(__entry->status, "|", aer_correctable_errors) :
+ __print_flags(__entry->status, "|", aer_uncorrectable_errors))
+);
+
+#endif /* _TRACE_AER_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v8 2/3] aerdrv: Enhanced AER logging
2013-01-02 23:27 [PATCH v8 1/3] aerdrv: Trace Event for AER Lance Ortiz
@ 2013-01-02 23:27 ` Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 3/3] aerdrv: Cleanup log output for AER Lance Ortiz
1 sibling, 0 replies; 12+ messages in thread
From: Lance Ortiz @ 2013-01-02 23:27 UTC (permalink / raw)
To: bhelgaas, lance_ortiz, jiang.liu, tony.luck, bp, rostedt, mchehab,
linux-acpi, linux-pci, linux-kernel
This patch will provide a more reliable and easy way for user-space
applications to have access to AER logs rather than reading them from the
message buffer. It also provides a way to notify user-space when an AER
event occurs.
The aer driver is updated to generate a trace event of function 'aer_event'
when a PCIe error is reported over the AER interface. The trace event was
added to both the interrupt based aer path and the firmware first path.
v1-v2 fix compile errors in ifdefs.
v2-v3 Update to new location of trace header. Update print to remove
warning.
v3-v4 Reworked logic when getting ready to call cper_print_aer
v6-v7 Change print from pr_info to pr_err if !dev
v7-v8 Add pfx argument back into call to cper_print_aer()
Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/acpi/apei/cper.c | 19 ++++++++++++++++---
drivers/pci/pcie/aer/aerdrv_errprint.c | 9 ++++++++-
include/linux/aer.h | 4 ++--
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/cper.c b/drivers/acpi/apei/cper.c
index e6defd8..1e5d8a4 100644
--- a/drivers/acpi/apei/cper.c
+++ b/drivers/acpi/apei/cper.c
@@ -29,6 +29,7 @@
#include <linux/time.h>
#include <linux/cper.h>
#include <linux/acpi.h>
+#include <linux/pci.h>
#include <linux/aer.h>
/*
@@ -249,6 +250,10 @@ static const char *cper_pcie_port_type_strs[] = {
static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
const struct acpi_hest_generic_data *gdata)
{
+#ifdef CONFIG_ACPI_APEI_PCIEAER
+ struct pci_dev *dev;
+#endif
+
if (pcie->validation_bits & CPER_PCIE_VALID_PORT_TYPE)
printk("%s""port_type: %d, %s\n", pfx, pcie->port_type,
pcie->port_type < ARRAY_SIZE(cper_pcie_port_type_strs) ?
@@ -281,10 +286,18 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
"%s""bridge: secondary_status: 0x%04x, control: 0x%04x\n",
pfx, pcie->bridge.secondary_status, pcie->bridge.control);
#ifdef CONFIG_ACPI_APEI_PCIEAER
- if (pcie->validation_bits & CPER_PCIE_VALID_AER_INFO) {
- struct aer_capability_regs *aer_regs = (void *)pcie->aer_info;
- cper_print_aer(pfx, gdata->error_severity, aer_regs);
+ dev = pci_get_domain_bus_and_slot(pcie->device_id.segment,
+ pcie->device_id.bus, pcie->device_id.function);
+ if (!dev) {
+ pr_err("PCI AER Cannot get PCI device %04x:%02x:%02x.%d\n",
+ pcie->device_id.segment, pcie->device_id.bus,
+ pcie->device_id.slot, pcie->device_id.function);
+ return;
}
+ if (pcie->validation_bits & CPER_PCIE_VALID_AER_INFO)
+ cper_print_aer(pfx, dev, gdata->error_severity,
+ (struct aer_capability_regs *) pcie->aer_info);
+ pci_dev_put(dev);
#endif
}
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 3ea5173..d3e5fc5 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -23,6 +23,9 @@
#include "aerdrv.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/ras.h>
+
#define AER_AGENT_RECEIVER 0
#define AER_AGENT_REQUESTER 1
#define AER_AGENT_COMPLETER 2
@@ -194,6 +197,8 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
if (info->id && info->error_dev_num > 1 && info->id == id)
printk("%s"" Error of this Agent(%04x) is reported first\n",
prefix, id);
+ trace_aer_event(dev_name(&dev->dev), (info->status & ~info->mask),
+ info->severity);
}
void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info)
@@ -217,7 +222,7 @@ int cper_severity_to_aer(int cper_severity)
}
EXPORT_SYMBOL_GPL(cper_severity_to_aer);
-void cper_print_aer(const char *prefix, int cper_severity,
+void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
struct aer_capability_regs *aer)
{
int aer_severity, layer, agent, status_strs_size, tlp_header_valid = 0;
@@ -259,5 +264,7 @@ void cper_print_aer(const char *prefix, int cper_severity,
*(tlp + 8), *(tlp + 15), *(tlp + 14),
*(tlp + 13), *(tlp + 12));
}
+ trace_aer_event(dev_name(&dev->dev), (status & ~mask),
+ aer_severity);
}
#endif
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 544abdb..ec10e1b 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -49,8 +49,8 @@ static inline int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev)
}
#endif
-extern void cper_print_aer(const char *prefix, int cper_severity,
- struct aer_capability_regs *aer);
+extern void cper_print_aer(const char *prefix, struct pci_dev *dev,
+ int cper_severity, struct aer_capability_regs *aer);
extern int cper_severity_to_aer(int cper_severity);
extern void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
int severity);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v8 3/3] aerdrv: Cleanup log output for AER
2013-01-02 23:27 ` [PATCH v8 3/3] aerdrv: Cleanup log output for AER Lance Ortiz
@ 2013-01-02 23:27 ` Joe Perches
2013-01-02 23:41 ` Tony Luck
2013-01-03 0:06 ` [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL> Joe Perches
1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-01-02 23:27 UTC (permalink / raw)
To: Lance Ortiz
Cc: bhelgaas, lance_ortiz, jiang.liu, tony.luck, bp, rostedt, mchehab,
linux-acpi, linux-pci, linux-kernel
On Wed, 2013-01-02 at 16:27 -0700, Lance Ortiz wrote:
> These changes make cper_print_aer more consistent with aer_print_error
> and clean things up by elimiating the use of the prefix variable and
> replacing it with dev_printk.
[]
> v7-v8 Updated to use dev_printk instated of prefix. Changed
> log levels to KERN_ERR as per Mauro's suggestion
Just use dev_err( instead of dev_printk(KERN_ERR,
It's a function and it makes the object code smaller.
> diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
[]
> @@ -144,26 +143,24 @@ static void __aer_print_error(const char *prefix,
> aer_uncorrectable_error_string[i] : NULL;
>
> if (errmsg)
> - printk("%s"" [%2d] %-22s%s\n", prefix, i, errmsg,
> + dev_printk(KERN_ERR, &dev->dev,
> + " [%2d] %-22s%s\n", i, errmsg,
> info->first_error == i ? " (First)" : "");
> else
> - printk("%s"" [%2d] Unknown Error Bit%s\n", prefix, i,
> - info->first_error == i ? " (First)" : "");
> + dev_printk(KERN_ERR, &dev->dev,
> + " [%2d] Unknown Error Bit%s\n",
> + i, info->first_error == i ? " (First)" : "");
> }
[]
> if (info->status == 0) {
> - printk("%s""PCIe Bus Error: severity=%s, type=Unaccessible, "
> - "id=%04x(Unregistered Agent ID)\n", prefix,
> + dev_printk(KERN_ERR, &dev->dev,
> + "PCIe Bus Error: severity=%s, type=Unaccessible, "
> + "id=%04x(Unregistered Agent ID)\n",
[]
> @@ -171,22 +168,24 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
[]
> + dev_printk(KERN_ERR, &dev->dev,
> + "PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
> + aer_error_severity_string[info->severity],
[]
> + dev_printk(KERN_ERR, &dev->dev,
> + " device [%04x:%04x] error status/mask=%08x/%08x\n",
> + dev->vendor, dev->device,
> info->status, info->mask);
[]
> + dev_printk(KERN_ERR, &dev->dev, " TLP Header:"
> " %02x%02x%02x%02x %02x%02x%02x%02x"
> " %02x%02x%02x%02x %02x%02x%02x%02x\n",
[]
> @@ -195,8 +194,9 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
[]
> + dev_printk(KERN_ERR, &dev->dev,
> + " Error of this Agent(%04x) is reported first\n",
> + id);
[]
> @@ -244,21 +244,21 @@ void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
[]
> + dev_printk(KERN_ERR, &dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
> + status, mask);
[]
> + dev_printk(KERN_ERR, &dev->dev, "aer_layer=%s, aer_agent=%s\n",
> aer_error_layer[layer], aer_agent_string[agent]);
[]
> + dev_printk(KERN_ERR, &dev->dev, "aer_tlp_header:"
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 3/3] aerdrv: Cleanup log output for AER
2013-01-02 23:27 [PATCH v8 1/3] aerdrv: Trace Event for AER Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 2/3] aerdrv: Enhanced AER logging Lance Ortiz
@ 2013-01-02 23:27 ` Lance Ortiz
2013-01-02 23:27 ` Joe Perches
2013-01-03 0:06 ` [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL> Joe Perches
1 sibling, 2 replies; 12+ messages in thread
From: Lance Ortiz @ 2013-01-02 23:27 UTC (permalink / raw)
To: bhelgaas, lance_ortiz, jiang.liu, tony.luck, bp, rostedt, mchehab,
linux-acpi, linux-pci, linux-kernel
These changes make cper_print_aer more consistent with aer_print_error
and clean things up by elimiating the use of the prefix variable and
replacing it with dev_printk.
v1-v2 fix some compile errors withinn the #ifdef
v3-v4 remove agent id stuff and kept print the same to avoid
compatibility issues
v7-v8 Updated to use dev_printk instated of prefix. Changed
log levels to KERN_ERR as per Mauro's suggestion.
Signed-off-by: Lance Ortiz <lance.ortiz@hp.com>
---
drivers/pci/pcie/aer/aerdrv_errprint.c | 56 ++++++++++++++++----------------
1 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index d3e5fc5..dfd9641 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -124,12 +124,11 @@ static const char *aer_agent_string[] = {
"Transmitter ID"
};
-static void __aer_print_error(const char *prefix,
+static void __aer_print_error(struct pci_dev *dev,
struct aer_err_info *info)
{
int i, status;
const char *errmsg = NULL;
-
status = (info->status & ~info->mask);
for (i = 0; i < 32; i++) {
@@ -144,26 +143,24 @@ static void __aer_print_error(const char *prefix,
aer_uncorrectable_error_string[i] : NULL;
if (errmsg)
- printk("%s"" [%2d] %-22s%s\n", prefix, i, errmsg,
+ dev_printk(KERN_ERR, &dev->dev,
+ " [%2d] %-22s%s\n", i, errmsg,
info->first_error == i ? " (First)" : "");
else
- printk("%s"" [%2d] Unknown Error Bit%s\n", prefix, i,
- info->first_error == i ? " (First)" : "");
+ dev_printk(KERN_ERR, &dev->dev,
+ " [%2d] Unknown Error Bit%s\n",
+ i, info->first_error == i ? " (First)" : "");
}
}
void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
{
int id = ((dev->bus->number << 8) | dev->devfn);
- char prefix[44];
-
- snprintf(prefix, sizeof(prefix), "%s%s %s: ",
- (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR,
- dev_driver_string(&dev->dev), dev_name(&dev->dev));
if (info->status == 0) {
- printk("%s""PCIe Bus Error: severity=%s, type=Unaccessible, "
- "id=%04x(Unregistered Agent ID)\n", prefix,
+ dev_printk(KERN_ERR, &dev->dev,
+ "PCIe Bus Error: severity=%s, type=Unaccessible, "
+ "id=%04x(Unregistered Agent ID)\n",
aer_error_severity_string[info->severity], id);
} else {
int layer, agent;
@@ -171,22 +168,24 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
layer = AER_GET_LAYER_ERROR(info->severity, info->status);
agent = AER_GET_AGENT(info->severity, info->status);
- printk("%s""PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
- prefix, aer_error_severity_string[info->severity],
+ dev_printk(KERN_ERR, &dev->dev,
+ "PCIe Bus Error: severity=%s, type=%s, id=%04x(%s)\n",
+ aer_error_severity_string[info->severity],
aer_error_layer[layer], id, aer_agent_string[agent]);
- printk("%s"" device [%04x:%04x] error status/mask=%08x/%08x\n",
- prefix, dev->vendor, dev->device,
+ dev_printk(KERN_ERR, &dev->dev,
+ " device [%04x:%04x] error status/mask=%08x/%08x\n",
+ dev->vendor, dev->device,
info->status, info->mask);
- __aer_print_error(prefix, info);
+ __aer_print_error(dev, info);
if (info->tlp_header_valid) {
unsigned char *tlp = (unsigned char *) &info->tlp;
- printk("%s"" TLP Header:"
+ dev_printk(KERN_ERR, &dev->dev, " TLP Header:"
" %02x%02x%02x%02x %02x%02x%02x%02x"
" %02x%02x%02x%02x %02x%02x%02x%02x\n",
- prefix, *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
+ *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
*(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4),
*(tlp + 11), *(tlp + 10), *(tlp + 9),
*(tlp + 8), *(tlp + 15), *(tlp + 14),
@@ -195,8 +194,9 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
}
if (info->id && info->error_dev_num > 1 && info->id == id)
- printk("%s"" Error of this Agent(%04x) is reported first\n",
- prefix, id);
+ dev_printk(KERN_ERR, &dev->dev,
+ " Error of this Agent(%04x) is reported first\n",
+ id);
trace_aer_event(dev_name(&dev->dev), (info->status & ~info->mask),
info->severity);
}
@@ -244,21 +244,21 @@ void cper_print_aer(const char *prefix, struct pci_dev *dev, int cper_severity,
}
layer = AER_GET_LAYER_ERROR(aer_severity, status);
agent = AER_GET_AGENT(aer_severity, status);
- printk("%s""aer_status: 0x%08x, aer_mask: 0x%08x\n",
- prefix, status, mask);
+ dev_printk(KERN_ERR, &dev->dev, "aer_status: 0x%08x, aer_mask: 0x%08x\n",
+ status, mask);
cper_print_bits(prefix, status, status_strs, status_strs_size);
- printk("%s""aer_layer=%s, aer_agent=%s\n", prefix,
+ dev_printk(KERN_ERR, &dev->dev, "aer_layer=%s, aer_agent=%s\n",
aer_error_layer[layer], aer_agent_string[agent]);
if (aer_severity != AER_CORRECTABLE)
- printk("%s""aer_uncor_severity: 0x%08x\n",
- prefix, aer->uncor_severity);
+ dev_printk(KERN_ERR, &dev->dev, "aer_uncor_severity: 0x%08x\n",
+ aer->uncor_severity);
if (tlp_header_valid) {
const unsigned char *tlp;
tlp = (const unsigned char *)&aer->header_log;
- printk("%s""aer_tlp_header:"
+ dev_printk(KERN_ERR, &dev->dev, "aer_tlp_header:"
" %02x%02x%02x%02x %02x%02x%02x%02x"
" %02x%02x%02x%02x %02x%02x%02x%02x\n",
- prefix, *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
+ *(tlp + 3), *(tlp + 2), *(tlp + 1), *tlp,
*(tlp + 7), *(tlp + 6), *(tlp + 5), *(tlp + 4),
*(tlp + 11), *(tlp + 10), *(tlp + 9),
*(tlp + 8), *(tlp + 15), *(tlp + 14),
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v8 3/3] aerdrv: Cleanup log output for AER
2013-01-02 23:27 ` Joe Perches
@ 2013-01-02 23:41 ` Tony Luck
2013-01-03 15:51 ` Borislav Petkov
0 siblings, 1 reply; 12+ messages in thread
From: Tony Luck @ 2013-01-02 23:41 UTC (permalink / raw)
To: Joe Perches
Cc: Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu, bp, rostedt,
mchehab, linux-acpi, linux-pci, linux-kernel
On Wed, Jan 2, 2013 at 3:27 PM, Joe Perches <joe@perches.com> wrote:
> Just use dev_err( instead of dev_printk(KERN_ERR,
> It's a function and it makes the object code smaller.
Looks like we are almost converged on a solution (Lance: thanks for
your patience and diligence in making changes).
Anyone on the "To:" list want to claim this for their tree to commit?
The series touches pci, acpi, RAS, and tracing ... so there are
several possible owners.
If someone else wants it, then add an:
Acked-by: Tony Luck <tony.luck@intel.com>
to all three parts.
If there isn't a strong claim, I'll add v9[*] to the RAS tree
and see if the TIP tree folks will pull it from me.
-Tony
[*] When Lance makes the change suggested by Joe.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-02 23:27 ` [PATCH v8 3/3] aerdrv: Cleanup log output for AER Lance Ortiz
2013-01-02 23:27 ` Joe Perches
@ 2013-01-03 0:06 ` Joe Perches
2013-01-03 0:34 ` Bjorn Helgaas
2013-01-03 10:08 ` Andy Whitcroft
1 sibling, 2 replies; 12+ messages in thread
From: Joe Perches @ 2013-01-03 0:06 UTC (permalink / raw)
To: Andrew Morton, Andy Whitcroft
Cc: Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu, tony.luck, bp,
rostedt, mchehab, linux-acpi, linux-pci, linux-kernel
Add YA check to printk style.
dev_<level> uses are functions and generate smaller
object code than dev_printk(KERN_<LEVEL>.
Signed-off-by: Joe Perches <joe@perches.com>
---
scripts/checkpatch.pl | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4d2c7df..f50b32d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2430,6 +2430,16 @@ sub process {
"Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
}
+ if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
+ my $orig = $1;
+ my $level = lc($orig);
+ $level = "warn" if ($level eq "warning");
+ my $level2 = $level;
+ $level2 = "dbg" if ($level eq "debug");
+ WARN("PREFER_DEV_LEVEL",
+ "Prefer dev_$level2(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
+ }
+
# function brace can't be on same line, except for #defines of do while,
# or if closed on same line
if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-03 0:06 ` [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL> Joe Perches
@ 2013-01-03 0:34 ` Bjorn Helgaas
2013-01-03 0:39 ` Joe Perches
2013-01-03 10:08 ` Andy Whitcroft
1 sibling, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2013-01-03 0:34 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Andy Whitcroft, Lance Ortiz, lance_ortiz,
jiang.liu, tony.luck, bp, rostedt, mchehab, linux-acpi, linux-pci,
linux-kernel
On Wed, Jan 2, 2013 at 6:06 PM, Joe Perches <joe@perches.com> wrote:
> Add YA check to printk style.
>
> dev_<level> uses are functions and generate smaller
> object code than dev_printk(KERN_<LEVEL>.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> scripts/checkpatch.pl | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4d2c7df..f50b32d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2430,6 +2430,16 @@ sub process {
> "Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
> }
>
> + if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
> + my $orig = $1;
> + my $level = lc($orig);
> + $level = "warn" if ($level eq "warning");
> + my $level2 = $level;
> + $level2 = "dbg" if ($level eq "debug");
> + WARN("PREFER_DEV_LEVEL",
> + "Prefer dev_$level2(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
This suggests dev_dbg() instead of dev_printk(KERN_DEBUG), doesn't it?
Those aren't equivalent (dev_printk() always does the printk, but in
many cases dev_dbg() does not, depending on CONFIG_DEBUG,
CONFIG_DYNAMIC_DEBUG, etc.)
> + }
> +
> # function brace can't be on same line, except for #defines of do while,
> # or if closed on same line
> if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-03 0:34 ` Bjorn Helgaas
@ 2013-01-03 0:39 ` Joe Perches
0 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2013-01-03 0:39 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Andrew Morton, Andy Whitcroft, Lance Ortiz, lance_ortiz,
jiang.liu, tony.luck, bp, rostedt, mchehab, linux-acpi, linux-pci,
linux-kernel
On Wed, 2013-01-02 at 18:34 -0600, Bjorn Helgaas wrote:
> On Wed, Jan 2, 2013 at 6:06 PM, Joe Perches <joe@perches.com> wrote:
> > Add YA check to printk style.
> >
> > dev_<level> uses are functions and generate smaller
> > object code than dev_printk(KERN_<LEVEL>.
[]
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > @@ -2430,6 +2430,16 @@ sub process {
> > "Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
> > }
> >
> > + if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
> > + my $orig = $1;
> > + my $level = lc($orig);
> > + $level = "warn" if ($level eq "warning");
> > + my $level2 = $level;
> > + $level2 = "dbg" if ($level eq "debug");
> > + WARN("PREFER_DEV_LEVEL",
> > + "Prefer dev_$level2(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
>
> This suggests dev_dbg() instead of dev_printk(KERN_DEBUG), doesn't it?
> Those aren't equivalent (dev_printk() always does the printk, but in
> many cases dev_dbg() does not, depending on CONFIG_DEBUG,
> CONFIG_DYNAMIC_DEBUG, etc.)
True, I think the suggested change is appropriate though.
If people really want debug output in a specific file,
then they should #define DEBUG and/or enable dynamic debug.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-03 0:06 ` [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL> Joe Perches
2013-01-03 0:34 ` Bjorn Helgaas
@ 2013-01-03 10:08 ` Andy Whitcroft
2013-01-03 16:28 ` Joe Perches
1 sibling, 1 reply; 12+ messages in thread
From: Andy Whitcroft @ 2013-01-03 10:08 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu,
tony.luck, bp, rostedt, mchehab, linux-acpi, linux-pci,
linux-kernel
On Wed, Jan 02, 2013 at 04:06:34PM -0800, Joe Perches wrote:
> Add YA check to printk style.
>
> dev_<level> uses are functions and generate smaller
> object code than dev_printk(KERN_<LEVEL>.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> scripts/checkpatch.pl | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4d2c7df..f50b32d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2430,6 +2430,16 @@ sub process {
> "Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
> }
>
> + if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
> + my $orig = $1;
> + my $level = lc($orig);
> + $level = "warn" if ($level eq "warning");
> + my $level2 = $level;
> + $level2 = "dbg" if ($level eq "debug");
Is there some sublty I am not seeing here such that level2 is necessary?
As far as I can see the two above could be the below to the same effect?
$level2= "dbg" if ($level eq "debug");
(With the obvious change to the print of course)
-apw
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 3/3] aerdrv: Cleanup log output for AER
2013-01-02 23:41 ` Tony Luck
@ 2013-01-03 15:51 ` Borislav Petkov
0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-01-03 15:51 UTC (permalink / raw)
To: Tony Luck
Cc: Joe Perches, Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu,
rostedt, mchehab, linux-acpi, linux-pci, linux-kernel
On Wed, Jan 02, 2013 at 03:41:19PM -0800, Tony Luck wrote:
> On Wed, Jan 2, 2013 at 3:27 PM, Joe Perches <joe@perches.com> wrote:
> > Just use dev_err( instead of dev_printk(KERN_ERR,
> > It's a function and it makes the object code smaller.
>
> Looks like we are almost converged on a solution (Lance: thanks for
> your patience and diligence in making changes).
>
> Anyone on the "To:" list want to claim this for their tree to commit?
> The series touches pci, acpi, RAS, and tracing ... so there are
> several possible owners.
>
> If someone else wants it, then add an:
> Acked-by: Tony Luck <tony.luck@intel.com>
> to all three parts.
Ditto.
Acked-by: Borislav Petkov <bp@alien8.de>
Thanks.
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-03 10:08 ` Andy Whitcroft
@ 2013-01-03 16:28 ` Joe Perches
2013-01-03 16:49 ` [PATCH V2] " Joe Perches
0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-01-03 16:28 UTC (permalink / raw)
To: Andy Whitcroft
Cc: Andrew Morton, Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu,
tony.luck, bp, rostedt, mchehab, linux-acpi, linux-pci,
linux-kernel
On Thu, 2013-01-03 at 10:08 +0000, Andy Whitcroft wrote:
> On Wed, Jan 02, 2013 at 04:06:34PM -0800, Joe Perches wrote:
> > Add YA check to printk style.
> >
> > dev_<level> uses are functions and generate smaller
> > object code than dev_printk(KERN_<LEVEL>.
[]
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > + if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
> > + my $orig = $1;
> > + my $level = lc($orig);
> > + $level = "warn" if ($level eq "warning");
> > + my $level2 = $level;
> > + $level2 = "dbg" if ($level eq "debug");
>
> Is there some sublty I am not seeing here such that level2 is necessary?
No, your vision is 20/20, it's not necessary.
It was a copy/paste from the pr_<level> conversion above it
where 2 styles to convert to (dev_dbg and pr_debug) are used.
Here there is just one form and your review is appreciated.
I'll submit a V2 later.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL>
2013-01-03 16:28 ` Joe Perches
@ 2013-01-03 16:49 ` Joe Perches
0 siblings, 0 replies; 12+ messages in thread
From: Joe Perches @ 2013-01-03 16:49 UTC (permalink / raw)
To: Andy Whitcroft
Cc: Andrew Morton, Lance Ortiz, bhelgaas, lance_ortiz, jiang.liu,
tony.luck, bp, rostedt, mchehab, linux-acpi, linux-pci,
linux-kernel
Add YA check to printk style.
dev_<level> uses are functions and generate smaller
object code than dev_printk(KERN_<LEVEL>.
Signed-off-by: Joe Perches <joe@perches.com>
---
V2: Remove unnecessary copy/paste logic from pr_<level>
conversion above it.
scripts/checkpatch.pl | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9de3a69..9a1ae72 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2425,6 +2425,15 @@ sub process {
"Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
}
+ if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) {
+ my $orig = $1;
+ my $level = lc($orig);
+ $level = "warn" if ($level eq "warning");
+ $level = "dbg" if ($level eq "debug");
+ WARN("PREFER_DEV_LEVEL",
+ "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr);
+ }
+
# function brace can't be on same line, except for #defines of do while,
# or if closed on same line
if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-01-03 16:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-02 23:27 [PATCH v8 1/3] aerdrv: Trace Event for AER Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 2/3] aerdrv: Enhanced AER logging Lance Ortiz
2013-01-02 23:27 ` [PATCH v8 3/3] aerdrv: Cleanup log output for AER Lance Ortiz
2013-01-02 23:27 ` Joe Perches
2013-01-02 23:41 ` Tony Luck
2013-01-03 15:51 ` Borislav Petkov
2013-01-03 0:06 ` [PATCH] checkpatch: prefer dev_<level>( to dev_printk(KERN_<LEVEL> Joe Perches
2013-01-03 0:34 ` Bjorn Helgaas
2013-01-03 0:39 ` Joe Perches
2013-01-03 10:08 ` Andy Whitcroft
2013-01-03 16:28 ` Joe Perches
2013-01-03 16:49 ` [PATCH V2] " Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).