On 11/24/21 3:02 AM, Adrian Hunter wrote:
On 19/11/2021 21:57, Bart Van Assche wrote:The only functional change in this patch is the addition of a blk_mq_start_request() call in ufshcd_issue_devman_upiu_cmd(). Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx> --- drivers/scsi/ufs/ufshcd.c | 46 +++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index fced4528ee90..dfa5f127342b 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2933,6 +2933,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, { struct request_queue *q = hba->cmd_queue; DECLARE_COMPLETION_ONSTACK(wait); + struct scsi_cmnd *scmd; struct request *req; struct ufshcd_lrb *lrbp; int err; @@ -2945,15 +2946,18 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, * Even though we use wait_event() which sleeps indefinitely, * the maximum wait time is bounded by SCSI request timeout. */ - 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);We do not need the block layer, nor SCSI commands which begs the question, why involve them at all? For example, the following is much simpler and seems to work: [ ... ]
That patch bypasses the block layer for device management commands. So that patch breaks a very basic assumption on which the block layer has been built, namely that the block layer core knows whether or not any requests are ongoing. That patch breaks at least the following functionality: * Run-time power management. blk_pre_runtime_suspend() checks whether q_usage_counter is zero before initiating runtime power management. q_usage_counter is increased by blk_mq_alloc_request() and decreased by blk_mq_free_request(). I don't think it is safe to change the power state while a device management request is in progress. * The code in blk_cleanup_queue() that waits for pending requests to finish before resources associated with the request queue are freed. ufshcd_remove() calls blk_cleanup_queue(hba->cmd_queue) and hence waits until pending device management commands have finished. That would no longer be the case if the block layer is bypassed to submit device management commands. Bart.
