Re: [PATCH v2] ufs: core: fix ufshcd_abort_all racing issue

[Bart Van Assche <bvanassche@acm.org>]

On 6/24/24 5:11 AM, peter.wang@mediatek.com wrote:
 > [ ... ]
In this patch there are two call traces, two fixes tags and two code
changes. Please split this patch into two patches with each one call
trace, one Fixes: tag and one code change. Additionally, please include
a changelog when posting a second or later version.

> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 8944548c30fa..3b2e5bcb08a7 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -512,8 +512,9 @@ int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag)
>   		return -ETIMEDOUT;
>   
>   	if (task_tag != hba->nutrs - UFSHCD_NUM_RESERVED) {
> -		if (!cmd)
> -			return -EINVAL;
> +		/* Should return 0 if cmd is already complete by irq */
> +		if (!cmd || !ufshcd_cmd_inflight(cmd))
> +			return 0;
>   		hwq = ufshcd_mcq_req_to_hwq(hba, scsi_cmd_to_rq(cmd));
>   	} else {
>   		hwq = hba->dev_cmd_queue;

Does the call trace show that blk_mq_unique_tag() tries to dereference 
address 0x194? If so, how is this possible? There are
only two lrbp->cmd assignments in the UFS driver. These assignments
either assign a valid SCSI command pointer or NULL. Even after a SCSI
command has been completed, the SCSI command pointer remains valid. So
how can an invalid pointer be passed to blk_mq_unique_tag()? Please
root-cause this issue instead of posting a code change that reduces a
race window without closing the race window completely.

Thanks,

Bart.