Re: [PATCH v3 4/9] scsi: ufs: Complete the cmd before returning in queuecommand

From: Can Guo
Date: Sun Jun 13 2021 - 09:30:44 EST


On 2021-06-12 23:50, Bart Van Assche wrote:
On 6/12/21 12:38 AM, Can Guo wrote:
On 2021-06-12 04:52, Bart Van Assche wrote:
On 6/9/21 9:43 PM, Can Guo wrote:
@@ -2768,15 +2778,6 @@ static int ufshcd_queuecommand(struct
Scsi_Host *host, struct scsi_cmnd *cmd)
     WARN_ON(ufshcd_is_clkgating_allowed(hba) &&
         (hba->clk_gating.state != CLKS_ON));

-    if (unlikely(test_bit(tag, &hba->outstanding_reqs))) {
-        if (hba->wl_pm_op_in_progress)
-            set_host_byte(cmd, DID_BAD_TARGET);
-        else
-            err = SCSI_MLQUEUE_HOST_BUSY;
-        ufshcd_release(hba);
-        goto out;
-    }
-
     lrbp = &hba->lrb[tag];
     WARN_ON(lrbp->cmd);
     lrbp->cmd = cmd;

Can the code under "if (unlikely(test_bit(tag,
&hba->outstanding_reqs)))" be deleted instead of moving it? I don't
think that it is useful to verify whether the block layer tag allocator
works correctly. Additionally, I'm not aware of any similar code in any
other SCSI LLD.

ufshcd_abort() aborts PM requests differently from other requests -
it simply evicts the cmd from lrbp [1], schedules error handler and
returns SUCCESS (the reason why I am doing it this way is in patch #8).

After ufshcd_abort() returns, the tag shall be released, the logic
here is to prevent subsequent cmds re-use the lrbp [1] before error
handler recovers the device and host.

Thanks for the background information. However, this approach sounds
cumbersome to me. For PM requests, please change the UFS driver such
that calling scsi_done() for aborted requests is postponed until error
handling has finished and delete the code shown above from
ufshcd_queuecommand().

I will delete the code in next version, since I believe the hba_state
checks before the code is enough to achieve the same purpose, so this
code becomes redundant.

Thanks,

Can Guo.


Thanks,

Bart.