From: James Bottomley <jejb@linux.ibm.com>
To: Can Guo <cang@codeaurora.org>,
asutoshd@codeaurora.org, nguyenb@codeaurora.org,
hongwus@codeaurora.org, ziqichen@codeaurora.org,
rnayak@codeaurora.org, linux-scsi@vger.kernel.org,
kernel-team@android.com, saravanak@google.com,
salyzyn@google.com
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
Avri Altman <avri.altman@wdc.com>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
Stanley Chu <stanley.chu@mediatek.com>,
Bean Huo <beanhuo@micron.com>,
Bart Van Assche <bvanassche@acm.org>,
open list <linux-kernel@vger.kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-arm-kernel@lists.infradead.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell
Date: Tue, 08 Sep 2020 22:05:06 -0700 [thread overview]
Message-ID: <1599627906.10803.65.camel@linux.ibm.com> (raw)
In-Reply-To: <1599099873-32579-2-git-send-email-cang@codeaurora.org>
I can't reconcile this hunk:
On Wed, 2020-09-02 at 19:24 -0700, Can Guo wrote:
> @@ -6504,6 +6505,80 @@ static void ufshcd_set_req_abort_skip(struct
> ufs_hba *hba, unsigned long bitmap)
> * issued. To avoid that, first issue UFS_QUERY_TASK to check if the
> command is
> * really issued and then try to abort it.
> *
> + * Returns zero on success, non-zero on failure
> + */
> +static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag)
> +{
> + struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> + int err = 0;
> + int poll_cnt;
> + u8 resp = 0xF;
> + u32 reg;
> +
> + for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> + UFS_QUERY_TASK, &resp);
> + if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> + /* cmd pending in the device */
> + dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> + __func__, tag);
> + break;
> + } else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> + /*
> + * cmd not pending in the device, check if
> it is
> + * in transition.
> + */
> + dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> + __func__, tag);
> + reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> + if (reg & (1 << tag)) {
> + /* sleep for max. 200us to stabilize
> */
> + usleep_range(100, 200);
> + continue;
> + }
> + /* command completed already */
> + dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> + __func__, tag);
> + goto out;
> + } else {
> + dev_err(hba->dev,
> + "%s: no response from device. tag =
> %d, err %d\n",
> + __func__, tag, err);
> + if (!err)
> + err = resp; /* service response
> error */
> + goto out;
> + }
> + }
> +
> + if (!poll_cnt) {
> + err = -EBUSY;
> + goto out;
> + }
> +
> + err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> + UFS_ABORT_TASK, &resp);
> + if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> + if (!err) {
> + err = resp; /* service response error */
> + dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> + __func__, tag, err);
> + }
> + goto out;
> + }
> +
> + err = ufshcd_clear_cmd(hba, tag);
> + if (err)
> + dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> + __func__, tag, err);
> +
> +out:
> + return err;
> +}
> +
> +/**
> + * ufshcd_abort - scsi host template eh_abort_handler callback
> + * @cmd: SCSI command pointer
> + *
> * Returns SUCCESS/FAILED
> */
> static int ufshcd_abort(struct scsi_cmnd *cmd)
> @@ -6513,8 +6588,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> unsigned long flags;
> unsigned int tag;
> int err = 0;
> - int poll_cnt;
> - u8 resp = 0xF;
> struct ufshcd_lrb *lrbp;
> u32 reg;
>
> @@ -6583,63 +6656,9 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> goto out;
> }
>
> - for (poll_cnt = 100; poll_cnt; poll_cnt--) {
> - err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp-
> >task_tag,
> - UFS_QUERY_TASK, &resp);
> - if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_SUCCEEDED) {
> - /* cmd pending in the device */
> - dev_err(hba->dev, "%s: cmd pending in the
> device. tag = %d\n",
> - __func__, tag);
> - break;
> - } else if (!err && resp ==
> UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - /*
> - * cmd not pending in the device, check if
> it is
> - * in transition.
> - */
> - dev_err(hba->dev, "%s: cmd at tag %d not
> pending in the device.\n",
> - __func__, tag);
> - reg = ufshcd_readl(hba,
> REG_UTP_TRANSFER_REQ_DOOR_BELL);
> - if (reg & (1 << tag)) {
> - /* sleep for max. 200us to stabilize
> */
> - usleep_range(100, 200);
> - continue;
> - }
> - /* command completed already */
> - dev_err(hba->dev, "%s: cmd at tag %d
> successfully cleared from DB.\n",
> - __func__, tag);
> - goto out;
> - } else {
> - dev_err(hba->dev,
> - "%s: no response from device. tag =
> %d, err %d\n",
> - __func__, tag, err);
> - if (!err)
> - err = resp; /* service response
> error */
> - goto out;
> - }
> - }
> -
> - if (!poll_cnt) {
> - err = -EBUSY;
> - goto out;
> - }
> -
> - err = ufshcd_issue_tm_cmd(hba, lrbp->lun, lrbp->task_tag,
> - UFS_ABORT_TASK, &resp);
> - if (err || resp != UPIU_TASK_MANAGEMENT_FUNC_COMPL) {
> - if (!err) {
> - err = resp; /* service response error */
> - dev_err(hba->dev, "%s: issued. tag = %d, err
> %d\n",
> - __func__, tag, err);
> - }
> - goto out;
> - }
> -
> - err = ufshcd_clear_cmd(hba, tag);
> - if (err) {
> - dev_err(hba->dev, "%s: Failed clearing cmd at tag
> %d, err %d\n",
> - __func__, tag, err);
> + err = ufshcd_try_to_abort_task(hba, tag);
> + if (err)
> goto out;
> - }
>
> spin_lock_irqsave(host->host_lock, flags);
> __ufshcd_transfer_req_compl(hba, (1UL << tag));
With the change in this fix:
commit b10178ee7fa88b68a9e8adc06534d2605cb0ec23
Author: Stanley Chu <stanley.chu@mediatek.com>
Date: Tue Aug 11 16:18:58 2020 +0200
scsi: ufs: Clean up completed request without interrupt
notification
It looks like there have to be two separate error returns from your new
ufshcd_try_to_abort_function() so it knows to continue with
usfhcd_transfer_req_complete(), or the whole function needs to be
refactored, but if this goes upstream as is it looks like it will
eliminate the bug fix.
James
next prev parent reply other threads:[~2020-09-09 5:05 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 2:24 [PATCH v2 0/2] Add UFS LINERESET handling Can Guo
2020-09-03 2:24 ` [PATCH v2 1/2] scsi: ufs: Abort tasks before clear them from doorbell Can Guo
2020-09-09 5:05 ` James Bottomley [this message]
2020-09-10 2:32 ` Martin K. Petersen
2020-09-10 2:48 ` Stanley Chu
2020-09-10 6:18 ` James Bottomley
2020-09-10 8:18 ` Stanley Chu
2020-09-10 16:09 ` James Bottomley
2020-09-11 2:16 ` Can Guo
2020-09-11 9:09 ` Bean Huo
2020-09-14 5:00 ` Can Guo
2020-09-15 3:14 ` Can Guo
2020-09-15 20:21 ` Martin K. Petersen
2020-09-16 6:34 ` Can Guo
2020-09-03 2:24 ` [PATCH v2 2/2] scsi: ufs: Handle LINERESET indication in err handler Can Guo
2020-09-09 2:09 ` [PATCH v2 0/2] Add UFS LINERESET handling Martin K. Petersen
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=1599627906.10803.65.camel@linux.ibm.com \
--to=jejb@linux.ibm.com \
--cc=alim.akhtar@samsung.com \
--cc=asutoshd@codeaurora.org \
--cc=avri.altman@wdc.com \
--cc=beanhuo@micron.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=hongwus@codeaurora.org \
--cc=kernel-team@android.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=matthias.bgg@gmail.com \
--cc=nguyenb@codeaurora.org \
--cc=rnayak@codeaurora.org \
--cc=salyzyn@google.com \
--cc=saravanak@google.com \
--cc=stanley.chu@mediatek.com \
--cc=ziqichen@codeaurora.org \
/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).