All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Rajat Jain <rajatja@google.com>
Cc: gregkh@linuxfoundation.com, Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	rajatxjain@gmail.com
Subject: Re: [PATCH 2/2] PCI/AER: Split the AER stats into multiple sysfs attributes
Date: Fri, 23 Aug 2019 10:45:34 -0700	[thread overview]
Message-ID: <20190823174534.GC8052@kroah.com> (raw)
In-Reply-To: <20190821231513.36454-2-rajatja@google.com>

On Wed, Aug 21, 2019 at 04:15:13PM -0700, Rajat Jain wrote:
> Split the AER stats into multiple sysfs atributes. Note that
> this changes the ABI of the AER stats, but hopefully, there
> aren't active users that need to change. This is how the AERs
> are being exposed now:
> 
> localhost /sys/devices/pci0000:00/0000:00:1c.0/aer_stats # ls -l
> total 0
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit0_RxErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit12_Timeout
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit13_NonFatalErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit14_CorrIntErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit15_HeaderOF
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit6_BadTLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit7_BadDLLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 correctable_bit8_Rollover
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit0_Undefined
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit12_TLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit13_FCP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit14_CmpltTO
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit15_CmpltAbrt
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit16_UnxCmplt
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit17_RxOF
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit18_MalfTLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit19_ECRC
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit20_UnsupReq
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit21_ACSViol
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit22_UncorrIntErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit23_BlockedTLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit24_AtomicOpBlocked
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit25_TLPBlockedErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit26_PoisonTLPBlocked
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit4_DLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 fatal_bit5_SDES
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit0_Undefined
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit12_TLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit13_FCP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit14_CmpltTO
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit15_CmpltAbrt
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit16_UnxCmplt
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit17_RxOF
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit18_MalfTLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit19_ECRC
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit20_UnsupReq
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit21_ACSViol
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit22_UncorrIntErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit23_BlockedTLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit24_AtomicOpBlocked
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit25_TLPBlockedErr
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit26_PoisonTLPBlocked
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit4_DLP
> -r--r--r--. 1 root root 4096 Aug 20 16:35 nonfatal_bit5_SDES
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_device_err_cor
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_device_err_fatal
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_device_err_nonfatal
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_rootport_err_cor
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_rootport_err_fatal
> -r--r--r--. 1 root root 4096 Aug 20 16:35 total_rootport_err_nonfatal
> localhost /sys/devices/pci0000:00/0000:00:1c.0/aer_stats #
> 
> Each file is has a single counter value. Single file containing all
> stats was frowned upon and discussed here:
> https://lkml.org/lkml/2019/6/28/220
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>
> ---
> I personally think that this makes it a little overwhelming for a human,
> e.g. I could look at total but don't exactly know while file to look at
> next in order to drill down. But I couldn't think of any other way. Some
> problems I'd have liked to fix but they require deeper surgery:
> 
> * Now each PCI device sysfs node will have a sub-directory called aer_stats.
>   (The subdirectory will have attributes only if it supports AER, but
>    the sub directory will always be present). 
> 
> * This patch isn't re-using the strings array like it was using earlier.
>   I thought of adding the attribute group at run time, so it will take
>   care of both the problems, but can only do that after device_add() call,
>   I think.
> 
> If we are comfortable introducing a call to a new function
> pci_aer_stats_init() after call to device_add() in in pci_device_add(),
> the above problems can be fixed.
> 
>  drivers/pci/pcie/aer.c | 166 +++++++++++++++++++++++++++++------------
>  1 file changed, 119 insertions(+), 47 deletions(-)

You need a Documentation/ABI/ update for the new sysfs files before we
can properly review this to see if you are doing what you think you are
doing :)

thanks,

greg k-h

  reply	other threads:[~2019-08-23 17:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21 23:15 [PATCH 1/2] PCI/AER: Add PoisonTLPBlocked to Uncorrectable errors Rajat Jain
2019-08-21 23:15 ` [PATCH 2/2] PCI/AER: Split the AER stats into multiple sysfs attributes Rajat Jain
2019-08-23 17:45   ` Greg KH [this message]
2019-08-27  0:51   ` [PATCH v2 1/2] PCI/AER: Add PoisonTLPBlocked to Uncorrectable errors Rajat Jain
2019-08-27  0:51     ` [PATCH v2 2/2] PCI/AER: Split the AER stats into multiple sysfs attributes Rajat Jain
2019-08-27  6:23       ` Greg KH
2019-08-27 22:21         ` [PATCH v3 1/2] PCI/AER: Add PoisonTLPBlocked to Uncorrectable errors Rajat Jain
2019-08-27 22:21           ` [PATCH v3 2/2] PCI/AER: Split the AER stats into multiple sysfs attributes Rajat Jain
2019-08-28  9:30             ` Greg KH
2019-08-29 23:18             ` Bjorn Helgaas
2019-08-29 23:27           ` [PATCH v3 1/2] PCI/AER: Add PoisonTLPBlocked to Uncorrectable errors Rajat Jain
2019-10-15 22:24           ` Bjorn Helgaas

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=20190823174534.GC8052@kroah.com \
    --to=greg@kroah.com \
    --cc=bhelgaas@google.com \
    --cc=gregkh@linuxfoundation.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rajatja@google.com \
    --cc=rajatxjain@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.