[PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
Define a structure to hold the AER statistics. There are 2 groups
of statistics: dev_* counters that are to be collected for all AER
capable devices and rootport_* counters that are collected for all
(AER capable) rootports only. Allocate and free this structure when
device is added or released (thus counters survive the lifetime of the
device).
Add a new file aerdrv_stats.c to hold the AER stats collection logic.
Signed-off-by: Rajat Jain
---
drivers/pci/pcie/aer/Makefile | 2 +-
drivers/pci/pcie/aer/aerdrv.h | 6 +++
drivers/pci/pcie/aer/aerdrv_core.c | 9
drivers/pci/pcie/aer/aerdrv_stats.c | 64 +
drivers/pci/probe.c | 1 +
include/linux/pci.h | 3 ++
6 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644 drivers/pci/pcie/aer/aerdrv_stats.c
diff --git a/drivers/pci/pcie/aer/Makefile b/drivers/pci/pcie/aer/Makefile
index 09bd890875a3..a06f9cc2bde5 100644
--- a/drivers/pci/pcie/aer/Makefile
+++ b/drivers/pci/pcie/aer/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_PCIEAER) += aerdriver.o
obj-$(CONFIG_PCIE_ECRC)+= ecrc.o
-aerdriver-objs := aerdrv_errprint.o aerdrv_core.o aerdrv.o
+aerdriver-objs := aerdrv_errprint.o aerdrv_core.o aerdrv.o aerdrv_stats.o
aerdriver-$(CONFIG_ACPI) += aerdrv_acpi.o
obj-$(CONFIG_PCIEAER_INJECT) += aer_inject.o
diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h
index b4c950683cc7..d8b9fba536ed 100644
--- a/drivers/pci/pcie/aer/aerdrv.h
+++ b/drivers/pci/pcie/aer/aerdrv.h
@@ -33,6 +33,10 @@
PCI_ERR_UNC_MALF_TLP)
#define AER_MAX_MULTI_ERR_DEVICES 5 /* Not likely to have more */
+
+#define AER_MAX_TYPEOF_CORRECTABLE_ERRS 16 /* as per PCI_ERR_COR_STATUS */
+#define AER_MAX_TYPEOF_UNCORRECTABLE_ERRS 26 /* as per PCI_ERR_UNCOR_STATUS*/
+
struct aer_err_info {
struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
int error_dev_num;
@@ -81,6 +85,8 @@ void aer_isr(struct work_struct *work);
void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info);
irqreturn_t aer_irq(int irq, void *context);
+int pci_aer_stats_init(struct pci_dev *pdev);
+void pci_aer_stats_exit(struct pci_dev *pdev);
#ifdef CONFIG_ACPI_APEI
int pcie_aer_get_firmware_first(struct pci_dev *pci_dev);
diff --git a/drivers/pci/pcie/aer/aerdrv_core.c
b/drivers/pci/pcie/aer/aerdrv_core.c
index 36e622d35c48..42a6f913069a 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -95,9 +95,18 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev)
int pci_aer_init(struct pci_dev *dev)
{
dev->aer_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
+
+ if (!dev->aer_cap || pci_aer_stats_init(dev))
+ return -EIO;
+
return pci_cleanup_aer_error_status_regs(dev);
}
+void pci_aer_exit(struct pci_dev *dev)
+{
+ pci_aer_stats_exit(dev);
+}
+
/**
* add_error_device - list device to be handled
* @e_info: pointer to error info
diff --git a/drivers/pci/pcie/aer/aerdrv_stats.c
b/drivers/pci/pcie/aer/aerdrv_stats.c
new file mode 100644
index ..b9f251992209
--- /dev/null
+++ b/drivers/pci/pcie/aer/aerdrv_stats.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Google Inc, All Rights Reserved.
+ * Rajat Jain (raja...@google.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * AER Statistics - exposed to userspace via /sysfs attributes.
+ */
+
+#include
+#include "aerdrv.h"
+
+/* AER stats for the device */
+struct aer_stats {
+
+ /*
+* Fields for all AER capable devices. They indicate the errors
+* "as seen by this device". Note that this may mean that if an
+* end point is causing problems, the AER counters may increment
+* at its link partner (e.g. root port) because the errors will be
+* "seen" by the link partner and not the the problematic end point
+* itself (which may report all counters as 0 as it never saw any
+* problems).
+*/
+ /* Individual counters for different type of correctable errors */
+ u64 dev_cor_errs[AER_MAX_TYPEOF_CORRECTABLE_ERRS];
+ /* Individual counters for different type of uncorrectable errors */
+ u64 dev_uncor_errs[AER_MAX_TYPEOF_UNCORRECTABLE_ERRS];
+ /* Total number of correctable errors seen by this device */
+ u64 dev_total_cor_errs;
+ /* Total number of fatal uncorrectable errors seen by this device */
+ u64 dev_total_fatal_errs;
+ /* Total number of fatal uncorrectable errors seen by this device */
+ u64 dev_total_nonfatal_errs;
+
+ /*
+* Fields for Root ports only, these indicate
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On Tue, May 22, 2018 at 03:28:01PM -0700, Rajat Jain wrote: > --- /dev/null > +++ b/drivers/pci/pcie/aer/aerdrv_stats.c > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Google Inc, All Rights Reserved. > + * Rajat Jain (raja...@google.com) Google has the copyright, not you, right? You might want to make that a bit more explicit by putting a blank line somewhere here... > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. If you have a SPDX line, you do not need this paragraph. Please drop it so we don't have to delete it later on. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On 05/22/2018 06:28 PM, Rajat Jain wrote:
> Define a structure to hold the AER statistics. There are 2 groups
> of statistics: dev_* counters that are to be collected for all AER
> capable devices and rootport_* counters that are collected for all
> (AER capable) rootports only. Allocate and free this structure when
> device is added or released (thus counters survive the lifetime of the
> device).
>
> Add a new file aerdrv_stats.c to hold the AER stats collection logic.
>
> Signed-off-by: Rajat Jain
> ---
> drivers/pci/pcie/aer/Makefile | 2 +-
> drivers/pci/pcie/aer/aerdrv.h | 6 +++
> drivers/pci/pcie/aer/aerdrv_core.c | 9
> drivers/pci/pcie/aer/aerdrv_stats.c | 64 +
> drivers/pci/probe.c | 1 +
> include/linux/pci.h | 3 ++
> 6 files changed, 84 insertions(+), 1 deletion(-)
> create mode 100644 drivers/pci/pcie/aer/aerdrv_stats.c
>
> diff --git a/drivers/pci/pcie/aer/Makefile b/drivers/pci/pcie/aer/Makefile
>
> -aerdriver-objs := aerdrv_errprint.o aerdrv_core.o aerdrv.o
> +aerdriver-objs := aerdrv_errprint.o aerdrv_core.o aerdrv.o aerdrv_stats.o
> aerdriver-$(CONFIG_ACPI) += aerdrv_acpi.o
>
> obj-$(CONFIG_PCIEAER_INJECT) += aer_inject.o
> diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h
> index b4c950683cc7..d8b9fba536ed 100644
> --- a/drivers/pci/pcie/aer/aerdrv.h
> +++ b/drivers/pci/pcie/aer/aerdrv.h
> @@ -33,6 +33,10 @@
> PCI_ERR_UNC_MALF_TLP)
>
> #define AER_MAX_MULTI_ERR_DEVICES5 /* Not likely to have more */
> +
> +#define AER_MAX_TYPEOF_CORRECTABLE_ERRS 16 /* as per PCI_ERR_COR_STATUS */
> +#define AER_MAX_TYPEOF_UNCORRECTABLE_ERRS 26 /* as per PCI_ERR_UNCOR_STATUS*/
> +
> struct aer_err_info {
> struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
> int error_dev_num;
> @@ -81,6 +85,8 @@ void aer_isr(struct work_struct *work);
> void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
> void aer_print_port_info(struct pci_dev *dev, struct aer_err_info *info);
> irqreturn_t aer_irq(int irq, void *context);
> +int pci_aer_stats_init(struct pci_dev *pdev);
> +void pci_aer_stats_exit(struct pci_dev *pdev);
>
> #ifdef CONFIG_ACPI_APEI
> int pcie_aer_get_firmware_first(struct pci_dev *pci_dev);
> diff --git a/drivers/pci/pcie/aer/aerdrv_core.c
> b/drivers/pci/pcie/aer/aerdrv_core.c
> index 36e622d35c48..42a6f913069a 100644
> --- a/drivers/pci/pcie/aer/aerdrv_core.c
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> @@ -95,9 +95,18 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev)
> int pci_aer_init(struct pci_dev *dev)
> {
> dev->aer_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
> +
> + if (!dev->aer_cap || pci_aer_stats_init(dev))
> + return -EIO;
> +
> return pci_cleanup_aer_error_status_regs(dev);
> }
>
> +void pci_aer_exit(struct pci_dev *dev)
> +{
> + pci_aer_stats_exit(dev);
> +}
> +
> /**
> * add_error_device - list device to be handled
> * @e_info: pointer to error info
> diff --git a/drivers/pci/pcie/aer/aerdrv_stats.c
> b/drivers/pci/pcie/aer/aerdrv_stats.c
> new file mode 100644
> index ..b9f251992209
> --- /dev/null
> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0
Fix the formatting please - that gross // gibberish doesn't belong there.
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On 05/23/2018 09:20 AM, Jes Sorensen wrote: > On 05/22/2018 06:28 PM, Rajat Jain wrote: >> new file mode 100644 >> index ..b9f251992209 >> --- /dev/null >> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c >> @@ -0,0 +1,64 @@ >> +// SPDX-License-Identifier: GPL-2.0 > > Fix the formatting please - that gross // gibberish doesn't belong there. Deep breath in. Deep breath out. git grep SPDX Although I don't like it, this format is already too common. Cheers, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On Wed, May 23, 2018 at 10:20:10AM -0400, Jes Sorensen wrote: > > +++ b/drivers/pci/pcie/aer/aerdrv_stats.c > > @@ -0,0 +1,64 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > Fix the formatting please - that gross // gibberish doesn't belong there. Sorry, Jes. The Chief Penguin has Spoken, and that's the preferred syntax: 2. Style: The SPDX license identifier is added in form of a comment. The comment style depends on the file type:: C source: // SPDX-License-Identifier: (you can dig up the discussion around this on the mailing list if you like. Linus actually thinks that C++ single-line comments are one of the few things that language got right) -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On 05/23/2018 10:26 AM, Alex G. wrote: > On 05/23/2018 09:20 AM, Jes Sorensen wrote: >> On 05/22/2018 06:28 PM, Rajat Jain wrote: >>> new file mode 100644 >>> index ..b9f251992209 >>> --- /dev/null >>> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c >>> @@ -0,0 +1,64 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >> >> Fix the formatting please - that gross // gibberish doesn't belong there. > > Deep breath in. Deep breath out. > > git grep SPDX > > Although I don't like it, this format is already too common. So? Just because some people did something wrong doesn't mean you should continue to do it. Jes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On 05/23/2018 09:32 AM, Jes Sorensen wrote: > On 05/23/2018 10:26 AM, Matthew Wilcox wrote: >> On Wed, May 23, 2018 at 10:20:10AM -0400, Jes Sorensen wrote: +++ b/drivers/pci/pcie/aer/aerdrv_stats.c @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0 >>> >>> Fix the formatting please - that gross // gibberish doesn't belong there. >> >> Sorry, Jes. The Chief Penguin has Spoken, and that's the preferred >> syntax: >> >> 2. Style: >> >>The SPDX license identifier is added in form of a comment. The comment >>style depends on the file type:: >> >> C source: // SPDX-License-Identifier: >> >> (you can dig up the discussion around this on the mailing list if you >> like. Linus actually thinks that C++ single-line comments are one of >> the few things that language got right) > > Well I'll agree to disagree with Linus on this one. It's ugly as fsck > and allows for ambiguous statements in the code. You misspelled "fuck". Alex -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On 05/23/2018 10:26 AM, Matthew Wilcox wrote: > On Wed, May 23, 2018 at 10:20:10AM -0400, Jes Sorensen wrote: >>> +++ b/drivers/pci/pcie/aer/aerdrv_stats.c >>> @@ -0,0 +1,64 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >> >> Fix the formatting please - that gross // gibberish doesn't belong there. > > Sorry, Jes. The Chief Penguin has Spoken, and that's the preferred > syntax: > > 2. Style: > >The SPDX license identifier is added in form of a comment. The comment >style depends on the file type:: > > C source: // SPDX-License-Identifier: > > (you can dig up the discussion around this on the mailing list if you > like. Linus actually thinks that C++ single-line comments are one of > the few things that language got right) Well I'll agree to disagree with Linus on this one. It's ugly as fsck and allows for ambiguous statements in the code. Jes -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] PCI/AER: Define and allocate aer_stats structure for AER capable devices
On Wed, 23 May 2018 09:33:30 -0500 "Alex G." wrote: > > Well I'll agree to disagree with Linus on this one. It's ugly as fsck > > and allows for ambiguous statements in the code. > > You misspelled "fuck". No, Jes is Danish. That's how they spell it. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
