| Message ID | 20221018202958.1902564-10-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix a deadlock in the UFS driver | expand |
On 10/18/22 3:29 PM, Bart Van Assche wrote: > Open-code scsi_execute() because a later patch will modify scmd->flags > and because scsi_execute() does not support setting scmd->flags. No > functionality is changed. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/ufs/core/ufshcd.c | 39 ++++++++++++++++++++++++++++++++++----- > 1 file changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 2a32bcc93d2e..c5ccc7ba583b 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -8729,6 +8729,39 @@ static void ufshcd_hba_exit(struct ufs_hba *hba) > } > } > > +static int ufshcd_execute_start_stop(struct scsi_device *sdev, > + enum ufs_dev_pwr_mode pwr_mode, > + struct scsi_sense_hdr *sshdr) > +{ > + unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 }; > + struct request *req; > + struct scsi_cmnd *scmd; > + int ret; > + > + req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, > + BLK_MQ_REQ_PM); > Can you hit a case where we have run out of tags (__blk_mq_alloc_requests is hitting the blk_mq_get_tag == BLK_MQ_NO_TAG check), the host has gone into recovery and so commands are completing to add a tag back and then we try to call this and get stuck waiting on a tag? Or for passthrough do we have some special reserve? If so do you need to use BLK_MQ_REQ_NOWAIT here? Maybe do the retry loop yourself like: retry: if host is in recovery return failure req = scsi_alloc_request(.... BLK_MQ_REQ_NOWAIT) if (!req and we have not hit some retry limit) goto retry or have some special reserve command/tag.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 2a32bcc93d2e..c5ccc7ba583b 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8729,6 +8729,39 @@ static void ufshcd_hba_exit(struct ufs_hba *hba) } } +static int ufshcd_execute_start_stop(struct scsi_device *sdev, + enum ufs_dev_pwr_mode pwr_mode, + struct scsi_sense_hdr *sshdr) +{ + unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 }; + struct request *req; + struct scsi_cmnd *scmd; + int ret; + + req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, + BLK_MQ_REQ_PM); + if (IS_ERR(req)) + return PTR_ERR(req); + + scmd = blk_mq_rq_to_pdu(req); + scmd->cmd_len = COMMAND_SIZE(cdb[0]); + memcpy(scmd->cmnd, cdb, scmd->cmd_len); + scmd->allowed = 0/*retries*/; + req->timeout = 1 * HZ; + req->rq_flags |= RQF_PM | RQF_QUIET; + + blk_execute_rq(req, /*at_head=*/true); + + if (sshdr) + scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len, + sshdr); + ret = scmd->result; + + blk_mq_free_request(req); + + return ret; +} + /** * ufshcd_set_dev_pwr_mode - sends START STOP UNIT command to set device * power mode @@ -8741,7 +8774,6 @@ static void ufshcd_hba_exit(struct ufs_hba *hba) static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, enum ufs_dev_pwr_mode pwr_mode) { - unsigned char cmd[6] = { START_STOP }; struct scsi_sense_hdr sshdr; struct scsi_device *sdp; unsigned long flags; @@ -8766,16 +8798,13 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, */ hba->host->eh_noresume = 1; - cmd[4] = pwr_mode << 4; - /* * Current function would be generally called from the power management * callbacks hence set the RQF_PM flag so that it doesn't resume the * already suspended childs. */ for (retries = 3; retries > 0; --retries) { - ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr, - HZ, 0, 0, RQF_PM, NULL); + ret = ufshcd_execute_start_stop(sdp, pwr_mode, &sshdr); /* * scsi_execute() only returns a negative value if the request * queue is dying.
Open-code scsi_execute() because a later patch will modify scmd->flags and because scsi_execute() does not support setting scmd->flags. No functionality is changed. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/ufs/core/ufshcd.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-)