linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Avri Altman <Avri.Altman@wdc.com>
To: Can Guo <cang@codeaurora.org>,
	"jaegeuk@kernel.org" <jaegeuk@kernel.org>,
	"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
	"nguyenb@codeaurora.org" <nguyenb@codeaurora.org>,
	"hongwus@codeaurora.org" <hongwus@codeaurora.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"kernel-team@android.com" <kernel-team@android.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Bean Huo <beanhuo@micron.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] scsi: ufs: Fix some problems in task management request implementation
Date: Mon, 25 Jan 2021 11:36:01 +0000	[thread overview]
Message-ID: <DM6PR04MB65754B0CF70B9A3EB036D157FCBD9@DM6PR04MB6575.namprd04.prod.outlook.com> (raw)
In-Reply-To: <1611199388-24668-1-git-send-email-cang@codeaurora.org>

 
> 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

  reply	other threads:[~2021-01-26 19:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21  3:23 [PATCH v2] scsi: ufs: Fix some problems in task management request implementation Can Guo
2021-01-25 11:36 ` Avri Altman [this message]
2021-01-26  2:10   ` Can Guo
2021-01-26  5:05 ` Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR04MB65754B0CF70B9A3EB036D157FCBD9@DM6PR04MB6575.namprd04.prod.outlook.com \
    --to=avri.altman@wdc.com \
    --cc=alim.akhtar@samsung.com \
    --cc=asutoshd@codeaurora.org \
    --cc=beanhuo@micron.com \
    --cc=cang@codeaurora.org \
    --cc=hongwus@codeaurora.org \
    --cc=jaegeuk@kernel.org \
    --cc=jejb@linux.ibm.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nguyenb@codeaurora.org \
    --cc=stanley.chu@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).