2017-12-13 16:51:05

by Tyler Baicar

[permalink] [raw]
Subject: Re: [PATCH] PCI/AER: update AER status string print to match other AER logs

On 11/15/2017 12:56 PM, Bjorn Helgaas wrote:
> Hi Tyler,
>
> On Wed, Nov 15, 2017 at 09:47:41AM -0500, Tyler Baicar wrote:
>> On 10/17/2017 11:42 AM, Tyler Baicar wrote:
>>> Currently the AER driver uses cper_print_bits() to print the AER status
>>> string. This causes the status string to not include the proper PCI device
>>> name prefix that the other AER prints include. Also, it has a different
>>> print level than all the other AER prints.
>>>
>>> Update the AER driver to print the AER status string with the proper string
>>> prefix and proper print level.
>> Hello,
>>
>> Will this patch be pulled into 4.15?
> Sorry, I am preparing the 4.15 pull request right now, and it doesn't
> include this change.
>
> I do like the dev_err() change, but would prefer fewer lines of
> output. I could have applied just the dev_err() change, but to
> minimize pain for people who parse the logs, I'd rather make one
> change in the output instead of making one change now and another
> later.
Hello Bjorn,

Are there existing abbreviations for these AER status strings that I cannot
find? Or do you want
me to abbreviate them similar to the style used with prints in lspci -vv?

Once they are abbreviated, you'd prefer to have all errors that have occurred to
be printed on
the same line, correct?

Thanks,
Tyler

--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.


2017-12-13 19:25:05

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] PCI/AER: update AER status string print to match other AER logs

On Wed, Dec 13, 2017 at 11:50:56AM -0500, Tyler Baicar wrote:
> On 11/15/2017 12:56 PM, Bjorn Helgaas wrote:
> >Hi Tyler,
> >
> >On Wed, Nov 15, 2017 at 09:47:41AM -0500, Tyler Baicar wrote:
> >>On 10/17/2017 11:42 AM, Tyler Baicar wrote:
> >>>Currently the AER driver uses cper_print_bits() to print the AER status
> >>>string. This causes the status string to not include the proper PCI device
> >>>name prefix that the other AER prints include. Also, it has a different
> >>>print level than all the other AER prints.
> >>>
> >>>Update the AER driver to print the AER status string with the proper string
> >>>prefix and proper print level.
> >>Hello,
> >>
> >>Will this patch be pulled into 4.15?
> >Sorry, I am preparing the 4.15 pull request right now, and it doesn't
> >include this change.
> >
> >I do like the dev_err() change, but would prefer fewer lines of
> >output. I could have applied just the dev_err() change, but to
> >minimize pain for people who parse the logs, I'd rather make one
> >change in the output instead of making one change now and another
> >later.
> Hello Bjorn,
>
> Are there existing abbreviations for these AER status strings that I
> cannot find? Or do you want
> me to abbreviate them similar to the style used with prints in lspci -vv?

I think the terms used by lspci -vv would be a good start.

> Once they are abbreviated, you'd prefer to have all errors that have
> occurred to be printed on
> the same line, correct?

Yes. Multiple lines suggests an ordering that really isn't there, so
if we can print them all at once, it both improves atomicity and
removes the erroneous suggestion that "this error occurred before this
other one".

Bjorn