Re: [PATCH v3] scsi: core: pair EH runtime PM get and put

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the SCSI error handler thread enters an infinite CPU spin (livelock) if `scsi_autopm_get_host(shost)` persistently fails.
--

--- Patch [1]: [PATCH v3] scsi: core: pair EH runtime PM get and put ---
commit c29848e943092f2d5f259d4461009f3a5aa7ad12
Author: Hongjie Fang <[email protected]>

scsi: core: pair EH runtime PM get and put

This commit modifies the SCSI error handler to snapshot the eh_noresume
flag using READ_ONCE() at the start of each EH iteration. This ensures
that runtime PM get and put operations remain balanced even if the flag
changes concurrently. It also changes eh_noresume from a bitfield to a
regular bool.

> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 453a2232452db..74b70801269bd 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c

[ ... ]

> @@ -2403,7 +2404,8 @@ int scsi_error_handler(void *data)
>  		 * what we need to do to get it up and online again (if we can).
>  		 * If we fail, we end up taking the thing offline.
>  		 */
> -		if (!shost->eh_noresume && scsi_autopm_get_host(shost) != 0) {
> +		eh_noresume = READ_ONCE(shost->eh_noresume);
> +		if (!eh_noresume && scsi_autopm_get_host(shost) != 0) {
>  			SCSI_LOG_ERROR_RECOVERY(1,
>  				shost_printk(KERN_ERR, shost,
>  					     "scsi_eh_%d: unable to autoresume\n",
>  					     shost->host_no));
>  			continue;
                        ^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but does this continue statement
cause an infinite CPU spin if scsi_autopm_get_host() persistently fails?

If the SCSI host encounters an error, shost->host_failed == scsi_host_busy(shost)
becomes true, and the error handler thread wakes up. If the PM resume returns
a non-zero error (like -ENODEV during hot-unplug), the code logs an error and
hits this continue statement.

This skips the scsi_unjam_host() call and the shost->host_failed = 0
assignment. On the next loop iteration in scsi_error_handler(), host_failed
still equals host_busy, so the sleep condition remains false. This causes
the thread to immediately loop and retry the PM resume, spinning endlessly
on a CPU core.

>  		}

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