| Message ID | 1563947418-16394-1-git-send-email-stanley.chu@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | scsi: ufs: fix broken hba->outstanding_tasks | expand |
Stanly, > > Currently bits in hba->outstanding_tasks are cleared only after their > corresponding task management commands are successfully done by > __ufshcd_issue_tm_cmd(). > > If timeout happens in a task management command, its corresponding > bit in hba->outstanding_tasks will not be cleared until next task > management command with the same tag used successfully finishes. I'm sorry - I still don't understand why you just can't release the tag either way, Just like we do in device management queries tags, Instead of adding all this unnecessary code. I will not object to your series - just step down and let other people review you patches. Thanks, Avri
Hi Avri, On Thu, 2019-07-25 at 07:54 +0000, Avri Altman wrote: > Stanly, > > > > > Currently bits in hba->outstanding_tasks are cleared only after their > > corresponding task management commands are successfully done by > > __ufshcd_issue_tm_cmd(). > > > > If timeout happens in a task management command, its corresponding > > bit in hba->outstanding_tasks will not be cleared until next task > > management command with the same tag used successfully finishes. > I'm sorry - I still don't understand why you just can't release the tag either way, > Just like we do in device management queries tags, > Instead of adding all this unnecessary code. > > I will not object to your series - > just step down and let other people review you patches. Sorry to not describe the failed scenario clearly. Simpliy focus on outstanding bits cleanup in failed (timeout) case: - For device command, if timeout happens, its tag can be cleared in ufshcd_wait_for_dev_cmd() which specifically uses ufshcd_outstanding_req_clear() to clear failed bit in outstanding_reqs mask. - For task management command, if timeout happens, current driver will not clear failed bit in outstanding_tasks mask: - __ufshcd_issue_tm_cmd() will not clear it, - ufshcd_tmc_handler() will not clear it either during reset flow. > Thanks, > Avri Thanks, Stanley > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi Avri, On Thu, 2019-07-25 at 16:52 +0800, Stanley Chu wrote: > Hi Avri, > > On Thu, 2019-07-25 at 07:54 +0000, Avri Altman wrote: > > Stanly, > > > > > > > > Currently bits in hba->outstanding_tasks are cleared only after their > > > corresponding task management commands are successfully done by > > > __ufshcd_issue_tm_cmd(). > > > > > > If timeout happens in a task management command, its corresponding > > > bit in hba->outstanding_tasks will not be cleared until next task > > > management command with the same tag used successfully finishes. > > I'm sorry - I still don't understand why you just can't release the tag either way, > > Just like we do in device management queries tags, > > Instead of adding all this unnecessary code. > > > > I will not object to your series - > > just step down and let other people review you patches. Sorry for late response due to these busy days. I just got your point and agreed with you: previous proposal may be too tricky. Simple always wins. So I will provide a short solution in next version. Many thanks! Stanley
Currently bits in hba->outstanding_tasks are cleared only after their corresponding task management commands are successfully done by __ufshcd_issue_tm_cmd(). If timeout happens in a task management command, its corresponding bit in hba->outstanding_tasks will not be cleared until next task management command with the same tag used successfully finishes. This is wrong and can lead to some issues, like power issue. For example, ufshcd_release() and ufshcd_gate_work() will do nothing if hba->outstanding_tasks is not zero even if both UFS host and devices are actually idle. Referring to error handling flow of hba->outstanding_reqs, all timed-out bits will be cleared by ufshcd_reset_and_restore() => ufshcd_transfer_req_compl() after reset is done. Therefore similar handling for hba->outstanding_tasks could be applied, for example, by ufshcd_reset_and_restore() => ufshcd_tmc_handler(). This patch tries to "re-factor" cleanup jobs first, and then add fixed flow to make the whole patch more readable. Stanley Chu (3): scsi: ufs: clean-up task resource immediately only if task is responded scsi: ufs: introduce ufshcd_tm_cmd_compl() to refactor task cleanup scsi: ufs: fix broken hba->outstanding_tasks drivers/scsi/ufs/ufshcd.c | 41 ++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-)