Re: [PATCH v2 11/20] scsi: ufs: Switch to scsi_(get|put)_internal_cmd()

Bart Van Assche
2021-11-23T17:54:56Z

On 11/23/21 4:20 AM, Bean Huo wrote:
> On Fri, 2021-11-19 at 11:57 -0800, Bart Van Assche wrote:
>> @@ -6722,13 +6728,19 @@ static int
>> ufshcd_issue_devman_upiu_cmd(struct ufs_hba *hba,
>>
>> down_read(&hba->clk_scaling_lock);
>>
>> - req = blk_mq_alloc_request(q, REQ_OP_DRV_OUT, 0);
>> - if (IS_ERR(req)) {
>> - err = PTR_ERR(req);
>> + scmd = scsi_get_internal_cmd(q, DMA_TO_DEVICE, 0);
>> + if (IS_ERR(scmd)) {
>> + err = PTR_ERR(scmd);
>> goto out_unlock;
>> }
>> + req = scsi_cmd_to_rq(scmd);
>> tag = req->tag;
>> WARN_ONCE(tag < 0, "Invalid tag %d\n", tag);
>> + /*
>> + * Start the request such that blk_mq_tag_idle() is called when
>> the
>> + * device management request finishes.
>> + */
>> + blk_mq_start_request(req);
>
> Bart,
>
> Calling blk_mq_start_request() will inject the trace print of the block
> issued, but we do not have its paired completion trace print.
> In addition, blk_mq_tag_idle() will not be called after the device
> management request is completed, it will be called after the timer
> expires.
>
> I remember that we used to not allow this kind of LLD internal commands
> to be attached to the block layer. I now find that to be correct way.

Hi Bean,

As you may remember commit d0b2b70eb12e ("scsi: ufs: core: Increase the
usable queue depth") introduced a blk_mq_start_request() call in
ufshcd_exec_dev_cmd() to restore the queue depth from 16 to 32. I think
we need the same fix in ufshcd_issue_devman_upiu_cmd(). How about modifying
patch 1/20 of this series such that tracing is skipped for internal
requests? Would that address your concern?

Thanks,

Bart.