| Message ID | 1617166236-39908-2-git-send-email-cang@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Two fixes for task management request implementation | expand |
> 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 b49555fa..d4f8cb2 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -6464,6 +6464,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); Maybe just set req->state to MQ_RQ_IN_FLIGHT Without all other irrelevant initializations such as add timeout etc. Thanks, Avri > > treq->req_header.dword_0 |= cpu_to_be32(task_tag); > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project.
On 2021-04-01 00:45, Avri Altman 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 b49555fa..d4f8cb2 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -6464,6 +6464,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); > Maybe just set req->state to MQ_RQ_IN_FLIGHT > Without all other irrelevant initializations such as add timeout etc. > I don't see any other drivers do that, is it appropriate to call WRITE_ONCE(rq->state, MQ_RQ_IN_FLIGHT) outside block layer? Thanks, Can Guo. > Thanks, > Avri >> >> treq->req_header.dword_0 |= cpu_to_be32(task_tag); >> >> -- >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a >> Linux Foundation Collaborative Project.
On 3/31/21 9:45 AM, Avri Altman 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 b49555fa..d4f8cb2 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -6464,6 +6464,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); > Maybe just set req->state to MQ_RQ_IN_FLIGHT > Without all other irrelevant initializations such as add timeout etc. Hmm ... I'm not sure that any of the actions performed by blk_mq_start_request() are irrelevant in this context. Additionally, no other block or SCSI driver sets MQ_RQ_IN_FLIGHT directly. Thanks, Bart.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index b49555fa..d4f8cb2 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6464,6 +6464,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(+)