Re: [PATCH 06/13] nvme: sysfs: emit the marginal path state in show_state()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional override of sysfs state attribute with 'marginal', masking critical controller states.
--

--- Patch [6]: [PATCH 06/13] nvme: sysfs: emit the marginal path state in show_state() ---
commit 881ed7168d40f086dcba6f49059055d962ce09aa
Author: Bryan Gurney <[email protected]>

nvme: sysfs: emit the marginal path state in show_state()

If a controller has received a link integrity or congestion event, and
has the NVME_CTRL_MARGINAL flag set, emit "marginal" in the state
instead of "live", to identify the marginal paths.

> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -527,7 +527,9 @@ static ssize_t nvme_sysfs_show_state(struct device *dev,
>  	};
>  
>  	if (state < ARRAY_SIZE(state_name) && state_name[state])
> -		return sysfs_emit(buf, "%s\n", state_name[state]);
> +		return sysfs_emit(buf, "%s\n",
> +			(nvme_ctrl_is_marginal(ctrl)) ? "marginal" :
> +			state_name[state]);

[Severity: High]
The commit message mentions emitting "marginal" in the state instead of
"live", but does this implementation unconditionally override all valid
states if the NVME_CTRL_MARGINAL flag is set?

If a marginal controller encounters a fatal error or is deleted, changing its
internal state to NVME_CTRL_DEAD, NVME_CTRL_RESETTING, or NVME_CTRL_DELETING,
won't this still emit "marginal" as long as the flag is set?

Could this mask critical controller states and cause userspace tools that
poll this sysfs attribute for teardown or error handling to hang while waiting
for the state to change?

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