linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mohamed Khalfella <mkhalfella@purestorage.com>
To: helgaas@kernel.org
Cc: bhelgaas@google.com, ebadger@purestorage.com,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, mkhalfella@purestorage.com,
	msaggi@purestorage.com, oohall@gmail.com, rajatja@google.com,
	ruscur@russell.cc, stable@vger.kernel.org
Subject: Re: [PATCH] PCI/AER: Iterate over error counters instead of error
Date: Tue, 10 May 2022 21:17:56 +0000	[thread overview]
Message-ID: <20220510211756.5237-1-mkhalfella@purestorage.com> (raw)
In-Reply-To: <20220510164305.GA678149@bhelgaas>

> Thanks for catching this; it definitely looks like a real issue!  I
> guess you're probably seeing junk in the sysfs files?

That is correct. The initial report was seeing junk when reading sysfs
files. As descibed, this is happening because we reading data past the
end of the stats counters array.


> I think maybe we should populate the currently NULL entries in the
> string[] arrays and simplify the code here, e.g.,
> 
> static const char *aer_correctable_error_string[] = {
>        "RxErr",                        /* Bit Position 0       */
>        "dev_cor_errs_bit[1]",
>	...
>
>  if (stats[i])
>    len += sysfs_emit_at(buf, len, "%s %llu\n", strings_array[i], stats[i]);

Doing it this way will change the output format. In this case we will show
stats only if their value is greater than zero. The current code shows all the
stats those have names (regardless of their value) plus those have non-zero
values.

>> @@ -1342,6 +1342,11 @@ static int aer_probe(struct pcie_device *dev)
>>  	struct device *device = &dev->device;
>>  	struct pci_dev *port = dev->port;
>>
>> +	BUILD_BUG_ON(ARRAY_SIZE(aer_correctable_error_string) <
>> +		     AER_MAX_TYPEOF_COR_ERRS);
>> +	BUILD_BUG_ON(ARRAY_SIZE(aer_uncorrectable_error_string) <
>> +		     AER_MAX_TYPEOF_UNCOR_ERRS);
>
> And make these check for "!=" instead of "<".

This will require unnecessarily extending stats arrays to have 32 entries
in order to match names arrays. If you don't feel strogly about changing
"<" to "!=", I prefer to keep the code as it is. 

  reply	other threads:[~2022-05-10 21:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 18:14 [PATCH] PCI/AER: Iterate over error counters instead of error strings Mohamed Khalfella
2022-05-10 16:43 ` Bjorn Helgaas
2022-05-10 21:17   ` Mohamed Khalfella [this message]
2022-06-03 22:12     ` [PATCH] PCI/AER: Iterate over error counters instead of error Mohamed Khalfella
2022-06-03 23:58       ` Bjorn Helgaas
2022-06-04 20:30         ` Mohamed Khalfella
2022-07-11 22:54 ` [PATCH] PCI/AER: Iterate over error counters instead of error strings Bjorn Helgaas
2022-07-11 23:02   ` Mohamed Khalfella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220510211756.5237-1-mkhalfella@purestorage.com \
    --to=mkhalfella@purestorage.com \
    --cc=bhelgaas@google.com \
    --cc=ebadger@purestorage.com \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=msaggi@purestorage.com \
    --cc=oohall@gmail.com \
    --cc=rajatja@google.com \
    --cc=ruscur@russell.cc \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).