Thread (4 messages) 4 messages, 3 authors, 2021-01-26

RE: [PATCH v2] scsi: ufs: Fix some problems in task management request implementation

From: Avri Altman <Avri.Altman@wdc.com>
Date: 2021-01-26 19:14:22
Also in: lkml

 
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?

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,
Avri
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help