[PATCH 4/4] PCI: Descope pci_printk() to aer_printk()

Ilpo Järvinen posted 4 patches 1 year, 1 month ago
[PATCH 4/4] PCI: Descope pci_printk() to aer_printk()
Posted by Ilpo Järvinen 1 year, 1 month ago
include/linux/pci.h provides low-level pci_printk() interface that is
only used by AER because it needs to print the same message with
different levels depending on the error severity. No other PCI code
uses that functionality and calls pci_<level>() logging functions
directly with the appropriate level.

Descope pci_printk() into AER as aer_printk().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pcie/aer.c | 10 +++++++---
 include/linux/pci.h    |  3 ---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 80c5ba8d8296..bfc6b94dad4d 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -17,6 +17,7 @@
 
 #include <linux/bitops.h>
 #include <linux/cper.h>
+#include <linux/dev_printk.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/sched.h>
@@ -35,6 +36,9 @@
 #include "../pci.h"
 #include "portdrv.h"
 
+#define aer_printk(level, pdev, fmt, arg...) \
+	dev_printk(level, &(pdev)->dev, fmt, ##arg)
+
 #define AER_ERROR_SOURCES_MAX		128
 
 #define AER_MAX_TYPEOF_COR_ERRS		16	/* as per PCI_ERR_COR_STATUS */
@@ -692,7 +696,7 @@ static void __aer_print_error(struct pci_dev *dev,
 		if (!errmsg)
 			errmsg = "Unknown Error Bit";
 
-		pci_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
+		aer_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
 				info->first_error == i ? " (First)" : "");
 	}
 	pci_dev_aer_stats_incr(dev, info);
@@ -715,11 +719,11 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 
 	level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
 
-	pci_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
+	aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
 		   aer_error_severity_string[info->severity],
 		   aer_error_layer[layer], aer_agent_string[agent]);
 
-	pci_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
+	aer_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
 		   dev->vendor, dev->device, info->status, info->mask);
 
 	__aer_print_error(dev, info);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index db9b47ce3eef..02d23e795915 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2685,9 +2685,6 @@ void pci_uevent_ers(struct pci_dev *pdev, enum  pci_ers_result err_type);
 
 #include <linux/dma-mapping.h>
 
-#define pci_printk(level, pdev, fmt, arg...) \
-	dev_printk(level, &(pdev)->dev, fmt, ##arg)
-
 #define pci_emerg(pdev, fmt, arg...)	dev_emerg(&(pdev)->dev, fmt, ##arg)
 #define pci_alert(pdev, fmt, arg...)	dev_alert(&(pdev)->dev, fmt, ##arg)
 #define pci_crit(pdev, fmt, arg...)	dev_crit(&(pdev)->dev, fmt, ##arg)
-- 
2.39.5

Re: [PATCH 4/4] PCI: Descope pci_printk() to aer_printk()
Posted by Bjorn Helgaas 12 months ago
On Mon, Dec 16, 2024 at 06:10:12PM +0200, Ilpo Järvinen wrote:
> include/linux/pci.h provides low-level pci_printk() interface that is
> only used by AER because it needs to print the same message with
> different levels depending on the error severity. No other PCI code
> uses that functionality and calls pci_<level>() logging functions
> directly with the appropriate level.
> 
> Descope pci_printk() into AER as aer_printk().
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

I applied this patch by itself on pci/aer for v6.15.

We also have some work-in-progress on rate limiting errors, which
might conflict, but this is simple and shouldn't be hard to reconcile.

> ---
>  drivers/pci/pcie/aer.c | 10 +++++++---
>  include/linux/pci.h    |  3 ---
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 80c5ba8d8296..bfc6b94dad4d 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/bitops.h>
>  #include <linux/cper.h>
> +#include <linux/dev_printk.h>
>  #include <linux/pci.h>
>  #include <linux/pci-acpi.h>
>  #include <linux/sched.h>
> @@ -35,6 +36,9 @@
>  #include "../pci.h"
>  #include "portdrv.h"
>  
> +#define aer_printk(level, pdev, fmt, arg...) \
> +	dev_printk(level, &(pdev)->dev, fmt, ##arg)
> +
>  #define AER_ERROR_SOURCES_MAX		128
>  
>  #define AER_MAX_TYPEOF_COR_ERRS		16	/* as per PCI_ERR_COR_STATUS */
> @@ -692,7 +696,7 @@ static void __aer_print_error(struct pci_dev *dev,
>  		if (!errmsg)
>  			errmsg = "Unknown Error Bit";
>  
> -		pci_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
> +		aer_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
>  				info->first_error == i ? " (First)" : "");
>  	}
>  	pci_dev_aer_stats_incr(dev, info);
> @@ -715,11 +719,11 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  
>  	level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
>  
> -	pci_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
> +	aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
>  		   aer_error_severity_string[info->severity],
>  		   aer_error_layer[layer], aer_agent_string[agent]);
>  
> -	pci_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
> +	aer_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
>  		   dev->vendor, dev->device, info->status, info->mask);
>  
>  	__aer_print_error(dev, info);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index db9b47ce3eef..02d23e795915 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2685,9 +2685,6 @@ void pci_uevent_ers(struct pci_dev *pdev, enum  pci_ers_result err_type);
>  
>  #include <linux/dma-mapping.h>
>  
> -#define pci_printk(level, pdev, fmt, arg...) \
> -	dev_printk(level, &(pdev)->dev, fmt, ##arg)
> -
>  #define pci_emerg(pdev, fmt, arg...)	dev_emerg(&(pdev)->dev, fmt, ##arg)
>  #define pci_alert(pdev, fmt, arg...)	dev_alert(&(pdev)->dev, fmt, ##arg)
>  #define pci_crit(pdev, fmt, arg...)	dev_crit(&(pdev)->dev, fmt, ##arg)
> -- 
> 2.39.5
> 
Re: [PATCH 4/4] PCI: Descope pci_printk() to aer_printk()
Posted by Ilpo Järvinen 12 months ago
On Thu, 13 Feb 2025, Bjorn Helgaas wrote:

> On Mon, Dec 16, 2024 at 06:10:12PM +0200, Ilpo Järvinen wrote:
> > include/linux/pci.h provides low-level pci_printk() interface that is
> > only used by AER because it needs to print the same message with
> > different levels depending on the error severity. No other PCI code
> > uses that functionality and calls pci_<level>() logging functions
> > directly with the appropriate level.
> > 
> > Descope pci_printk() into AER as aer_printk().
> > 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> I applied this patch by itself on pci/aer for v6.15.
> 
> We also have some work-in-progress on rate limiting errors, which
> might conflict, but this is simple and shouldn't be hard to reconcile.
> 
> > ---
> >  drivers/pci/pcie/aer.c | 10 +++++++---
> >  include/linux/pci.h    |  3 ---
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index 80c5ba8d8296..bfc6b94dad4d 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -17,6 +17,7 @@
> >  
> >  #include <linux/bitops.h>
> >  #include <linux/cper.h>
> > +#include <linux/dev_printk.h>
> >  #include <linux/pci.h>
> >  #include <linux/pci-acpi.h>
> >  #include <linux/sched.h>
> > @@ -35,6 +36,9 @@
> >  #include "../pci.h"
> >  #include "portdrv.h"
> >  
> > +#define aer_printk(level, pdev, fmt, arg...) \
> > +	dev_printk(level, &(pdev)->dev, fmt, ##arg)
> > +
> >  #define AER_ERROR_SOURCES_MAX		128
> >  
> >  #define AER_MAX_TYPEOF_COR_ERRS		16	/* as per PCI_ERR_COR_STATUS */
> > @@ -692,7 +696,7 @@ static void __aer_print_error(struct pci_dev *dev,
> >  		if (!errmsg)
> >  			errmsg = "Unknown Error Bit";
> >  
> > -		pci_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
> > +		aer_printk(level, dev, "   [%2d] %-22s%s\n", i, errmsg,
> >  				info->first_error == i ? " (First)" : "");
> >  	}
> >  	pci_dev_aer_stats_incr(dev, info);
> > @@ -715,11 +719,11 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
> >  
> >  	level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
> >  
> > -	pci_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
> > +	aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
> >  		   aer_error_severity_string[info->severity],
> >  		   aer_error_layer[layer], aer_agent_string[agent]);
> >  
> > -	pci_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
> > +	aer_printk(level, dev, "  device [%04x:%04x] error status/mask=%08x/%08x\n",
> >  		   dev->vendor, dev->device, info->status, info->mask);
> >  
> >  	__aer_print_error(dev, info);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index db9b47ce3eef..02d23e795915 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2685,9 +2685,6 @@ void pci_uevent_ers(struct pci_dev *pdev, enum  pci_ers_result err_type);
> >  
> >  #include <linux/dma-mapping.h>
> >  
> > -#define pci_printk(level, pdev, fmt, arg...) \
> > -	dev_printk(level, &(pdev)->dev, fmt, ##arg)

Both shpchp and aer do use pci_printk() before this series (it seems LKP 
has also catched it already).

If you split this series into different branches, this removal of 
pci_printk() has to be postponed until the next kernel release (fine for 
me if that's what you want to do, just remove this part from this patch 
and perhaps adjust the commit message to say it's to prepare for removal 
of the pci_printk()).

> >  #define pci_emerg(pdev, fmt, arg...)	dev_emerg(&(pdev)->dev, fmt, ##arg)
> >  #define pci_alert(pdev, fmt, arg...)	dev_alert(&(pdev)->dev, fmt, ##arg)
> >  #define pci_crit(pdev, fmt, arg...)	dev_crit(&(pdev)->dev, fmt, ##arg)

-- 
 i.
Re: [PATCH 4/4] PCI: Descope pci_printk() to aer_printk()
Posted by Bjorn Helgaas 11 months, 4 weeks ago
On Fri, Feb 14, 2025 at 01:56:47PM +0200, Ilpo Järvinen wrote:
> On Thu, 13 Feb 2025, Bjorn Helgaas wrote:
> > On Mon, Dec 16, 2024 at 06:10:12PM +0200, Ilpo Järvinen wrote:
> > > include/linux/pci.h provides low-level pci_printk() interface that is
> > > only used by AER because it needs to print the same message with
> > > different levels depending on the error severity. No other PCI code
> > > uses that functionality and calls pci_<level>() logging functions
> > > directly with the appropriate level.
> > > 
> > > Descope pci_printk() into AER as aer_printk().
> > > 
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > 
> > I applied this patch by itself on pci/aer for v6.15.
> > 
> > We also have some work-in-progress on rate limiting errors, which
> > might conflict, but this is simple and shouldn't be hard to reconcile.

> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index db9b47ce3eef..02d23e795915 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -2685,9 +2685,6 @@ void pci_uevent_ers(struct pci_dev *pdev, enum  pci_ers_result err_type);
> > >  
> > >  #include <linux/dma-mapping.h>
> > >  
> > > -#define pci_printk(level, pdev, fmt, arg...) \
> > > -	dev_printk(level, &(pdev)->dev, fmt, ##arg)
> 
> Both shpchp and aer do use pci_printk() before this series (it seems LKP 
> has also catched it already).
> 
> If you split this series into different branches, this removal of 
> pci_printk() has to be postponed until the next kernel release (fine for 
> me if that's what you want to do, just remove this part from this patch 
> and perhaps adjust the commit message to say it's to prepare for removal 
> of the pci_printk()).

OK.  I dropped the pci_printk() removal for now.  I'm anticipating
more AER changes this cycle, so I'm trying to keep those isolated.

Bjorn