linux-kernel - Re: [PATCH v13 07/25] CXL/AER: Replace device_lock() in cxl_rch_handle_error

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251208180555.GA3300834@bhelgaas>
Date: Mon, 8 Dec 2025 12:05:55 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Terry Bowman <terry.bowman@....com>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
	alison.schofield@...el.com, dan.j.williams@...el.com,
	bhelgaas@...gle.com, shiju.jose@...wei.com, ming.li@...omail.com,
	Smita.KoralahalliChannabasappa@....com, rrichter@....com,
	dan.carpenter@...aro.org, PradeepVineshReddy.Kodamati@....com,
	lukas@...ner.de, Benjamin.Cheatham@....com,
	sathyanarayanan.kuppuswamy@...ux.intel.com,
	linux-cxl@...r.kernel.org, alucerop@....com, ira.weiny@...el.com,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v13 07/25] CXL/AER: Replace device_lock() in
 cxl_rch_handle_error_iter() with guard() lock

On Mon, Nov 03, 2025 at 06:09:43PM -0600, Terry Bowman wrote:
> cxl_rch_handle_error_iter() includes a call to device_lock() using a goto
> for multiple return paths. Improve readability and maintainability by
> using the guard() lock variant.

Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>

I would reorder this patch to be after you move the function to
aer_cxl_rch.c.  Then the subjects could be more like this, which would
be a better match for the history:

  PCI/AER: Move CXL RCH error handling to aer_cxl_rch.c
  PCI/AER: Use guard() in cxl_rch_handle_error_iter()

I expect CXL content in drivers/pci/ to be mostly incidental, so I
think prefixes like "PCI/AER" or "PCI/ERR" are probably appropriate,
and "CXL" can appear in the rest of the line if relevant.

> +++ b/drivers/pci/pcie/aer.c
> @@ -1187,12 +1187,11 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>  	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev))
>  		return 0;
>  
> -	/* Protect dev->driver */

The comment seems worth keeping.

> -	device_lock(&dev->dev);
> +	guard(device)(&dev->dev);
>  
>  	err_handler = dev->driver ? dev->driver->err_handler : NULL;
>  	if (!err_handler)
> -		goto out;
> +		return 0;
>  
>  	if (info->severity == AER_CORRECTABLE) {
>  		if (err_handler->cor_error_detected)
> @@ -1203,8 +1202,6 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>  		else if (info->severity == AER_FATAL)
>  			err_handler->error_detected(dev, pci_channel_io_frozen);
>  	}
> -out:
> -	device_unlock(&dev->dev);
>  	return 0;
>  }
>  
> -- 
> 2.34.1
>