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

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

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.