Re: [PATCH v5 17/40] scsi: mpt3sas: use 16-bits defined sense codes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `rc` variable in `_scsih_determine_disposition` is declared as `static`, which causes a data race across concurrent executions.
--

commit 4f69934411b8a4f00e0553bbc3f43bf991c80d7a
Author: Damien Le Moal <dlemoal@kernel.org>

scsi: mpt3sas: use 16-bits defined sense codes

Refactor the mpt3sas driver to replace all hard-coded additional sense
codes and additional sense code qualifiers with the enum values defined in
include/scsi/scsi_sense.h. This helps with code clarity as the sense codes
being processed are easier to test and self-documented.

No functional change intended.

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 0e12009a87f62..085d4672db854 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
[ ... ]
> @@ -7691,38 +7691,41 @@ _scsih_determine_disposition(struct MPT3SAS_ADAPTER *ioc,
>  
>  	if (check_sense) {
>  		_scsih_normalize_sense(transfer_packet->sense, &sense_info);
> -		if (sense_info.skey == UNIT_ATTENTION)
> +		if (sense_info.skey == UNIT_ATTENTION) {
>  			rc = DEVICE_RETRY_UA;
> -		else if (sense_info.skey == NOT_READY) {
> -			/* medium isn't present */
> -			if (sense_info.asc == 0x3a)
> +		} else if (sense_info.skey == NOT_READY) {
> +			if (sense_info.asc == ASC_MEDIUM_NOT_PRESENT) {
>  				rc = DEVICE_READY;
                        ^^

[Severity: High]
This is a pre-existing issue, but does the use of the static rc variable
in _scsih_determine_disposition() cause a data race across concurrent
executions?

Looking at the variable declaration at the beginning of the function:

static enum device_responsive_state
_scsih_determine_disposition(struct MPT3SAS_ADAPTER *ioc,
	struct _scsi_io_transfer *transfer_packet)
{
	static enum device_responsive_state rc;

Since rc is declared as static, it is shared across all concurrent
invocations of the function. Can this cause the function to return an incorrect
device_responsive_state (e.g., DEVICE_ERROR instead of DEVICE_READY or vice
versa) for a device when executing concurrently during adapter scanning or event
processing across multiple threads?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907024343.1046387-1-dlemoal@kernel.org?part=17