There are quite a few places in the s390 architecture code for the PCI
subsystem where the kernel needs to lock `pci_rescan_remove_lock` now;
which is done by calling pci_lock_rescan_remove() to lock, and
pci_unlock_rescan_remove() to unlock the mutex.

Instead of always manually calling both functions, which induces a
certain amount of visual clutter, and in some cases of errors, cleanup,
and jumplabels more complexity, use either guard() or scoped_guard()
depending on the context.

Convert all users in the s390 architecture code for PCI.

Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
---
 arch/s390/pci/pci.c       |  8 +++----
 arch/s390/pci/pci_bus.c   |  3 +--
 arch/s390/pci/pci_event.c | 45 +++++++++++++++++----------------------
 arch/s390/pci/pci_iov.c   |  3 +--
 arch/s390/pci/pci_sysfs.c |  9 +++-----
 5 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index fd16e6ad21c1..86ef1e516857 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -632,10 +632,9 @@ void pcibios_release_device(struct pci_dev *pdev)
 {
 	struct zpci_dev *zdev = to_zpci(pdev);
 
-	pci_lock_rescan_remove();
+	guard(pci_rescan_remove)();
 	zpci_unmap_resources(pdev);
 	zpci_zdev_put(zdev);
-	pci_unlock_rescan_remove();
 }
 
 int pcibios_enable_device(struct pci_dev *pdev, int mask)
@@ -1213,9 +1212,8 @@ static int __init pci_base_init(void)
 	if (rc)
 		goto out_irq;
 
-	pci_lock_rescan_remove();
-	rc = zpci_scan_devices();
-	pci_unlock_rescan_remove();
+	scoped_guard(pci_rescan_remove)
+		rc = zpci_scan_devices();
 	if (rc)
 		goto out_find;
 
diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
index 2b598222c621..c1b48b572e86 100644
--- a/arch/s390/pci/pci_bus.c
+++ b/arch/s390/pci/pci_bus.c
@@ -82,9 +82,8 @@ int zpci_bus_scan_device(struct zpci_dev *zdev)
 	if (!pdev)
 		return -ENODEV;
 
-	pci_lock_rescan_remove();
+	guard(pci_rescan_remove)();
 	pci_bus_add_device(pdev);
-	pci_unlock_rescan_remove();
 
 	return 0;
 }
diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index edfaeed737ac..98253706b591 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -342,9 +342,8 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
 no_pdev:
 	if (zdev)
 		mutex_unlock(&zdev->state_lock);
-	pci_lock_rescan_remove();
+	guard(pci_rescan_remove)();
 	zpci_zdev_put(zdev);
-	pci_unlock_rescan_remove();
 }
 
 void zpci_event_error(void *data)
@@ -389,7 +388,6 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 	struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
 	bool existing_zdev = !!zdev;
 	enum zpci_state state;
-	int rc;
 
 	zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
 		 ccdf->fid, ccdf->fh, ccdf->pec);
@@ -403,12 +401,11 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
 			if (IS_ERR(zdev))
 				break;
-			pci_lock_rescan_remove();
-			rc = zpci_add_device(zdev);
-			pci_unlock_rescan_remove();
-			if (rc) {
-				kfree(zdev);
-				break;
+			scoped_guard(pci_rescan_remove) {
+				if (zpci_add_device(zdev)) {
+					kfree(zdev);
+					break;
+				}
 			}
 		} else {
 			if (zdev->state == ZPCI_FN_STATE_RESERVED)
@@ -425,12 +422,11 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
 			if (IS_ERR(zdev))
 				break;
-			pci_lock_rescan_remove();
-			rc = zpci_add_device(zdev);
-			pci_unlock_rescan_remove();
-			if (rc) {
-				kfree(zdev);
-				break;
+			scoped_guard(pci_rescan_remove) {
+				if (zpci_add_device(zdev)) {
+					kfree(zdev);
+					break;
+				}
 			}
 		} else {
 			if (zdev->state == ZPCI_FN_STATE_RESERVED)
@@ -459,33 +455,30 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
 			/* The 0x0304 event may immediately reserve the device */
 			if (!clp_get_state(zdev->fid, &state) &&
 			    state == ZPCI_FN_STATE_RESERVED) {
-				pci_lock_rescan_remove();
+				guard(pci_rescan_remove)();
 				zpci_device_reserved(zdev);
-				pci_unlock_rescan_remove();
 			}
 		}
 		break;
 	case 0x0306: /* 0x308 or 0x302 for multiple devices */
-		pci_lock_rescan_remove();
-		zpci_remove_reserved_devices();
-		zpci_scan_devices();
-		pci_unlock_rescan_remove();
+		scoped_guard(pci_rescan_remove) {
+			zpci_remove_reserved_devices();
+			zpci_scan_devices();
+		}
 		break;
 	case 0x0308: /* Standby -> Reserved */
 		if (!zdev)
 			break;
-		pci_lock_rescan_remove();
-		zpci_device_reserved(zdev);
-		pci_unlock_rescan_remove();
+		scoped_guard(pci_rescan_remove)
+			zpci_device_reserved(zdev);
 		break;
 	default:
 		break;
 	}
 	if (existing_zdev) {
 		mutex_unlock(&zdev->state_lock);
-		pci_lock_rescan_remove();
+		guard(pci_rescan_remove)();
 		zpci_zdev_put(zdev);
-		pci_unlock_rescan_remove();
 	}
 }
 
diff --git a/arch/s390/pci/pci_iov.c b/arch/s390/pci/pci_iov.c
index 13050ce5c3e9..1f7e4dd018e7 100644
--- a/arch/s390/pci/pci_iov.c
+++ b/arch/s390/pci/pci_iov.c
@@ -38,10 +38,9 @@ void zpci_iov_map_resources(struct pci_dev *pdev)
 
 void zpci_iov_remove_virtfn(struct pci_dev *pdev, int vfn)
 {
-	pci_lock_rescan_remove();
+	guard(pci_rescan_remove)();
 	/* Linux' vfid's start at 0 vfn at 1 */
 	pci_iov_remove_virtfn(pdev->physfn, vfn - 1);
-	pci_unlock_rescan_remove();
 }
 
 static int zpci_iov_link_virtfn(struct pci_dev *pdev, struct pci_dev *virtfn, int vfid)
diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
index c2444a23e26c..f5027aa95928 100644
--- a/arch/s390/pci/pci_sysfs.c
+++ b/arch/s390/pci/pci_sysfs.c
@@ -98,9 +98,9 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr,
 	WARN_ON_ONCE(!kn);
 
 	/* Device needs to be configured and state must not change */
-	mutex_lock(&zdev->state_lock);
+	guard(mutex)(&zdev->state_lock);
 	if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
-		goto out;
+		return count;
 
 	/* device_remove_file() serializes concurrent calls ignoring all but
 	 * the first
@@ -112,15 +112,12 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr,
 	 * Once it unblocks from pci_lock_rescan_remove() the original pdev
 	 * will already be removed.
 	 */
-	pci_lock_rescan_remove();
+	guard(pci_rescan_remove)();
 	if (pci_dev_is_added(pdev)) {
 		ret = _do_recover(pdev, zdev);
 	}
 	pci_rescan_bus(zdev->zbus->bus);
-	pci_unlock_rescan_remove();
 
-out:
-	mutex_unlock(&zdev->state_lock);
 	if (kn)
 		sysfs_unbreak_active_protection(kn);
 	return ret ? ret : count;
-- 
2.53.0

On Fri, 6 Mar 2026, Benjamin Block wrote:

> There are quite a few places in the s390 architecture code for the PCI
> subsystem where the kernel needs to lock `pci_rescan_remove_lock` now;
> which is done by calling pci_lock_rescan_remove() to lock, and
> pci_unlock_rescan_remove() to unlock the mutex.
> 
> Instead of always manually calling both functions, which induces a
> certain amount of visual clutter, and in some cases of errors, cleanup,
> and jumplabels more complexity, use either guard() or scoped_guard()
> depending on the context.
> 
> Convert all users in the s390 architecture code for PCI.
> 
> Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
> ---
>  arch/s390/pci/pci.c       |  8 +++----
>  arch/s390/pci/pci_bus.c   |  3 +--
>  arch/s390/pci/pci_event.c | 45 +++++++++++++++++----------------------
>  arch/s390/pci/pci_iov.c   |  3 +--
>  arch/s390/pci/pci_sysfs.c |  9 +++-----
>  5 files changed, 27 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index fd16e6ad21c1..86ef1e516857 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -632,10 +632,9 @@ void pcibios_release_device(struct pci_dev *pdev)
>  {
>  	struct zpci_dev *zdev = to_zpci(pdev);
>  
> -	pci_lock_rescan_remove();
> +	guard(pci_rescan_remove)();
>  	zpci_unmap_resources(pdev);
>  	zpci_zdev_put(zdev);
> -	pci_unlock_rescan_remove();
>  }
>  
>  int pcibios_enable_device(struct pci_dev *pdev, int mask)
> @@ -1213,9 +1212,8 @@ static int __init pci_base_init(void)
>  	if (rc)
>  		goto out_irq;
>  
> -	pci_lock_rescan_remove();
> -	rc = zpci_scan_devices();
> -	pci_unlock_rescan_remove();
> +	scoped_guard(pci_rescan_remove)
> +		rc = zpci_scan_devices();
>  	if (rc)
>  		goto out_find;
>  
> diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c
> index 2b598222c621..c1b48b572e86 100644
> --- a/arch/s390/pci/pci_bus.c
> +++ b/arch/s390/pci/pci_bus.c
> @@ -82,9 +82,8 @@ int zpci_bus_scan_device(struct zpci_dev *zdev)
>  	if (!pdev)
>  		return -ENODEV;
>  
> -	pci_lock_rescan_remove();
> +	guard(pci_rescan_remove)();
>  	pci_bus_add_device(pdev);
> -	pci_unlock_rescan_remove();
>  
>  	return 0;
>  }
> diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
> index edfaeed737ac..98253706b591 100644
> --- a/arch/s390/pci/pci_event.c
> +++ b/arch/s390/pci/pci_event.c
> @@ -342,9 +342,8 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
>  no_pdev:
>  	if (zdev)
>  		mutex_unlock(&zdev->state_lock);
> -	pci_lock_rescan_remove();
> +	guard(pci_rescan_remove)();
>  	zpci_zdev_put(zdev);
> -	pci_unlock_rescan_remove();
>  }
>  
>  void zpci_event_error(void *data)
> @@ -389,7 +388,6 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  	struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
>  	bool existing_zdev = !!zdev;
>  	enum zpci_state state;
> -	int rc;
>  
>  	zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
>  		 ccdf->fid, ccdf->fh, ccdf->pec);
> @@ -403,12 +401,11 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
>  			if (IS_ERR(zdev))
>  				break;
> -			pci_lock_rescan_remove();
> -			rc = zpci_add_device(zdev);
> -			pci_unlock_rescan_remove();
> -			if (rc) {
> -				kfree(zdev);
> -				break;
> +			scoped_guard(pci_rescan_remove) {
> +				if (zpci_add_device(zdev)) {
> +					kfree(zdev);
> +					break;
> +				}
>  			}
>  		} else {
>  			if (zdev->state == ZPCI_FN_STATE_RESERVED)
> @@ -425,12 +422,11 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  			zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
>  			if (IS_ERR(zdev))
>  				break;
> -			pci_lock_rescan_remove();
> -			rc = zpci_add_device(zdev);
> -			pci_unlock_rescan_remove();
> -			if (rc) {
> -				kfree(zdev);
> -				break;
> +			scoped_guard(pci_rescan_remove) {
> +				if (zpci_add_device(zdev)) {
> +					kfree(zdev);
> +					break;
> +				}
>  			}
>  		} else {
>  			if (zdev->state == ZPCI_FN_STATE_RESERVED)
> @@ -459,33 +455,30 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
>  			/* The 0x0304 event may immediately reserve the device */
>  			if (!clp_get_state(zdev->fid, &state) &&
>  			    state == ZPCI_FN_STATE_RESERVED) {
> -				pci_lock_rescan_remove();
> +				guard(pci_rescan_remove)();
>  				zpci_device_reserved(zdev);
> -				pci_unlock_rescan_remove();
>  			}
>  		}
>  		break;
>  	case 0x0306: /* 0x308 or 0x302 for multiple devices */
> -		pci_lock_rescan_remove();
> -		zpci_remove_reserved_devices();
> -		zpci_scan_devices();
> -		pci_unlock_rescan_remove();
> +		scoped_guard(pci_rescan_remove) {
> +			zpci_remove_reserved_devices();
> +			zpci_scan_devices();
> +		}
>  		break;
>  	case 0x0308: /* Standby -> Reserved */
>  		if (!zdev)
>  			break;
> -		pci_lock_rescan_remove();
> -		zpci_device_reserved(zdev);
> -		pci_unlock_rescan_remove();
> +		scoped_guard(pci_rescan_remove)
> +			zpci_device_reserved(zdev);

Order in this series is weird. Why not introduce *guard() support before 
the fix (reorder patches 2 and 3) and then use guard direct here so you 
don't have to immediately change the code again to "convert" it to use 
*guard() in patch 4?

-- 
 i.

>  		break;
>  	default:
>  		break;
>  	}
>  	if (existing_zdev) {
>  		mutex_unlock(&zdev->state_lock);
> -		pci_lock_rescan_remove();
> +		guard(pci_rescan_remove)();
>  		zpci_zdev_put(zdev);
> -		pci_unlock_rescan_remove();
>  	}
>  }
>  
> diff --git a/arch/s390/pci/pci_iov.c b/arch/s390/pci/pci_iov.c
> index 13050ce5c3e9..1f7e4dd018e7 100644
> --- a/arch/s390/pci/pci_iov.c
> +++ b/arch/s390/pci/pci_iov.c
> @@ -38,10 +38,9 @@ void zpci_iov_map_resources(struct pci_dev *pdev)
>  
>  void zpci_iov_remove_virtfn(struct pci_dev *pdev, int vfn)
>  {
> -	pci_lock_rescan_remove();
> +	guard(pci_rescan_remove)();
>  	/* Linux' vfid's start at 0 vfn at 1 */
>  	pci_iov_remove_virtfn(pdev->physfn, vfn - 1);
> -	pci_unlock_rescan_remove();
>  }
>  
>  static int zpci_iov_link_virtfn(struct pci_dev *pdev, struct pci_dev *virtfn, int vfid)
> diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
> index c2444a23e26c..f5027aa95928 100644
> --- a/arch/s390/pci/pci_sysfs.c
> +++ b/arch/s390/pci/pci_sysfs.c
> @@ -98,9 +98,9 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr,
>  	WARN_ON_ONCE(!kn);
>  
>  	/* Device needs to be configured and state must not change */
> -	mutex_lock(&zdev->state_lock);
> +	guard(mutex)(&zdev->state_lock);
>  	if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
> -		goto out;
> +		return count;
>  
>  	/* device_remove_file() serializes concurrent calls ignoring all but
>  	 * the first
> @@ -112,15 +112,12 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr,
>  	 * Once it unblocks from pci_lock_rescan_remove() the original pdev
>  	 * will already be removed.
>  	 */
> -	pci_lock_rescan_remove();
> +	guard(pci_rescan_remove)();
>  	if (pci_dev_is_added(pdev)) {
>  		ret = _do_recover(pdev, zdev);
>  	}
>  	pci_rescan_bus(zdev->zbus->bus);
> -	pci_unlock_rescan_remove();
>  
> -out:
> -	mutex_unlock(&zdev->state_lock);
>  	if (kn)
>  		sysfs_unbreak_active_protection(kn);
>  	return ret ? ret : count;
>

On Mon, Mar 09, 2026 at 09:38:45AM +0200, Ilpo Järvinen wrote:
> On Fri, 6 Mar 2026, Benjamin Block wrote:
> > -		pci_lock_rescan_remove();
> > -		zpci_device_reserved(zdev);
> > -		pci_unlock_rescan_remove();
> > +		scoped_guard(pci_rescan_remove)
> > +			zpci_device_reserved(zdev);
> 
> Order in this series is weird. Why not introduce *guard() support before 
> the fix (reorder patches 2 and 3) and then use guard direct here so you 
> don't have to immediately change the code again to "convert" it to use 
> *guard() in patch 4?

The main intention here was to make it easier on me (and others) to retrofit
this in stable and/or distribution Kernels. I know this isn't a main concern
for the upstream master, but it would make my life a bit easier :)

Although, ofc., it isn't required, so in doubt, I can change it. Given that
the Maintainers are actually OK with the use of guards, in any case.

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Ehningen     /     Registergericht: AmtsG Stuttgart, HRB 243294

On Mon, 2026-03-09 at 11:54 +0100, Benjamin Block wrote:
> On Mon, Mar 09, 2026 at 09:38:45AM +0200, Ilpo Järvinen wrote:
> > On Fri, 6 Mar 2026, Benjamin Block wrote:
> > > -		pci_lock_rescan_remove();
> > > -		zpci_device_reserved(zdev);
> > > -		pci_unlock_rescan_remove();
> > > +		scoped_guard(pci_rescan_remove)
> > > +			zpci_device_reserved(zdev);
> > 
> > Order in this series is weird. Why not introduce *guard() support before 
> > the fix (reorder patches 2 and 3) and then use guard direct here so you 
> > don't have to immediately change the code again to "convert" it to use 
> > *guard() in patch 4?
> 
> The main intention here was to make it easier on me (and others) to retrofit
> this in stable and/or distribution Kernels. I know this isn't a main concern
> for the upstream master, but it would make my life a bit easier :)
> 
> Although, ofc., it isn't required, so in doubt, I can change it. Given that
> the Maintainers are actually OK with the use of guards, in any case.

I do like the guards but thinking about it, I also do have a slight
preference for adding the guard support first and using it directly.
I'm not entirely convinced this makes it harder to backport either.
From a quick check it looks like all relevant distros have the macro
and we do try to keep the code close to upstream so would just end up
backporting the lock guard conversion too maybe immediately maybe
later.

Thanks,
Niklas