lkml.org 
[lkml]   [2021]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2021-01-26 11:49    [W:0.042 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site