Re: [PATCH net-next 1/2] s390/ctcm: Add __context_unsafe() attribute to various functions

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unsafe conditional locking using `!in_hardirq()` leads to guaranteed self-deadlocks on PREEMPT_RT or when using forced threaded interrupts (`threadirqs`).
--

--- Patch [1]: [PATCH net-next 1/2] s390/ctcm: Add __context_unsafe() attribute to various functions ---
commit c3d2fbe266a854e35b040af4b1c68b39c513a193
Author: Heiko Carstens <[email protected]>
Subject: s390/ctcm: Add __context_unsafe() attribute to various functions

Disable context analysis for various functions to get rid of context
analysis compile time warnings using clang caused by conditional
locking.

> diff --git a/drivers/s390/net/ctcm_fsms.c b/drivers/s390/net/ctcm_fsms.c
> index bf917f4264532..84fd394d35250 100644
> --- a/drivers/s390/net/ctcm_fsms.c
> +++ b/drivers/s390/net/ctcm_fsms.c

[ ... ]

> @@ -1462,7 +1456,7 @@ static void ctcmpc_chx_rx(fsm_instance *fi, int event, void *arg)
>  			spin_lock_irqsave(
>  				get_ccwdev_lock(ch->cdev), saveflags);
>  		rc = ccw_device_start(ch->cdev, &ch->ccw[0], 0, 0xff, 0);
> -		if (dolock) /* see remark about conditional locking */
> +		if (dolock)
>  			spin_unlock_irqrestore(
>  				get_ccwdev_lock(ch->cdev), saveflags);

[Severity: High]
This is a pre-existing issue, but does this conditional locking using
!in_hardirq() (which sets dolock earlier in ctcmpc_chx_rx) cause
self-deadlocks on PREEMPT_RT or when using forced threaded interrupts?

Since the CIO interrupt handler already holds the CCW device lock, when
threaded interrupts are enabled, the hardware interrupt handler runs in
process context and in_hardirq() evaluates to false.

Would this cause ctcmpc_chx_rx() to mistakenly attempt to re-acquire the lock
it already holds, leading to an immediate self-deadlock?

[ ... ]

> diff --git a/drivers/s390/net/ctcm_mpc.c b/drivers/s390/net/ctcm_mpc.c
> index aeb102537e7f5..08e36685e5788 100644
> --- a/drivers/s390/net/ctcm_mpc.c
> +++ b/drivers/s390/net/ctcm_mpc.c

[ ... ]

> @@ -1774,9 +1775,6 @@ static void mpc_action_side_xid(fsm_instance *fsm, void *arg, int side)
>  	CTCM_D3_DUMP((char *)ch->xid_id, 4);
>  
>  	if (!in_hardirq()) {
> -			 /* Such conditional locking is a known problem for
> -			  * sparse because its static undeterministic.
> -			  * Warnings should be ignored here. */
>  		spin_lock_irqsave(get_ccwdev_lock(ch->cdev), saveflags);
>  		gotlock = 1;
>  	}

[Severity: High]
This is a pre-existing issue, but does checking !in_hardirq() for conditional
locking lead to the same self-deadlock here in mpc_action_side_xid()?

If this FSM action is executed from the interrupt handler while threaded
interrupts are active, !in_hardirq() will evaluate to true, causing an attempt
to acquire the already-held get_ccwdev_lock().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1