public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Stanley Chu <stanley.chu@mediatek.com>
To: linux-scsi@vger.kernel.org, martin.petersen@oracle.com,
	avri.altman@wdc.com, alim.akhtar@samsung.com,
	pedrom.sousa@synopsys.com
Cc: marc.w.gonzalez@free.fr, andy.teng@mediatek.com,
	chun-hung.wu@mediatek.com, kuohong.wang@mediatek.com,
	evgreen@chromium.org, linux-mediatek@lists.infradead.org,
	peter.wang@mediatek.com, matthias.bgg@gmail.com,
	Stanley Chu <stanley.chu@mediatek.com>,
	linux-arm-kernel@lists.infradead.org, beanhuo@micron.com
Subject: [PATCH v1 2/2] scsi: ufs: Fix broken hba->outstanding_tasks
Date: Fri, 12 Jul 2019 12:44:16 +0800	[thread overview]
Message-ID: <1562906656-27154-3-git-send-email-stanley.chu@mediatek.com> (raw)
In-Reply-To: <1562906656-27154-1-git-send-email-stanley.chu@mediatek.com>

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 consumpton 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.

Because error handling flow, i.e., ufshcd_reset_and_restore(), will be
triggered after any task management command times out, we fix this by
clearing corresponding hba->outstanding_tasks bits during this flow.
To achieve this, we need a mask to track timed-out commands and thus
error handling flow can clear their tags specifically.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 38 +++++++++++++++++++++++++++++++-------
 drivers/scsi/ufs/ufshcd.h |  1 +
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a667dbb547f2..f780066edf26 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -731,7 +731,6 @@ static inline void ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag)
 static inline void ufshcd_outstanding_task_clear(struct ufs_hba *hba, int tag)
 {
 	__clear_bit(tag, &hba->outstanding_tasks);
-	dev_info(hba->dev, "clear outstanding_tasks: %d\n", tag);
 }
 
 /**
@@ -5540,11 +5539,34 @@ static void ufshcd_check_errors(struct ufs_hba *hba)
  */
 static void ufshcd_tmc_handler(struct ufs_hba *hba)
 {
-	u32 tm_doorbell;
+	u32 tm_doorbell, tag;
+	unsigned long tm_err_handled = 0;
+	unsigned long tm_done;
 
 	tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
 	hba->tm_condition = tm_doorbell ^ hba->outstanding_tasks;
-	wake_up(&hba->tm_wq);
+	tm_done = hba->tm_condition;
+
+	/* clean resources for timed-out tasks */
+	for_each_set_bit(tag, &hba->tm_condition, hba->nutmrs) {
+		if (test_and_clear_bit(tag, &hba->tm_slots_err)) {
+			clear_bit(tag, &hba->tm_condition);
+			ufshcd_put_tm_slot(hba, tag);
+			ufshcd_outstanding_task_clear(hba, tag);
+			__set_bit(tag, &tm_err_handled);
+		}
+	}
+
+	/*
+	 * Now tag waiters can get free tags if tags were occupied
+	 * by timed-out tasks
+	 */
+	if (tm_err_handled)
+		wake_up(&hba->tm_tag_wq);
+
+	/* if we have normal tasks, they shall have post-processing */
+	if (tm_err_handled != tm_done)
+		wake_up(&hba->tm_wq);
 }
 
 /**
@@ -5682,6 +5704,7 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba,
 		if (ufshcd_clear_tm_cmd(hba, free_slot))
 			dev_WARN(hba->dev, "%s: unable clear tm cmd (slot %d) after timeout\n",
 					__func__, free_slot);
+		set_bit(free_slot, &hba->tm_slots_err);
 		err = -ETIMEDOUT;
 	} else {
 		err = 0;
@@ -5692,12 +5715,13 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba,
 		spin_lock_irqsave(hba->host->host_lock, flags);
 		ufshcd_outstanding_task_clear(hba, free_slot);
 		spin_unlock_irqrestore(hba->host->host_lock, flags);
-
 	}
 
-	clear_bit(free_slot, &hba->tm_condition);
-	ufshcd_put_tm_slot(hba, free_slot);
-	wake_up(&hba->tm_tag_wq);
+	if (!(test_bit(free_slot, &hba->tm_slots_err))) {
+		clear_bit(free_slot, &hba->tm_condition);
+		ufshcd_put_tm_slot(hba, free_slot);
+		wake_up(&hba->tm_tag_wq);
+	}
 
 	ufshcd_release(hba);
 	return err;
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index a43c7135f33d..4e4dfa6e233c 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -645,6 +645,7 @@ struct ufs_hba {
 	wait_queue_head_t tm_tag_wq;
 	unsigned long tm_condition;
 	unsigned long tm_slots_in_use;
+	unsigned long tm_slots_err;
 
 	struct uic_command *active_uic_cmd;
 	struct mutex uic_cmd_mutex;
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2019-07-12  4:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12  4:44 [PATCH v1 0/2] scsi: ufs: Fix broken hba->outstanding_tasks Stanley Chu
2019-07-12  4:44 ` [PATCH v1 1/2] scsi: ufs: Make new function for clearing outstanding task bits Stanley Chu
2019-07-12  4:44 ` Stanley Chu [this message]
2019-07-18  5:21 ` [PATCH v1 0/2] scsi: ufs: Fix broken hba->outstanding_tasks Stanley Chu
2019-07-21 12:46 ` Avri Altman
     [not found]   ` <SN6PR04MB4925208835D4760249E82DB7FCC50-UKdxhu0+N/VnT3GYGerMaFM8qxBPnqtHvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-07-21 12:52     ` Avri Altman
2019-07-22  6:10   ` Avri Altman
2019-07-24  5:08     ` Stanley Chu
2019-07-24  7:09       ` Avri Altman

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=1562906656-27154-3-git-send-email-stanley.chu@mediatek.com \
    --to=stanley.chu@mediatek.com \
    --cc=alim.akhtar@samsung.com \
    --cc=andy.teng@mediatek.com \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=chun-hung.wu@mediatek.com \
    --cc=evgreen@chromium.org \
    --cc=kuohong.wang@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=marc.w.gonzalez@free.fr \
    --cc=martin.petersen@oracle.com \
    --cc=matthias.bgg@gmail.com \
    --cc=pedrom.sousa@synopsys.com \
    --cc=peter.wang@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