public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michael.christie@oracle.com>
To: mwilck@suse.com, john.g.garry@oracle.com, bvanassche@acm.org,
	hch@lst.de, martin.petersen@oracle.com,
	linux-scsi@vger.kernel.org,
	james.bottomley@hansenpartnership.com
Cc: Mike Christie <michael.christie@oracle.com>
Subject: [PATCH v12 06/20] scsi: Have scsi-ml retry sd_spinup_disk errors
Date: Mon, 13 Nov 2023 19:37:36 -0600	[thread overview]
Message-ID: <20231114013750.76609-7-michael.christie@oracle.com> (raw)
In-Reply-To: <20231114013750.76609-1-michael.christie@oracle.com>

This simplifies sd_spinup_disk so scsi-ml retries errors for it. Note that
we retried specifically on a UA and also if scsi_status_is_good returned
failed which would happen for all check conditions. In this patch we use
SCMD_FAILURE_STAT_ANY which will trigger for the same conditions as
when scsi_status_is_good returns false and there is status. This will
cover all CCs including UAs so there is no explicit failures arrary entry
for UAs.

There is one behavior change where we no longer retry when
scsi_execute_cmd returns < 0, but we should be ok. We don't need to retry
for failures like the queue being removed, and for the case where there
are no tags/reqs the block layer waits/retries for us. For possible memory
allocation failures from blk_rq_map_kern we use GFP_NOIO, so retrying
will probably not help.

We do not handle the outside loop's retries because we want to sleep
between tries and we don't support that yet.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/sd.c | 77 +++++++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 641f9c9c0674..cda0d029ab7f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2235,55 +2235,68 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 static void
 sd_spinup_disk(struct scsi_disk *sdkp)
 {
-	unsigned char cmd[10];
+	static const u8 cmd[10] = { TEST_UNIT_READY };
 	unsigned long spintime_expire = 0;
-	int retries, spintime;
+	int spintime, sense_valid = 0;
 	unsigned int the_result;
 	struct scsi_sense_hdr sshdr;
+	struct scsi_failure failure_defs[] = {
+		/* Do not retry Medium Not Present */
+		{
+			.sense = UNIT_ATTENTION,
+			.asc = 0x3A,
+			.ascq = SCMD_FAILURE_ASCQ_ANY,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{
+			.sense = NOT_READY,
+			.asc = 0x3A,
+			.ascq = SCMD_FAILURE_ASCQ_ANY,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		/* Retry when scsi_status_is_good would return false 3 times */
+		{
+			.result = SCMD_FAILURE_STAT_ANY,
+			.allowed = 3,
+		},
+		{}
+	};
+	struct scsi_failures failures = {
+		.failure_definitions = failure_defs,
+	};
 	const struct scsi_exec_args exec_args = {
 		.sshdr = &sshdr,
+		.failures = &failures,
 	};
-	int sense_valid = 0;
 
 	spintime = 0;
 
 	/* Spin up drives, as required.  Only do this at boot time */
 	/* Spinup needs to be done for module loads too. */
 	do {
-		retries = 0;
+		bool media_was_present = sdkp->media_present;
 
-		do {
-			bool media_was_present = sdkp->media_present;
+		scsi_reset_failures(&failures);
 
-			cmd[0] = TEST_UNIT_READY;
-			memset((void *) &cmd[1], 0, 9);
+		the_result = scsi_execute_cmd(sdkp->device, cmd, REQ_OP_DRV_IN,
+					      NULL, 0, SD_TIMEOUT,
+					      sdkp->max_retries, &exec_args);
 
-			the_result = scsi_execute_cmd(sdkp->device, cmd,
-						      REQ_OP_DRV_IN, NULL, 0,
-						      SD_TIMEOUT,
-						      sdkp->max_retries,
-						      &exec_args);
 
-			if (the_result > 0) {
-				/*
-				 * If the drive has indicated to us that it
-				 * doesn't have any media in it, don't bother
-				 * with any more polling.
-				 */
-				if (media_not_present(sdkp, &sshdr)) {
-					if (media_was_present)
-						sd_printk(KERN_NOTICE, sdkp,
-							  "Media removed, stopped polling\n");
-					return;
-				}
-
-				sense_valid = scsi_sense_valid(&sshdr);
+		if (the_result > 0) {
+			/*
+			 * If the drive has indicated to us that it doesn't
+			 * have any media in it, don't bother with any more
+			 * polling.
+			 */
+			if (media_not_present(sdkp, &sshdr)) {
+				if (media_was_present)
+					sd_printk(KERN_NOTICE, sdkp,
+						  "Media removed, stopped polling\n");
+				return;
 			}
-			retries++;
-		} while (retries < 3 &&
-			 (!scsi_status_is_good(the_result) ||
-			  (scsi_status_is_check_condition(the_result) &&
-			  sense_valid && sshdr.sense_key == UNIT_ATTENTION)));
+			sense_valid = scsi_sense_valid(&sshdr);
+		}
 
 		if (!scsi_status_is_check_condition(the_result)) {
 			/* no sense, TUR either succeeded or failed
-- 
2.34.1


  parent reply	other threads:[~2023-11-14  1:39 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-14  1:37 [PATCH v12 00/20] scsi: Allow scsi_execute users to request retries Mike Christie
2023-11-14  1:37 ` [PATCH v12 01/20] scsi: Allow passthrough to request scsi-ml retries Mike Christie
2023-11-16 10:53   ` John Garry
2023-11-14  1:37 ` [PATCH v12 02/20] scsi: Have scsi-ml retry scsi_probe_lun errors Mike Christie
2023-11-16 11:14   ` John Garry
2023-11-16 16:38     ` Mike Christie
2023-11-14  1:37 ` [PATCH v12 03/20] scsi: retry INQUIRY after timeout Mike Christie
2023-11-14  1:37 ` [PATCH v12 04/20] scsi: sd: Have scsi-ml retry read_capacity_16 errors Mike Christie
2023-11-16 11:39   ` John Garry
2023-11-16 17:15     ` Mike Christie
2023-11-16 17:57       ` Martin Wilck
2023-11-14  1:37 ` [PATCH v12 05/20] scsi: Use separate buf for START_STOP in sd_spinup_disk Mike Christie
2023-11-14  1:37 ` Mike Christie [this message]
2023-11-16 12:13   ` [PATCH v12 06/20] scsi: Have scsi-ml retry sd_spinup_disk errors John Garry
2023-11-16 16:44     ` Mike Christie
2023-11-14  1:37 ` [PATCH v12 07/20] scsi: hp_sw: Have scsi-ml retry scsi_execute_cmd errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 08/20] scsi: rdac: Have scsi-ml retry send_mode_select errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 09/20] scsi: spi: Have scsi-ml retry spi_execute UAs Mike Christie
2023-11-14  1:37 ` [PATCH v12 10/20] scsi: sd: Have scsi-ml retry sd_sync_cache errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 11/20] scsi: ch: Remove unit_attention Mike Christie
2023-11-14  1:37 ` [PATCH v12 12/20] scsi: ch: Have scsi-ml retry ch_do_scsi UAs Mike Christie
2023-11-14  1:37 ` [PATCH v12 13/20] scsi: Have scsi-ml retry scsi_mode_sense UAs Mike Christie
2023-11-14  1:37 ` [PATCH v12 14/20] scsi: Have scsi-ml retry scsi_report_lun_scan errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 15/20] scsi: sd: Have pr commands retry UAs Mike Christie
2023-11-14  1:37 ` [PATCH v12 16/20] scsi: sd: Have scsi-ml retry read_capacity_10 errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 17/20] scsi: ses: Have scsi-ml retry scsi_execute_cmd errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 18/20] scsi: sr: Have scsi-ml retry get_sectorsize errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 19/20] scsi: ufs: Have scsi-ml retry start stop errors Mike Christie
2023-11-14  1:37 ` [PATCH v12 20/20] scsi: Add kunit tests for scsi_check_passthrough Mike Christie
2023-11-14 18:38   ` Bart Van Assche
2023-11-16 11:02 ` [PATCH v12 00/20] scsi: Allow scsi_execute users to request retries John Garry
2023-11-16 16:29   ` Mike Christie

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=20231114013750.76609-7-michael.christie@oracle.com \
    --to=michael.christie@oracle.com \
    --cc=bvanassche@acm.org \
    --cc=hch@lst.de \
    --cc=james.bottomley@hansenpartnership.com \
    --cc=john.g.garry@oracle.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mwilck@suse.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