Re: [PATCH v3 2/2] ufs: core: requeue aborted request

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

On 9/9/24 1:21 AM, peter.wang@mediatek.com wrote:
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 3903947dbed1..1f57ebf24a39 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -642,6 +642,7 @@ static bool ufshcd_mcq_sqe_search(struct ufs_hba *hba,
>   		match = le64_to_cpu(utrd->command_desc_base_addr) & CQE_UCD_BA;
>   		if (addr == match) {
>   			ufshcd_mcq_nullify_sqe(utrd);
> +			lrbp->abort_initiated_by_err = true;
>   			ret = true;
>   			goto out;
>   		}

As mentioned before, I think that this change is wrong. Setting
lrbp->abort_initiated_by_err affects the value of scsi_cmnd::result.
This member variable is ignored for aborted commands. Although the
above change does not affect the SCSI error handler, I think it should
be left out because it will confuse anyone who reads the UFS driver code
and who has not followed this discussion.

> @@ -7561,6 +7552,16 @@ int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
>   		goto out;
>   	}
>   
> +	/*
> +	 * When the host software receives a "FUNCTION COMPLETE", set flag
> +	 * to requeue command after receive response with OCS_ABORTED
> +	 * SDB mode: UTRLCLR Task Management response which means a Transfer
> +	 *           Request was aborted.
> +	 * MCQ mode: Host will post to CQ with OCS_ABORTED after SQ cleanup
> +	 */
> +	if (!err)
> +		lrbp->abort_initiated_by_err = true;

Please add a comment that explains that the purpose of this code is to 
requeue commands aborted by ufshcd_abort_all().

> + * @abort_initiated_by_err: abort initiated by error

The member variable name and also the explanation of this member
variable are incomprehensible to me.

Thanks,

Bart.