Messages in this thread | | | Date | Tue, 26 Jan 2021 10:10:31 +0800 | From | Can Guo <> | Subject | Re: [PATCH v2] scsi: ufs: Fix some problems in task management request implementation |
| |
On 2021-01-25 19:36, Avri Altman wrote: >> Current task management request send/compl implementation is broken, >> the >> problems and fixes are listed as below: >> >> Problem: TMR completion timeout. 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 >> started requests, so ufshcd_compl_tm() never gets a chance to >> run. >> Fix: Call blk_mq_start_request() in __ufshcd_issue_tm_cmd(). >> >> Problem: Race condition in send/compl paths. ufshcd_compl_tm() looks >> for >> all 0 bits in the REG_UTP_TASK_REQ_DOOR_BELL and call >> complete() >> for each req who has the req->end_io_data set. There can be a >> race >> condition btw tmc send/compl, because req->end_io_data is >> set, in >> __ufshcd_issue_tm_cmd(), without host lock protection, so it >> is >> possible that when ufshcd_compl_tm() checks the >> req->end_io_data, >> req->end_io_data is set but the corresponding tag has not >> been set >> in the REG_UTP_TASK_REQ_DOOR_BELL. Thus, ufshcd_tmc_handler() >> may >> wrongly complete TMRs which have not been sent. >> Fix: Protect req->end_io_data with host lock. And let >> ufshcd_compl_tm() >> only handle those tm cmds which have been completed instead >> of >> looking for 0 bits in the REG_UTP_TASK_REQ_DOOR_BELL. >> >> Problem: In __ufshcd_issue_tm_cmd(), it is not right to use hba->nutrs >> + >> req->tag as the Task Tag in one TMR UPIU. >> Fix: Directly use req->tag as Task Tag. >> >> Cc: Jaegeuk Kim <jaegeuk@kernel.org> > Since you are practically reverting Bart's change (69a6c269c097), > maybe cc him as well, > And add a fixes tag? >
Hi Avri,
It is not reverting Bart's change, but making TMR work properly based on it. I am ok with the Bart's idea of getting a tag for TMR from blk_get_request(), and this patch respects that idea.
> Also, even though all those fixes are around the same place, but > fixing different issues, > You might want to consider to separate those. Whatever you think. >
Thanks for the suggestion. I treat it as a whole because it is convenient for me to get it ported and tested over different platforms. I may revise it in next version after more comments come on it.
Thanks, Can Guo.
> Thanks, > Avri
| |