On 19/11/2021 21:57, Bart Van Assche wrote:
> Release resources when aborting a command. Make sure that aborted commands
> are completed once by clearing the corresponding tag bit from
> hba->outstanding_reqs.
>
> Fixes: 7a3e97b0dc4b ("[SCSI] ufshcd: UFS Host controller driver")
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/scsi/ufs/ufshcd.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 39dcf997a638..7e27d6436889 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7042,8 +7042,12 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>
> ufshcd_hold(hba, false);
> reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - /* If command is already aborted/completed, return FAILED. */
> - if (!(test_bit(tag, &hba->outstanding_reqs))) {
> + /*
> + * If the command is already aborted/completed, return FAILED. This
> + * should never happen since the SCSI core serializes error handling
> + * and scsi_done() calls.
I don't think that is correct. ufshcd_transfer_req_compl() gets called directly
by the interrupt handler.
> + */
> + if (WARN_ON_ONCE(!(test_bit(tag, &hba->outstanding_reqs)))) {
> dev_err(hba->dev,
> "%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n",
> __func__, tag, hba->outstanding_reqs, reg);
> @@ -7113,6 +7117,16 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> goto release;
> }
>
> + /*
> + * Clear the corresponding bit from outstanding_reqs since the command
> + * has been aborted successfully.
> + */
> + spin_lock_irqsave(&hba->outstanding_lock, flags);
> + __clear_bit(tag, &hba->outstanding_reqs);
> + spin_unlock_irqrestore(&hba->outstanding_lock, flags);
> +
> + ufshcd_release_scsi_cmd(hba, lrbp);
ufshcd_release_scsi_cmd() must not be called if the bit was already clear
i.e.
spin_lock_irqsave(&hba->outstanding_lock, flags);
rel = __test_and_clear_bit(tag, &hba->outstanding_reqs);
spin_unlock_irqrestore(&hba->outstanding_lock, flags);
if (rel)
ufshcd_release_scsi_cmd(hba, lrbp);
> +
> err = SUCCESS;
>
> release:
>
