Re: [PATCH 2/5] PCI/AER: Add sysfs stats for AER capable devices

[Greg Kroah-Hartman]

On Tue, May 22, 2018 at 03:28:02PM -0700, Rajat Jain wrote:
> +#define aer_stats_aggregate_attr(field)                                        \
> +	static ssize_t                                                         \
> +	field##_show(struct device *dev, struct device_attribute *attr,        \
> +		     char *buf)                                                \
> +{                                                                              \
> +	struct pci_dev *pdev = to_pci_dev(dev);                                \
> +	return sprintf(buf, "0x%llx\n", pdev->aer_stats->field);               \
> +}                                                                              \

Use tabs at the end please, otherwise your trailing \ look horrid.

> +static DEVICE_ATTR_RO(field)
> +
> +aer_stats_aggregate_attr(dev_total_cor_errs);
> +aer_stats_aggregate_attr(dev_total_fatal_errs);
> +aer_stats_aggregate_attr(dev_total_nonfatal_errs);
> +
> +static struct attribute *aer_stats_attrs[] __ro_after_init = {
> +	&dev_attr_dev_total_cor_errs.attr,
> +	&dev_attr_dev_total_fatal_errs.attr,
> +	&dev_attr_dev_total_nonfatal_errs.attr,
> +	NULL
> +};
> +
> +static umode_t aer_stats_attrs_are_visible(struct kobject *kobj,
> +					   struct attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (!pdev->aer_stats)
> +		return 0;
> +
> +	return a->mode;
> +}
> +
> +const struct attribute_group aer_stats_attr_group = {
> +	.name  = "aer_stats",
> +	.attrs  = aer_stats_attrs,
> +	.is_visible = aer_stats_attrs_are_visible,
> +};
> +
> +void pci_dev_aer_stats_incr(struct pci_dev *pdev, struct aer_err_info *info)
> +{
> +	int status, i, max = -1;
> +	u64 *counter = NULL;
> +	struct aer_stats *aer_stats = pdev->aer_stats;
> +
> +	if (unlikely(!aer_stats))
> +		return;

Can you measure the speed difference with and without that unlikely()
macro?  If not, please don't use it.  Hint, the cpu and compiler are
always always better at this than we are...

thanks,

greg k-h