All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: "Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-scsi@vger.kernel.org,
	Adrian Hunter <adrian.hunter@intel.com>,
	Bart Van Assche <bvanassche@acm.org>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Bean Huo <beanhuo@micron.com>, Avri Altman <avri.altman@wdc.com>,
	Jinyoung Choi <j-young.choi@samsung.com>
Subject: [PATCH v3 8/8] scsi: ufs: Fix a deadlock between PM and the SCSI error handler
Date: Thu, 29 Sep 2022 15:00:21 -0700	[thread overview]
Message-ID: <20220929220021.247097-9-bvanassche@acm.org> (raw)
In-Reply-To: <20220929220021.247097-1-bvanassche@acm.org>

The following deadlock has been observed on multiple test setups:
* ufshcd_wl_suspend() is waiting for blk_execute_rq() to complete while it
  holds host_sem.
* ufshcd_eh_host_reset_handler() invokes ufshcd_err_handler() and the
  latter function tries to obtain host_sem.
This is a deadlock because blk_execute_rq() can't execute SCSI commands
while the host is in the SHOST_RECOVERY state and because the error
handler cannot make progress either.

Fix this deadlock as follows:
* Fail attempts to suspend the system while the SCSI error handler is in
  progress.
* If the system is suspending and a START STOP UNIT command times out,
  handle the SCSI command timeout from inside the context of the SCSI
  timeout handler instead of activating the SCSI error handler.
* Reduce the START STOP UNIT command timeout to one second since on
  Android devices a kernel panic is triggered if an attempt to suspend
  the system takes more than 20 seconds. One second should be enough for
  the START STOP UNIT command since this command completes in less than
  a millisecond for the UFS devices I have access to.

The runtime power management code is not affected by this deadlock since
hba->host_sem is not touched by the runtime power management functions
in the UFS driver.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/ufs/core/ufshcd.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 5507d93a4bba..0294b51e776a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8295,6 +8295,26 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 	}
 }
 
+static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
+{
+	struct ufs_hba *hba = shost_priv(scmd->device->host);
+
+	if (!hba->system_suspending) {
+		/* Activate the error handler in the SCSI core. */
+		return SCSI_EH_NOT_HANDLED;
+	}
+
+	/*
+	 * Handle errors directly to prevent a deadlock between
+	 * ufshcd_set_dev_pwr_mode() and ufshcd_err_handler().
+	 */
+	ufshcd_link_recovery(hba);
+	dev_info(hba->dev, "%s() finished; outstanding_tasks = %#lx.\n",
+		 __func__, hba->outstanding_tasks);
+
+	return hba->outstanding_tasks ? SCSI_EH_RESET_TIMER : SCSI_EH_DONE;
+}
+
 static const struct attribute_group *ufshcd_driver_groups[] = {
 	&ufs_sysfs_unit_descriptor_group,
 	&ufs_sysfs_lun_attributes_group,
@@ -8329,6 +8349,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.eh_abort_handler	= ufshcd_abort,
 	.eh_device_reset_handler = ufshcd_eh_device_reset_handler,
 	.eh_host_reset_handler   = ufshcd_eh_host_reset_handler,
+	.eh_timed_out		= ufshcd_eh_timed_out,
 	.this_id		= -1,
 	.sg_tablesize		= SG_ALL,
 	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
@@ -8783,7 +8804,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
 	 */
 	for (retries = 3; retries > 0; --retries) {
 		ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
-				START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
+				   1 * HZ, 0, REQ_FAILFAST_DEV, RQF_PM, NULL);
 		if (ret < 0)
 			break;
 		if (host_byte(ret) == 0 && scsi_status_is_good(ret))

  parent reply	other threads:[~2022-09-29 22:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 22:00 [PATCH v3 0/8] Fix a deadlock in the UFS driver Bart Van Assche
2022-09-29 22:00 ` [PATCH v3 1/8] scsi: core: Fix a race between scsi_done() and scsi_timeout() Bart Van Assche
2022-09-30  0:17   ` Mike Christie
2022-09-30  0:32     ` Bart Van Assche
2022-09-29 22:00 ` [PATCH v3 2/8] scsi: core: Change the return type of .eh_timed_out() Bart Van Assche
2022-09-29 22:00 ` [PATCH v3 3/8] scsi: core: Support failing requests while recovering Bart Van Assche
2022-10-03 17:27   ` Mike Christie
2022-10-03 19:20     ` Bart Van Assche
2022-09-29 22:00 ` [PATCH v3 4/8] scsi: ufs: Remove an outdated comment Bart Van Assche
2022-09-30 10:26   ` Bean Huo
2022-10-03  5:55   ` Adrian Hunter
2022-09-29 22:00 ` [PATCH v3 5/8] scsi: ufs: Use 'else' in ufshcd_set_dev_pwr_mode() Bart Van Assche
2022-09-30 10:28   ` Bean Huo
2022-10-03  5:58   ` Adrian Hunter
2022-09-29 22:00 ` [PATCH v3 6/8] scsi: ufs: Try harder to change the power mode Bart Van Assche
2022-09-30 12:14   ` Bean Huo
2022-10-03  6:10   ` Adrian Hunter
2022-10-03 16:38     ` Bart Van Assche
2022-09-29 22:00 ` [PATCH v3 7/8] scsi: ufs: Track system suspend / resume activity Bart Van Assche
2022-10-03  6:10   ` Adrian Hunter
2022-09-29 22:00 ` Bart Van Assche [this message]
2022-09-30 15:03   ` [PATCH v3 8/8] scsi: ufs: Fix a deadlock between PM and the SCSI error handler Bean Huo
2022-09-30 17:15     ` Bart Van Assche
2022-10-02 21:21       ` Bean Huo
2022-10-04  0:21         ` Bart Van Assche
2022-10-03  6:18   ` Adrian Hunter
2022-10-03 16:45     ` 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=20220929220021.247097-9-bvanassche@acm.org \
    --to=bvanassche@acm.org \
    --cc=adrian.hunter@intel.com \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=j-young.choi@samsung.com \
    --cc=jaegeuk@kernel.org \
    --cc=jejb@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.