* [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS
@ 2026-08-03 14:29 Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Heiko Carstens @ 2026-08-03 14:29 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Niklas Schnelle, Gerd Bayer
Cc: linux-s390, linux-kernel
v2:
- Address potential zdev reference leak (false positive) by adding a
comment, and moving get_zdev_by_fid() [Sashiko, Niklas]
- Note: this reveals a pre-existing bug: zpci_remove_reserved_devices()
calls zpci_device_reserved() without holding locks. It looks like
this can be fixed easily by adding a mutex_lock()/mutex_unlock()
pair in zpci_remove_reserved_devices(), but that is not within
the scope of this series, and up to the pci maintainers
v1:
Enable CONTEXT_ANALYSYS for s390's pci code.
Static code checking for acquiring and releasing locks used to be done
with sparse. That was removed with [1] and replaced with a clang based
approach [2]. The new approach requires that each subsystem needs to be
explicitly enabled for checking.
Do that for s390's pci code. In order to avoid false positives the code
has to be slightly reworked, since conditionally acquiring and releasing
locks does not work with the checker (besides that this is sub optimal
coding style).
[1] 5b63d0ae94cc ("compiler-context-analysis: Remove Sparse support")
[2] 3269701cb256 ("compiler-context-analysis: Add infrastructure for Context Analysis with Clang")
Notes:
- More patches for other s390 specific subsystems will follow
- "Pre-existing issues" reported by AI will not be addressed in the
context of this series
Heiko Carstens (3):
s390/pci: Rework __zpci_event_error() to remove conditional locking
s390/pci: Rework__zpci_event_availability() to remove conditional locking
s390/pci: Enable CONTEXT_ANALYSIS
arch/s390/pci/Makefile | 2 +
arch/s390/pci/pci_event.c | 151 +++++++++++++++++++-------------------
2 files changed, 79 insertions(+), 74 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
@ 2026-08-03 14:29 ` Heiko Carstens
2026-08-03 16:40 ` Niklas Schnelle
2026-08-03 14:29 ` [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() " Heiko Carstens
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2026-08-03 14:29 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Niklas Schnelle, Gerd Bayer
Cc: linux-s390, linux-kernel
Clang's compiler based static context analysis does not work with
locks that are conditionally taken like in __zpci_event_error():
arch/s390/pci/pci_event.c:320:2: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
is not held on every path through here [-Wthread-safety-analysis]
Given that code which takes locks conditionally can be considered
suboptimal rework __zpci_event_error() to get rid of this.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/pci/pci_event.c | 43 ++++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 839bd91c056e..48fa26dcbee1 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -288,6 +288,12 @@ static void zpci_event_io_failure(struct pci_dev *pdev, pci_channel_state_t es)
pci_dev_unlock(pdev);
}
+static void __zpci_event_print_error(struct pci_dev *pdev, struct zpci_ccdf_err *ccdf)
+{
+ pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
+ pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
+}
+
static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
{
struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
@@ -301,24 +307,24 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
zpci_err("error CCDF:\n");
zpci_err_hex(ccdf, sizeof(*ccdf));
- if (zdev) {
- mutex_lock(&zdev->state_lock);
- rc = clp_refresh_fh(zdev->fid, &fh);
- if (rc)
- goto no_pdev;
- if (!fh || ccdf->fh != fh) {
- /* Ignore events with stale handles */
- zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
- ccdf->fid, fh, ccdf->fh);
- goto no_pdev;
- }
- zpci_update_fh(zdev, ccdf->fh);
- if (zdev->zbus->bus)
- pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
- }
+ if (!zdev)
+ return __zpci_event_print_error(pdev, ccdf);
- pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
- pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
+ mutex_lock(&zdev->state_lock);
+ rc = clp_refresh_fh(zdev->fid, &fh);
+ if (rc)
+ goto no_pdev;
+ if (!fh || ccdf->fh != fh) {
+ /* Ignore events with stale handles */
+ zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
+ ccdf->fid, fh, ccdf->fh);
+ goto no_pdev;
+ }
+ zpci_update_fh(zdev, ccdf->fh);
+ if (zdev->zbus->bus)
+ pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
+
+ __zpci_event_print_error(pdev, ccdf);
if (!pdev)
goto no_pdev;
@@ -340,8 +346,7 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
}
pci_dev_put(pdev);
no_pdev:
- if (zdev)
- mutex_unlock(&zdev->state_lock);
+ mutex_unlock(&zdev->state_lock);
zpci_zdev_put(zdev);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
@ 2026-08-03 14:29 ` Heiko Carstens
2026-08-05 11:32 ` Niklas Schnelle
2026-08-03 14:29 ` [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:55 ` [PATCH v2 0/3] " Niklas Schnelle
3 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2026-08-03 14:29 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Niklas Schnelle, Gerd Bayer
Cc: linux-s390, linux-kernel
Clang's compiler based static context analysis does not work with locks
that are conditionally taken like in __zpci_event_availability():
arch/s390/pci/pci_event.c:402:10: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
is not held on every path through here [-Wthread-safety-analysis]
Given that code which takes locks conditionally can be considered
suboptimal rework __zpci_event_availability() to get rid of this.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/pci/pci_event.c | 108 +++++++++++++++++++-------------------
1 file changed, 53 insertions(+), 55 deletions(-)
diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index 48fa26dcbee1..f96ee87405f9 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -389,19 +389,25 @@ static void zpci_event_reappear(struct zpci_dev *zdev)
static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
{
- struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
- bool existing_zdev = !!zdev;
+ struct zpci_dev *zdev;
enum zpci_state state;
zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
ccdf->fid, ccdf->fh, ccdf->pec);
- if (existing_zdev)
- mutex_lock(&zdev->state_lock);
+ /* 0x0306 - No handle or fid stored */
+ if (ccdf->pec == 0x0306) {
+ /* 0x308 or 0x302 for multiple devices */
+ zpci_remove_reserved_devices();
+ zpci_scan_devices();
+ return;
+ }
- switch (ccdf->pec) {
- case 0x0301: /* Reserved|Standby -> Configured */
- if (!zdev) {
+ zdev = get_zdev_by_fid(ccdf->fid);
+
+ if (!zdev) {
+ switch (ccdf->pec) {
+ case 0x0301: /* Reserved|Standby -> Configured */
zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
if (IS_ERR(zdev))
break;
@@ -409,18 +415,9 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
kfree(zdev);
break;
}
- } else {
- if (zdev->state == ZPCI_FN_STATE_RESERVED)
- zpci_event_reappear(zdev);
- /* the configuration request may be stale */
- else if (zdev->state != ZPCI_FN_STATE_STANDBY)
- break;
- zdev->state = ZPCI_FN_STATE_CONFIGURED;
- }
- zpci_scan_configured_device(zdev, ccdf->fh);
- break;
- case 0x0302: /* Reserved -> Standby */
- if (!zdev) {
+ zpci_scan_configured_device(zdev, ccdf->fh);
+ break;
+ case 0x0302: /* Reserved -> Standby */
zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_STANDBY);
if (IS_ERR(zdev))
break;
@@ -428,53 +425,54 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
kfree(zdev);
break;
}
- } else {
- if (zdev->state == ZPCI_FN_STATE_RESERVED)
- zpci_event_reappear(zdev);
- zpci_update_fh(zdev, ccdf->fh);
+ break;
}
+ return;
+ }
+
+ mutex_lock(&zdev->state_lock);
+ switch (ccdf->pec) {
+ case 0x0301: /* Reserved|Standby -> Configured */
+ if (zdev->state == ZPCI_FN_STATE_RESERVED)
+ zpci_event_reappear(zdev);
+ /* the configuration request may be stale */
+ else if (zdev->state != ZPCI_FN_STATE_STANDBY)
+ break;
+ zdev->state = ZPCI_FN_STATE_CONFIGURED;
+ zpci_scan_configured_device(zdev, ccdf->fh);
+ break;
+ case 0x0302: /* Reserved -> Standby */
+ if (zdev->state == ZPCI_FN_STATE_RESERVED)
+ zpci_event_reappear(zdev);
+ zpci_update_fh(zdev, ccdf->fh);
break;
case 0x0303: /* Deconfiguration requested */
- if (zdev) {
- /* The event may have been queued before we configured
- * the device.
- */
- if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
- break;
- zpci_update_fh(zdev, ccdf->fh);
- zpci_deconfigure_device(zdev);
- }
+ /* The event may have been queued before we configured
+ * the device.
+ */
+ if (zdev->state != ZPCI_FN_STATE_CONFIGURED)
+ break;
+ zpci_update_fh(zdev, ccdf->fh);
+ zpci_deconfigure_device(zdev);
break;
case 0x0304: /* Configured -> Standby|Reserved */
- if (zdev) {
- /* The event may have been queued before we configured
- * the device.:
- */
- if (zdev->state == ZPCI_FN_STATE_CONFIGURED)
- zpci_event_hard_deconfigured(zdev, ccdf->fh);
- /* The 0x0304 event may immediately reserve the device */
- if (!clp_get_state(zdev->fid, &state) &&
- state == ZPCI_FN_STATE_RESERVED) {
- zpci_device_reserved(zdev);
- }
+ /* The event may have been queued before we configured
+ * the device.:
+ */
+ if (zdev->state == ZPCI_FN_STATE_CONFIGURED)
+ zpci_event_hard_deconfigured(zdev, ccdf->fh);
+ /* The 0x0304 event may immediately reserve the device */
+ if (!clp_get_state(zdev->fid, &state) &&
+ state == ZPCI_FN_STATE_RESERVED) {
+ zpci_device_reserved(zdev);
}
break;
- case 0x0306: /* 0x308 or 0x302 for multiple devices */
- zpci_remove_reserved_devices();
- zpci_scan_devices();
- break;
case 0x0308: /* Standby -> Reserved */
- if (!zdev)
- break;
zpci_device_reserved(zdev);
break;
- default:
- break;
- }
- if (existing_zdev) {
- mutex_unlock(&zdev->state_lock);
- zpci_zdev_put(zdev);
}
+ mutex_unlock(&zdev->state_lock);
+ zpci_zdev_put(zdev);
}
void zpci_event_availability(void *data)
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() " Heiko Carstens
@ 2026-08-03 14:29 ` Heiko Carstens
2026-08-05 13:22 ` Niklas Schnelle
2026-08-03 14:55 ` [PATCH v2 0/3] " Niklas Schnelle
3 siblings, 1 reply; 9+ messages in thread
From: Heiko Carstens @ 2026-08-03 14:29 UTC (permalink / raw)
To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Niklas Schnelle, Gerd Bayer
Cc: linux-s390, linux-kernel
Enable CONTEXT_ANALYSIS since s390's pci code compiles now without
warnings.
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/pci/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile
index 1810e0944a4e..b8be1316491b 100644
--- a/arch/s390/pci/Makefile
+++ b/arch/s390/pci/Makefile
@@ -3,6 +3,8 @@
# Makefile for the s390 PCI subsystem.
#
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_clp.o \
pci_event.o pci_debug.o pci_insn.o pci_mmio.o \
pci_bus.o pci_kvm_hook.o pci_report.o pci_fixup.o
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
` (2 preceding siblings ...)
2026-08-03 14:29 ` [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
@ 2026-08-03 14:55 ` Niklas Schnelle
3 siblings, 0 replies; 9+ messages in thread
From: Niklas Schnelle @ 2026-08-03 14:55 UTC (permalink / raw)
To: Heiko Carstens, Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Gerd Bayer
Cc: linux-s390, linux-kernel
On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> v2:
>
> - Address potential zdev reference leak (false positive) by adding a
> comment, and moving get_zdev_by_fid() [Sashiko, Niklas]
>
> - Note: this reveals a pre-existing bug: zpci_remove_reserved_devices()
> calls zpci_device_reserved() without holding locks. It looks like
> this can be fixed easily by adding a mutex_lock()/mutex_unlock()
> pair in zpci_remove_reserved_devices(), but that is not within
> the scope of this series, and up to the pci maintainers
I agree with this assessment and can provide a patch on top of this
series to address this.
Thanks,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
@ 2026-08-03 16:40 ` Niklas Schnelle
0 siblings, 0 replies; 9+ messages in thread
From: Niklas Schnelle @ 2026-08-03 16:40 UTC (permalink / raw)
To: Heiko Carstens, Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Gerd Bayer
Cc: linux-s390, linux-kernel
On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> Clang's compiler based static context analysis does not work with
> locks that are conditionally taken like in __zpci_event_error():
>
> arch/s390/pci/pci_event.c:320:2: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
> is not held on every path through here [-Wthread-safety-analysis]
>
> Given that code which takes locks conditionally can be considered
> suboptimal rework __zpci_event_error() to get rid of this.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/pci/pci_event.c | 43 ++++++++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
> index 839bd91c056e..48fa26dcbee1 100644
> --- a/arch/s390/pci/pci_event.c
> +++ b/arch/s390/pci/pci_event.c
> @@ -288,6 +288,12 @@ static void zpci_event_io_failure(struct pci_dev *pdev, pci_channel_state_t es)
> pci_dev_unlock(pdev);
> }
>
> +static void __zpci_event_print_error(struct pci_dev *pdev, struct zpci_ccdf_err *ccdf)
> +{
> + pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
> + pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
> +}
> +
> static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
> {
> struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
> @@ -301,24 +307,24 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
> zpci_err("error CCDF:\n");
> zpci_err_hex(ccdf, sizeof(*ccdf));
>
> - if (zdev) {
> - mutex_lock(&zdev->state_lock);
> - rc = clp_refresh_fh(zdev->fid, &fh);
> - if (rc)
> - goto no_pdev;
> - if (!fh || ccdf->fh != fh) {
> - /* Ignore events with stale handles */
> - zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
> - ccdf->fid, fh, ccdf->fh);
> - goto no_pdev;
> - }
> - zpci_update_fh(zdev, ccdf->fh);
> - if (zdev->zbus->bus)
> - pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
> - }
> + if (!zdev)
> + return __zpci_event_print_error(pdev, ccdf);
Nit: Here pdev is always NULL because the assignment moved below.
Functionally this is fine but it would be more clear this is the case
if you passed NULL for pdev explicitly.
>
> - pr_err("%s: Event 0x%x reports an error for PCI function 0x%x\n",
> - pdev ? pci_name(pdev) : "n/a", ccdf->pec, ccdf->fid);
> + mutex_lock(&zdev->state_lock);
> + rc = clp_refresh_fh(zdev->fid, &fh);
> + if (rc)
> + goto no_pdev;
> + if (!fh || ccdf->fh != fh) {
> + /* Ignore events with stale handles */
> + zpci_dbg(3, "err fid:%x, fh:%x (stale %x)\n",
> + ccdf->fid, fh, ccdf->fh);
> + goto no_pdev;
> + }
> + zpci_update_fh(zdev, ccdf->fh);
> + if (zdev->zbus->bus)
> + pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn);
> +
> + __zpci_event_print_error(pdev, ccdf);
>
> if (!pdev)
> goto no_pdev;
> @@ -340,8 +346,7 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf)
> }
> pci_dev_put(pdev);
> no_pdev:
> - if (zdev)
> - mutex_unlock(&zdev->state_lock);
> + mutex_unlock(&zdev->state_lock);
> zpci_zdev_put(zdev);
> }
>
Thanks for cleaning this up. Even on its own this would be a net
improvement. So with or without my nit resolved feel free to add:
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking
2026-08-03 14:29 ` [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() " Heiko Carstens
@ 2026-08-05 11:32 ` Niklas Schnelle
2026-08-05 12:43 ` Heiko Carstens
0 siblings, 1 reply; 9+ messages in thread
From: Niklas Schnelle @ 2026-08-05 11:32 UTC (permalink / raw)
To: Heiko Carstens, Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Gerd Bayer
Cc: linux-s390, linux-kernel
On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> Clang's compiler based static context analysis does not work with locks
> that are conditionally taken like in __zpci_event_availability():
>
> arch/s390/pci/pci_event.c:402:10: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
> is not held on every path through here [-Wthread-safety-analysis]
>
> Given that code which takes locks conditionally can be considered
> suboptimal rework __zpci_event_availability() to get rid of this.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/pci/pci_event.c | 108 +++++++++++++++++++-------------------
> 1 file changed, 53 insertions(+), 55 deletions(-)
>
> diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
> index 48fa26dcbee1..f96ee87405f9 100644
> --- a/arch/s390/pci/pci_event.c
> +++ b/arch/s390/pci/pci_event.c
> @@ -389,19 +389,25 @@ static void zpci_event_reappear(struct zpci_dev *zdev)
>
> static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf)
> {
> - struct zpci_dev *zdev = get_zdev_by_fid(ccdf->fid);
> - bool existing_zdev = !!zdev;
> + struct zpci_dev *zdev;
> enum zpci_state state;
>
> zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n",
> ccdf->fid, ccdf->fh, ccdf->pec);
>
> - if (existing_zdev)
> - mutex_lock(&zdev->state_lock);
> + /* 0x0306 - No handle or fid stored */
> + if (ccdf->pec == 0x0306) {
> + /* 0x308 or 0x302 for multiple devices */
> + zpci_remove_reserved_devices();
> + zpci_scan_devices();
> + return;
> + }
>
> - switch (ccdf->pec) {
> - case 0x0301: /* Reserved|Standby -> Configured */
> - if (!zdev) {
> + zdev = get_zdev_by_fid(ccdf->fid);
> +
Nit: Stray empty line
> + if (!zdev) {
> + switch (ccdf->pec) {
> + case 0x0301: /* Reserved|Standby -> Configured */
> zdev = zpci_create_device(ccdf->fid, ccdf->fh, ZPCI_FN_STATE_CONFIGURED);
> if (IS_ERR(zdev))
> break;
--- snip ---
> + break;
> }
> + return;
> + }
--- snip ---
> +
> + mutex_lock(&zdev->state_lock);
> + switch (ccdf->pec) {
> + case 0x0301: /* Reserved|Standby -> Configured */
> + if (zdev->state == ZPCI_FN_STATE_RESERVED)
> + zpci_event_reappear(zdev);
> + /* the configuration request may be stale */
> + else if (zdev->state != ZPCI_FN_STATE_STANDBY)
> + break;
> + zdev->state = ZPCI_FN_STATE_CONFIGURED;
> + zpci_scan_configured_device(zdev, ccdf->fh);
> + break;
> + case 0x0302: /* Reserved -> Standby */
> + if (zdev->state == ZPCI_FN_STATE_RESERVED)
> + zpci_event_reappear(zdev);
> + zpci_update_fh(zdev, ccdf->fh);
--- snip ---
> }
> + mutex_unlock(&zdev->state_lock);
> + zpci_zdev_put(zdev);
> }
>
> void zpci_event_availability(void *data)
Personally I think I'd put the 0x0306 and each of the two switches in
helper functions to improve readability as this is getting awfully
long. Maybe something like zpci_event_avail_any_device() (0x0306),
zpci_event_avail_new_device() and zpci_event_avail_existing_device().
If you prefer and since I'm doing a follow up for the missing locking
in zpci_reserved_devices() I can also do that in a separate patch. Also
just to clarify Sashiko is right in that the missing locking is a pre-
existing issue as the state_lock was already not taken in that case,
it's just more obvious now.
Either way, functionality looks good to me so feel free to add:
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking
2026-08-05 11:32 ` Niklas Schnelle
@ 2026-08-05 12:43 ` Heiko Carstens
0 siblings, 0 replies; 9+ messages in thread
From: Heiko Carstens @ 2026-08-05 12:43 UTC (permalink / raw)
To: Niklas Schnelle
Cc: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Gerd Bayer, linux-s390, linux-kernel
On Wed, Aug 05, 2026 at 01:32:10PM +0200, Niklas Schnelle wrote:
> On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> > Clang's compiler based static context analysis does not work with locks
> > that are conditionally taken like in __zpci_event_availability():
> >
> > arch/s390/pci/pci_event.c:402:10: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
> > is not held on every path through here [-Wthread-safety-analysis]
> >
> > Given that code which takes locks conditionally can be considered
> > suboptimal rework __zpci_event_availability() to get rid of this.
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> > arch/s390/pci/pci_event.c | 108 +++++++++++++++++++-------------------
> > 1 file changed, 53 insertions(+), 55 deletions(-)
...
> Personally I think I'd put the 0x0306 and each of the two switches in
> helper functions to improve readability as this is getting awfully
> long. Maybe something like zpci_event_avail_any_device() (0x0306),
> zpci_event_avail_new_device() and zpci_event_avail_existing_device().
>
> If you prefer and since I'm doing a follow up for the missing locking
> in zpci_reserved_devices() I can also do that in a separate patch. Also
> just to clarify Sashiko is right in that the missing locking is a pre-
> existing issue as the state_lock was already not taken in that case,
> it's just more obvious now.
>
> Either way, functionality looks good to me so feel free to add:
>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks, I'll address the two nits for patch 1 and 2, and will send a
new version. However I prefer if you do additional code refactoring
with addon patches, so the result looks exactly like you want it.
Otherwise we will go back and forth :)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS
2026-08-03 14:29 ` [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
@ 2026-08-05 13:22 ` Niklas Schnelle
0 siblings, 0 replies; 9+ messages in thread
From: Niklas Schnelle @ 2026-08-05 13:22 UTC (permalink / raw)
To: Heiko Carstens, Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
Christian Borntraeger, Gerd Bayer
Cc: linux-s390, linux-kernel
On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> Enable CONTEXT_ANALYSIS since s390's pci code compiles now without
> warnings.
>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/pci/Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/s390/pci/Makefile b/arch/s390/pci/Makefile
> index 1810e0944a4e..b8be1316491b 100644
> --- a/arch/s390/pci/Makefile
> +++ b/arch/s390/pci/Makefile
> @@ -3,6 +3,8 @@
> # Makefile for the s390 PCI subsystem.
> #
>
> +CONTEXT_ANALYSIS := y
> +
> obj-$(CONFIG_PCI) += pci.o pci_irq.o pci_clp.o \
> pci_event.o pci_debug.o pci_insn.o pci_mmio.o \
> pci_bus.o pci_kvm_hook.o pci_report.o pci_fixup.o
Looks good to me and it didn't raise any issues when compiling with
clang and debug_defconfig (CONFIG_WARN_CONTEXT_ANALYSIS=y) for me. Also
used that build to confirm the locking issue with lockdep. It needs the
device to be in standby to get into that path.
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks,
Niklas
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-05 13:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
2026-08-03 16:40 ` Niklas Schnelle
2026-08-03 14:29 ` [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() " Heiko Carstens
2026-08-05 11:32 ` Niklas Schnelle
2026-08-05 12:43 ` Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-05 13:22 ` Niklas Schnelle
2026-08-03 14:55 ` [PATCH v2 0/3] " Niklas Schnelle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox