| Message ID | 1611807365-35513-2-git-send-email-cang@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] scsi: ufs: Fix task management request completion timeout | expand |
On 1/27/21 8:16 PM, Can Guo wrote: > ufshcd_tmc_handler() calls blk_mq_tagset_busy_iter(fn = ufshcd_compl_tm()), > but since blk_mq_tagset_busy_iter() only iterates over all reserved tags > and requests which are not in IDLE state, ufshcd_compl_tm() never gets a > chance to run. Thus, TMR always ends up with completion timeout. Fix it by > calling blk_mq_start_request() in __ufshcd_issue_tm_cmd(). > > Fixes: 69a6c269c097 ("scsi: ufs: Use blk_{get,put}_request() to allocate and free TMFs") > > Signed-off-by: Can Guo <cang@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 8da75e6..c0c5925 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -6395,6 +6395,7 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba, > > spin_lock_irqsave(host->host_lock, flags); > task_tag = hba->nutrs + free_slot; > + blk_mq_start_request(req); > > treq->req_header.dword_0 |= cpu_to_be32(task_tag); blk_mq_start_request() not only marks a request as in-flight but also starts a timer. However, no timeout handler has been defined in ufshcd_tmf_ops. Should a timeout handler be defined in that data structure? Thanks, Bart.
On 2021-01-29 11:22, Bart Van Assche wrote: > On 1/27/21 8:16 PM, Can Guo wrote: >> ufshcd_tmc_handler() calls blk_mq_tagset_busy_iter(fn = >> ufshcd_compl_tm()), >> but since blk_mq_tagset_busy_iter() only iterates over all reserved >> tags >> and requests which are not in IDLE state, ufshcd_compl_tm() never gets >> a >> chance to run. Thus, TMR always ends up with completion timeout. Fix >> it by >> calling blk_mq_start_request() in __ufshcd_issue_tm_cmd(). >> >> Fixes: 69a6c269c097 ("scsi: ufs: Use blk_{get,put}_request() to >> allocate and free TMFs") >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> --- >> drivers/scsi/ufs/ufshcd.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index 8da75e6..c0c5925 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -6395,6 +6395,7 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba >> *hba, >> >> spin_lock_irqsave(host->host_lock, flags); >> task_tag = hba->nutrs + free_slot; >> + blk_mq_start_request(req); >> >> treq->req_header.dword_0 |= cpu_to_be32(task_tag); > > blk_mq_start_request() not only marks a request as in-flight but also > starts a timer. However, no timeout handler has been defined in > ufshcd_tmf_ops. Should a timeout handler be defined in that data > structure? > Block mq driver gives 30s as default timeout, TMR timeout is 100ms in UFS driver. So we don't need a timeout handler as of now. Thanks, Can Guo. > Thanks, > > Bart.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8da75e6..c0c5925 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6395,6 +6395,7 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba, spin_lock_irqsave(host->host_lock, flags); task_tag = hba->nutrs + free_slot; + blk_mq_start_request(req); treq->req_header.dword_0 |= cpu_to_be32(task_tag);
ufshcd_tmc_handler() calls blk_mq_tagset_busy_iter(fn = ufshcd_compl_tm()), but since blk_mq_tagset_busy_iter() only iterates over all reserved tags and requests which are not in IDLE state, ufshcd_compl_tm() never gets a chance to run. Thus, TMR always ends up with completion timeout. Fix it by calling blk_mq_start_request() in __ufshcd_issue_tm_cmd(). Fixes: 69a6c269c097 ("scsi: ufs: Use blk_{get,put}_request() to allocate and free TMFs") Signed-off-by: Can Guo <cang@codeaurora.org> --- drivers/scsi/ufs/ufshcd.c | 1 + 1 file changed, 1 insertion(+)