[rfc] suppress excessive AER output

[rfc] suppress excessive AER output

[Dave Jones]

I have a machine that has developed some kind of problem with
its onboard ethernet.  It still boots, but spewed almost 1.5G of text
(2381585 instances of the warning below) before we realised what
was going on, and blacklisted the igb driver.

Is it worth logging every single error when we're flooding like this ?
It seems unlikely that we'll find useful information in amongst that much data
that wasn't already in the first 100 instances.

I picked 100 in the (untested) example patch below arbitarily, but the exact
value could be smaller, or slightly bigger..

could we do something like this maybe ?

	Dave

diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 3ea5173..4ec88c6 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -153,6 +153,17 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int id = ((dev->bus->number << 8) | dev->devfn);
 	char prefix[44];
+	static unsigned long aer_printk_limit = 0;
+
+	aer_printk_limit++;
+
+	if (aer_printk_limit > 100)
+		return;
+
+	if (aer_printk_limit == 100) {
+		printk(KERN_ERR "Reached limit of 100 AER errors. Further AER output suppressed.\n");
+		return;
+	}
 
 	snprintf(prefix, sizeof(prefix), "%s%s %s: ",
 		 (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR,

RE: [rfc] suppress excessive AER output

[Zhang, Yanmin]

Dave,

How about adding a new module parameter aer_printk_limit, so user space could reset it any time?

Yanmin

-----Original Message-----
From: Dave Jones [mailto:davej@redhat.com] 
Sent: Thursday, August 04, 2011 6:34 AM
To: Linux Kernel
Cc: Nguyen, Tom L; Zhang, Yanmin
Subject: [rfc] suppress excessive AER output

I have a machine that has developed some kind of problem with
its onboard ethernet.  It still boots, but spewed almost 1.5G of text
(2381585 instances of the warning below) before we realised what
was going on, and blacklisted the igb driver.

Is it worth logging every single error when we're flooding like this ?
It seems unlikely that we'll find useful information in amongst that much data
that wasn't already in the first 100 instances.

I picked 100 in the (untested) example patch below arbitarily, but the exact
value could be smaller, or slightly bigger..

could we do something like this maybe ?

	Dave

diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
index 3ea5173..4ec88c6 100644
--- a/drivers/pci/pcie/aer/aerdrv_errprint.c
+++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
@@ -153,6 +153,17 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	int id = ((dev->bus->number << 8) | dev->devfn);
 	char prefix[44];
+	static unsigned long aer_printk_limit = 0;
+
+	aer_printk_limit++;
+
+	if (aer_printk_limit > 100)
+		return;
+
+	if (aer_printk_limit == 100) {
+		printk(KERN_ERR "Reached limit of 100 AER errors. Further AER output suppressed.\n");
+		return;
+	}
 
 	snprintf(prefix, sizeof(prefix), "%s%s %s: ",
 		 (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR,

Re: [rfc] suppress excessive AER output

[huang ying]

On Thu, Aug 4, 2011 at 6:34 AM, Dave Jones <davej@redhat.com> wrote:
> I have a machine that has developed some kind of problem with
> its onboard ethernet.  It still boots, but spewed almost 1.5G of text
> (2381585 instances of the warning below) before we realised what
> was going on, and blacklisted the igb driver.
>
> Is it worth logging every single error when we're flooding like this ?
> It seems unlikely that we'll find useful information in amongst that much data
> that wasn't already in the first 100 instances.
>
> I picked 100 in the (untested) example patch below arbitarily, but the exact
> value could be smaller, or slightly bigger..
>
> could we do something like this maybe ?

Why not use __ratelimit to implement this feature?

Best Regards,
Huang Ying

Re: [rfc] suppress excessive AER output

[Dave Jones]

On Thu, Aug 04, 2011 at 01:54:19PM +0800, Zhang, Yanmin wrote:
 > Dave,
 > 
 > How about adding a new module parameter aer_printk_limit, so user space could reset it any time?

Sure. Though for the built-in case, how would that work ?

	Dave

Re: [rfc] suppress excessive AER output

[Dave Jones]

On Thu, Aug 04, 2011 at 02:45:01PM +0800, huang ying wrote:
 > On Thu, Aug 4, 2011 at 6:34 AM, Dave Jones <davej@redhat.com> wrote:
 > > I have a machine that has developed some kind of problem with
 > > its onboard ethernet.  It still boots, but spewed almost 1.5G of text
 > > (2381585 instances of the warning below) before we realised what
 > > was going on, and blacklisted the igb driver.
 > >
 > > Is it worth logging every single error when we're flooding like this ?
 > > It seems unlikely that we'll find useful information in amongst that much data
 > > that wasn't already in the first 100 instances.
 > >
 > > I picked 100 in the (untested) example patch below arbitarily, but the exact
 > > value could be smaller, or slightly bigger..
 > >
 > > could we do something like this maybe ?
 > 
 > Why not use __ratelimit to implement this feature?

that would be better than the current situation probably, but my gut feeling is
that there's still going to be a lot of spew.

	Dave

RE: [rfc] suppress excessive AER output

[Zhang, Yanmin]

Dave,

My idea is application has opportunity to reset it. Consider the critical mission environment, admin could hot-unplug any failed devices without rebooting system. Then, admin doesn't want to lose AER monitoring.

With the module parameter, admin could change it under /sys/module/aer_drv/parameters.

Yanmin

-----Original Message-----
From: Dave Jones [mailto:davej@redhat.com] 
Sent: Friday, August 05, 2011 9:50 AM
To: Zhang, Yanmin
Cc: Linux Kernel; Nguyen, Tom L; Huang, Ying
Subject: Re: [rfc] suppress excessive AER output

On Thu, Aug 04, 2011 at 01:54:19PM +0800, Zhang, Yanmin wrote:
 > Dave,
 > 
 > How about adding a new module parameter aer_printk_limit, so user space could reset it any time?

Sure. Though for the built-in case, how would that work ?

	Dave

Re: [rfc] suppress excessive AER output

[Dave Jones]

On Fri, Aug 05, 2011 at 09:59:39AM +0800, Zhang, Yanmin wrote:
 > Dave,
 > 
 > My idea is application has opportunity to reset it. Consider the critical mission environment, admin could hot-unplug any failed devices without rebooting system. Then, admin doesn't want to lose AER monitoring.
 > 
 > With the module parameter, admin could change it under /sys/module/aer_drv/parameters.
 
ah, I see. Yes, that sounds sensible.

	Dave.
 

Re: [rfc] suppress excessive AER output

[Arnaud Lacombe]

Hi,

On Wed, Aug 3, 2011 at 6:34 PM, Dave Jones <davej@redhat.com> wrote:
> I have a machine that has developed some kind of problem with
> its onboard ethernet.  It still boots, but spewed almost 1.5G of text
> (2381585 instances of the warning below) before we realised what
> was going on, and blacklisted the igb driver.
>
> Is it worth logging every single error when we're flooding like this ?
> It seems unlikely that we'll find useful information in amongst that much data
> that wasn't already in the first 100 instances.
>
> I picked 100 in the (untested) example patch below arbitarily, but the exact
> value could be smaller, or slightly bigger..
>
> could we do something like this maybe ?
>
Please do not reinvent the wheel and use printk_ratelimited().

Thanks,
 - Arnaud

>        Dave
>
> diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c
> index 3ea5173..4ec88c6 100644
> --- a/drivers/pci/pcie/aer/aerdrv_errprint.c
> +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c
> @@ -153,6 +153,17 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
>        int id = ((dev->bus->number << 8) | dev->devfn);
>        char prefix[44];
> +       static unsigned long aer_printk_limit = 0;
> +
> +       aer_printk_limit++;
> +
> +       if (aer_printk_limit > 100)
> +               return;
> +
> +       if (aer_printk_limit == 100) {
> +               printk(KERN_ERR "Reached limit of 100 AER errors. Further AER output suppressed.\n");
> +               return;
> +       }
>
>        snprintf(prefix, sizeof(prefix), "%s%s %s: ",
>                 (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [rfc] suppress excessive AER output

[Dave Jones]

On Thu, Aug 04, 2011 at 10:24:07PM -0400, Arnaud Lacombe wrote:
 > Hi,
 > 
 > On Wed, Aug 3, 2011 at 6:34 PM, Dave Jones <davej@redhat.com> wrote:
 > > I have a machine that has developed some kind of problem with
 > > its onboard ethernet.  It still boots, but spewed almost 1.5G of text
 > > (2381585 instances of the warning below) before we realised what
 > > was going on, and blacklisted the igb driver.
 > >
 > > Is it worth logging every single error when we're flooding like this ?
 > > It seems unlikely that we'll find useful information in amongst that much data
 > > that wasn't already in the first 100 instances.
 > >
 > > I picked 100 in the (untested) example patch below arbitarily, but the exact
 > > value could be smaller, or slightly bigger..
 > >
 > > could we do something like this maybe ?
 > >
 > Please do not reinvent the wheel and use printk_ratelimited().

It's a different wheel.

printk_ratelimit slows down the output, but would still cause a lot of messages.

my diff turns it off completely after a threshold (apart from at the ulong wrap,
which I overlooked).

	Dave