lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fdd8a39f-de1c-77bb-f091-96c9817b3d8a@linux.intel.com>
Date: Tue, 20 May 2025 14:26:06 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Bjorn Helgaas <helgaas@...nel.org>
cc: linux-pci@...r.kernel.org, Jon Pan-Doh <pandoh@...gle.com>, 
    Karolina Stolarek <karolina.stolarek@...cle.com>, 
    Martin Petersen <martin.petersen@...cle.com>, 
    Ben Fuller <ben.fuller@...cle.com>, Drew Walton <drewwalton@...rosoft.com>, 
    Anil Agrawal <anilagrawal@...a.com>, Tony Luck <tony.luck@...el.com>, 
    Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@...ux.intel.com>, 
    Lukas Wunner <lukas@...ner.de>, 
    Jonathan Cameron <Jonathan.Cameron@...wei.com>, 
    Sargun Dhillon <sargun@...a.com>, "Paul E . McKenney" <paulmck@...nel.org>, 
    Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, 
    Oliver O'Halloran <oohall@...il.com>, Kai-Heng Feng <kaihengf@...dia.com>, 
    Keith Busch <kbusch@...nel.org>, Robert Richter <rrichter@....com>, 
    Terry Bowman <terry.bowman@....com>, Shiju Jose <shiju.jose@...wei.com>, 
    Dave Jiang <dave.jiang@...el.com>, LKML <linux-kernel@...r.kernel.org>, 
    linuxppc-dev@...ts.ozlabs.org, Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v6 11/16] PCI/AER: Check log level once and remember it

On Mon, 19 May 2025, Bjorn Helgaas wrote:

> From: Karolina Stolarek <karolina.stolarek@...cle.com>
> 
> When reporting an AER error, we check its type multiple times to determine
> the log level for each message. Do this check only in the top-level
> functions (aer_isr_one_error(), pci_print_aer()) and save the level in
> struct aer_err_info.
> 
> [bhelgaas: save log level in struct aer_err_info instead of passing it
> as a parameter]
> Link: https://lore.kernel.org/r/20250321015806.954866-2-pandoh@google.com
> Signed-off-by: Karolina Stolarek <karolina.stolarek@...cle.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
> ---
>  drivers/pci/pci.h      |  1 +
>  drivers/pci/pcie/aer.c | 21 ++++++++++-----------
>  drivers/pci/pcie/dpc.c |  1 +
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index b81e99cd4b62..705f9ef58acc 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -588,6 +588,7 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
>  struct aer_err_info {
>  	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
>  	int error_dev_num;
> +	const char *level;		/* printk level */

As a general direction, wouldn't it be better to start adding these 
comments in the kerneldoc compatible format (even if not yet enabling the 
kerneldoc with /**)?

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

-- 
 i.

>  
>  	unsigned int id:16;
>  
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 4683a99c7568..73b03a195b14 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -672,21 +672,18 @@ static void pci_rootport_aer_stats_incr(struct pci_dev *pdev,
>  	}
>  }
>  
> -static void __aer_print_error(struct pci_dev *dev,
> -			      struct aer_err_info *info)
> +static void __aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
>  	const char **strings;
>  	unsigned long status = info->status & ~info->mask;
> -	const char *level, *errmsg;
> +	const char *level = info->level;
> +	const char *errmsg;
>  	int i;
>  
> -	if (info->severity == AER_CORRECTABLE) {
> +	if (info->severity == AER_CORRECTABLE)
>  		strings = aer_correctable_error_string;
> -		level = KERN_WARNING;
> -	} else {
> +	else
>  		strings = aer_uncorrectable_error_string;
> -		level = KERN_ERR;
> -	}
>  
>  	for_each_set_bit(i, &status, 32) {
>  		errmsg = strings[i];
> @@ -714,7 +711,7 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
>  	int layer, agent;
>  	int id = pci_dev_id(dev);
> -	const char *level;
> +	const char *level = info->level;
>  
>  	pci_dev_aer_stats_incr(dev, info);
>  
> @@ -727,8 +724,6 @@ void aer_print_error(struct pci_dev *dev, struct aer_err_info *info)
>  	layer = AER_GET_LAYER_ERROR(info->severity, info->status);
>  	agent = AER_GET_AGENT(info->severity, info->status);
>  
> -	level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
> -
>  	aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
>  		   aer_error_severity_string[info->severity],
>  		   aer_error_layer[layer], aer_agent_string[agent]);
> @@ -774,9 +769,11 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>  	if (aer_severity == AER_CORRECTABLE) {
>  		status = aer->cor_status;
>  		mask = aer->cor_mask;
> +		info.level = KERN_WARNING;
>  	} else {
>  		status = aer->uncor_status;
>  		mask = aer->uncor_mask;
> +		info.level = KERN_ERR;
>  		tlp_header_valid = status & AER_LOG_TLP_MASKS;
>  	}
>  
> @@ -1297,6 +1294,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
>  		struct aer_err_info e_info = {
>  			.id = ERR_COR_ID(e_src->id),
>  			.severity = AER_CORRECTABLE,
> +			.level = KERN_WARNING,
>  			.multi_error_valid = multi ? 1 : 0,
>  		};
>  
> @@ -1312,6 +1310,7 @@ static void aer_isr_one_error(struct aer_rpc *rpc,
>  		struct aer_err_info e_info = {
>  			.id = ERR_UNCOR_ID(e_src->id),
>  			.severity = fatal ? AER_FATAL : AER_NONFATAL,
> +			.level = KERN_ERR,
>  			.multi_error_valid = multi ? 1 : 0,
>  		};
>  
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 315bf2bfd570..34af0ea45c0d 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -252,6 +252,7 @@ static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
>  	else
>  		info->severity = AER_NONFATAL;
>  
> +	info->level = KERN_WARNING;
>  	return 1;
>  }
>  
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ