Re: [PATCH v2] ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ufs: mcq: re-read header.ocs after timeout in ufshcd_mcq_sq_cleanup()
--

commit 840d70c7b6d87e03c26e1ea5e9102ecbc05ff2f0
Author: Kiwoong Kim <kwmad.kim@samsung.com>

ufs: mcq: fix dev cmd abort path and add OCS poll after sq cleanup

This commit skips the task management path in ufshcd_try_to_abort_task()
for reserved device management commands and calls ufshcd_clear_cmd()
directly. It also extends ufshcd_mcq_sq_cleanup() to poll header.ocs for
device management commands after SQ_CUS is set.

Link: https://lore.kernel.org/linux-scsi/20261030083311.602909-1-kwmad.kim@samsung.com/

> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 25e392f8ede2b..8c8b8e018fbdc 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -613,6 +613,31 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
>  			 __func__, id,
>  			 FIELD_GET(SQ_ICU_ERR_CODE_MASK, readl(reg)));
>  
> +	/*
> +	 * For device management commands, there is no order guaranteed between
> +	 * OCS update and CUS update, which is why we need to wait here. SCSI
> +	 * commands are excluded because the OCS in their UTRD is not updated
> +	 * from the CQE.
> +	 */
> +	if (!err && blk_mq_is_reserved_rq(rq)) {
> +		enum utp_ocs ocs;
> +		ktime_t start = ktime_get();
> +
> +		do {
> +			unsigned long flags;
> +
> +			spin_lock_irqsave(&hwq->cq_lock, flags);
> +			ocs = READ_ONCE(lrbp->utr_descriptor_ptr->header.ocs);
> +			spin_unlock_irqrestore(&hwq->cq_lock, flags);
> +			if (ocs != OCS_INVALID_COMMAND_STATUS)
> +				break;
> +			usleep_range(20, 50);
> +		} while (ktime_us_delta(ktime_get(), start) < MCQ_POLL_US);
> +
> +		if (ocs == OCS_INVALID_COMMAND_STATUS)
> +			err = -ETIMEDOUT;
> +	}

[Severity: Medium]
Can this loop in ufshcd_mcq_sq_cleanup() return a false -ETIMEDOUT if the
CQ interrupt updates lrbp->utr_descriptor_ptr->header.ocs while the task
is sleeping in usleep_range(20, 50)?

If the task wakes up after MCQ_POLL_US has elapsed, either on the final
polling iteration or due to scheduling delay on a busy CPU, the while
condition evaluates to false and exits the loop without re-reading
header.ocs.

The subsequent check then evaluates the stale pre-sleep ocs value and sets
err = -ETIMEDOUT, which causes ufshcd_mcq_abort() to fail and trigger an
unnecessary full SCSI host reset even though the command cleanup completed.

Should this re-read lrbp->utr_descriptor_ptr->header.ocs under hwq->cq_lock
after the loop exits (or use poll_timeout_us() from include/linux/iopoll.h)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030093645.790159-1-kwmad.kim@samsung.com?part=1