public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: "Martin K . Petersen" <martin.petersen@oracle.com>
Cc: "James E . J . Bottomley" <jejb@linux.vnet.ibm.com>,
	Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	linux-scsi@vger.kernel.org, linux-block@vger.kernel.org,
	Bart Van Assche <bvanassche@acm.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	Woody Suwalski <terraluna977@gmail.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Can Guo <cang@codeaurora.org>,
	Stanley Chu <stanley.chu@mediatek.com>,
	Ming Lei <ming.lei@redhat.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Stan Johnson <userm57@yahoo.com>
Subject: [PATCH v2 7/9] scsi_transport_spi: Freeze request queues instead of quiescing
Date: Sun, 15 Nov 2020 19:04:57 -0800	[thread overview]
Message-ID: <20201116030459.13963-8-bvanassche@acm.org> (raw)
In-Reply-To: <20201116030459.13963-1-bvanassche@acm.org>

Instead of quiescing the request queues involved in domain validation,
freeze these. As a result, the struct request_queue pm_only member is no
longer set during domain validation. That will allow to modify
scsi_execute() such that it stops setting the BLK_MQ_REQ_PREEMPT flag.
Three additional changes in this patch are that scsi_mq_alloc_queue() is
exported, that scsi_device_quiesce() is no longer exported and that
scsi_target_{quiesce,resume}() have been changed into
scsi_target_{freeze,unfreeze}().

Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Woody Suwalski <terraluna977@gmail.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Cc: Can Guo <cang@codeaurora.org>
Cc: Stanley Chu <stanley.chu@mediatek.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c           | 22 +++++-----
 drivers/scsi/scsi_priv.h          |  2 +
 drivers/scsi/scsi_transport_spi.c | 72 ++++++++++++++++++++-----------
 include/scsi/scsi_device.h        |  6 +--
 4 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ff480fa6261e..df1f22b32964 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1893,6 +1893,7 @@ struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
 	blk_queue_flag_set(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
 	return q;
 }
+EXPORT_SYMBOL_GPL(scsi_mq_alloc_queue);
 
 int scsi_mq_setup_tags(struct Scsi_Host *shost)
 {
@@ -2568,7 +2569,6 @@ scsi_device_quiesce(struct scsi_device *sdev)
 
 	return err;
 }
-EXPORT_SYMBOL(scsi_device_quiesce);
 
 /**
  *	scsi_device_resume - Restart user issued commands to a quiesced device.
@@ -2597,30 +2597,30 @@ void scsi_device_resume(struct scsi_device *sdev)
 EXPORT_SYMBOL(scsi_device_resume);
 
 static void
-device_quiesce_fn(struct scsi_device *sdev, void *data)
+device_freeze_fn(struct scsi_device *sdev, void *data)
 {
-	scsi_device_quiesce(sdev);
+	blk_mq_freeze_queue(sdev->request_queue);
 }
 
 void
-scsi_target_quiesce(struct scsi_target *starget)
+scsi_target_freeze(struct scsi_target *starget)
 {
-	starget_for_each_device(starget, NULL, device_quiesce_fn);
+	starget_for_each_device(starget, NULL, device_freeze_fn);
 }
-EXPORT_SYMBOL(scsi_target_quiesce);
+EXPORT_SYMBOL(scsi_target_freeze);
 
 static void
-device_resume_fn(struct scsi_device *sdev, void *data)
+device_unfreeze_fn(struct scsi_device *sdev, void *data)
 {
-	scsi_device_resume(sdev);
+	blk_mq_unfreeze_queue(sdev->request_queue);
 }
 
 void
-scsi_target_resume(struct scsi_target *starget)
+scsi_target_unfreeze(struct scsi_target *starget)
 {
-	starget_for_each_device(starget, NULL, device_resume_fn);
+	starget_for_each_device(starget, NULL, device_unfreeze_fn);
 }
-EXPORT_SYMBOL(scsi_target_resume);
+EXPORT_SYMBOL(scsi_target_unfreeze);
 
 /**
  * scsi_internal_device_block_nowait - try to transition to the SDEV_BLOCK state
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 180636d54982..3c64929291c5 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -96,6 +96,8 @@ extern int scsi_mq_setup_tags(struct Scsi_Host *shost);
 extern void scsi_mq_destroy_tags(struct Scsi_Host *shost);
 extern void scsi_exit_queue(void);
 extern void scsi_evt_thread(struct work_struct *work);
+extern int scsi_device_quiesce(struct scsi_device *sdev);
+extern void scsi_device_resume(struct scsi_device *sdev);
 struct request_queue;
 struct request;
 
diff --git a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
index 959990f66865..f0ef9ab008c5 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -997,59 +997,79 @@ void
 spi_dv_device(struct scsi_device *sdev)
 {
 	struct scsi_target *starget = sdev->sdev_target;
+	struct request_queue *q2;
 	u8 *buffer;
 	const int len = SPI_MAX_ECHO_BUFFER_SIZE*2;
 
 	/*
-	 * Because this function and the power management code both call
-	 * scsi_device_quiesce(), it is not safe to perform domain validation
-	 * while suspend or resume is in progress. Hence the
-	 * lock/unlock_system_sleep() calls.
+	 * Because this function creates a new request queue that is not
+	 * visible to the rest of the system, this function must be serialized
+	 * against suspend, resume and runtime power management. Hence the
+	 * lock/unlock_system_sleep() and scsi_autopm_{get,put}_device()
+	 * calls.
 	 */
 	lock_system_sleep();
 
+	if (scsi_autopm_get_device(sdev))
+		goto unlock_system_sleep;
+
 	if (unlikely(spi_dv_in_progress(starget)))
-		goto unlock;
+		goto put_autopm;
 
 	if (unlikely(scsi_device_get(sdev)))
-		goto unlock;
-
-	spi_dv_in_progress(starget) = 1;
+		goto put_autopm;
 
 	buffer = kzalloc(len, GFP_KERNEL);
 
 	if (unlikely(!buffer))
-		goto out_put;
-
-	/* We need to verify that the actual device will quiesce; the
-	 * later target quiesce is just a nice to have */
-	if (unlikely(scsi_device_quiesce(sdev)))
-		goto out_free;
-
-	scsi_target_quiesce(starget);
+		goto put_sdev;
 
 	spi_dv_pending(starget) = 1;
+
 	mutex_lock(&spi_dv_mutex(starget));
+	if (unlikely(spi_dv_in_progress(starget)))
+		goto clear_pending;
+
+	spi_dv_in_progress(starget) = 1;
 
 	starget_printk(KERN_INFO, starget, "Beginning Domain Validation\n");
 
-	spi_dv_device_internal(sdev, sdev->request_queue, buffer);
+	q2 = scsi_mq_alloc_queue(sdev);
+
+	if (q2) {
+		/*
+		 * Freeze the target such that no other subsystem can submit
+		 * SCSI commands to 'sdev'. Submitting SCSI commands through
+		 * q2 may trigger the SCSI error handler. The SCSI error
+		 * handler must be able to handle a frozen sdev->request_queue
+		 * and must also use blk_mq_rq_from_pdu(q2)->q instead of
+		 * sdev->request_queue if it would be necessary to access q2
+		 * directly.
+		 */
+		scsi_target_freeze(starget);
+		spi_dv_device_internal(sdev, q2, buffer);
+		blk_cleanup_queue(q2);
+		scsi_target_unfreeze(starget);
+	}
 
 	starget_printk(KERN_INFO, starget, "Ending Domain Validation\n");
 
-	mutex_unlock(&spi_dv_mutex(starget));
-	spi_dv_pending(starget) = 0;
-
-	scsi_target_resume(starget);
-
 	spi_initial_dv(starget) = 1;
+	spi_dv_in_progress(starget) = 0;
+
+clear_pending:
+	spi_dv_pending(starget) = 0;
+	mutex_unlock(&spi_dv_mutex(starget));
 
- out_free:
 	kfree(buffer);
- out_put:
-	spi_dv_in_progress(starget) = 0;
+
+put_sdev:
 	scsi_device_put(sdev);
-unlock:
+
+put_autopm:
+	scsi_autopm_put_device(sdev);
+
+unlock_system_sleep:
 	unlock_system_sleep();
 }
 EXPORT_SYMBOL(spi_dv_device);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index f47fdf9cf788..dc193d7f479a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -423,10 +423,8 @@ extern struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
 extern void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt);
 extern void sdev_evt_send_simple(struct scsi_device *sdev,
 			  enum scsi_device_event evt_type, gfp_t gfpflags);
-extern int scsi_device_quiesce(struct scsi_device *sdev);
-extern void scsi_device_resume(struct scsi_device *sdev);
-extern void scsi_target_quiesce(struct scsi_target *);
-extern void scsi_target_resume(struct scsi_target *);
+extern void scsi_target_freeze(struct scsi_target *);
+extern void scsi_target_unfreeze(struct scsi_target *);
 extern void scsi_scan_target(struct device *parent, unsigned int channel,
 			     unsigned int id, u64 lun,
 			     enum scsi_scan_mode rescan);

  parent reply	other threads:[~2020-11-16  3:05 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16  3:04 [PATCH v2 0/9] Rework runtime suspend and SCSI domain validation Bart Van Assche
2020-11-16  3:04 ` [PATCH v2 1/9] block: Fix a race in the runtime power management code Bart Van Assche
2020-11-16 17:14   ` Christoph Hellwig
2020-11-16  3:04 ` [PATCH v2 2/9] ide: Do not set the RQF_PREEMPT flag for sense requests Bart Van Assche
2020-11-16 17:14   ` Christoph Hellwig
2020-11-16  3:04 ` [PATCH v2 3/9] scsi: Pass a request queue pointer to __scsi_execute() Bart Van Assche
2020-11-16 17:16   ` Christoph Hellwig
2020-11-18  1:16   ` Can Guo
2020-11-16  3:04 ` [PATCH v2 4/9] scsi: Rework scsi_mq_alloc_queue() Bart Van Assche
2020-11-16 17:17   ` Christoph Hellwig
2020-11-16 18:01     ` Bart Van Assche
2020-11-20  1:36       ` Bart Van Assche
2020-11-18  1:15   ` Can Guo
2020-11-16  3:04 ` [PATCH v2 5/9] scsi: Do not wait for a request in scsi_eh_lock_door() Bart Van Assche
2020-11-16 17:18   ` Christoph Hellwig
2020-11-16  3:04 ` [PATCH v2 6/9] scsi_transport_spi: Make spi_execute() accept a request queue pointer Bart Van Assche
2020-11-16 17:18   ` Christoph Hellwig
2020-11-16  3:04 ` Bart Van Assche [this message]
2020-11-16 17:22   ` [PATCH v2 7/9] scsi_transport_spi: Freeze request queues instead of quiescing Christoph Hellwig
2020-11-16 17:51     ` Bart Van Assche
2020-11-16  3:04 ` [PATCH v2 8/9] block, scsi, ide: Only process PM requests if rpm_status != RPM_ACTIVE Bart Van Assche
2020-11-16 17:22   ` Christoph Hellwig
2020-11-17 16:00   ` [block, scsi, ide] 3e3b42fee6: kmsg.sd#:#:#:#:[sdf]Asking_for_cache_data_failed kernel test robot
2020-11-17 16:46     ` Bart Van Assche
2020-11-18  1:16       ` [LKP] " Philip Li
2020-11-18  1:13   ` [PATCH v2 8/9] block, scsi, ide: Only process PM requests if rpm_status != RPM_ACTIVE Can Guo
2020-11-18  9:08   ` Stanley Chu
2020-11-16  3:04 ` [PATCH v2 9/9] block: Do not accept any requests while suspended Bart Van Assche
2020-11-16 17:23   ` Christoph Hellwig
2020-11-18  1:12   ` Can Guo
2020-11-18  9:05   ` Stanley Chu

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=20201116030459.13963-8-bvanassche@acm.org \
    --to=bvanassche@acm.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=cang@codeaurora.org \
    --cc=hch@lst.de \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ming.lei@redhat.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=stanley.chu@mediatek.com \
    --cc=stern@rowland.harvard.edu \
    --cc=terraluna977@gmail.com \
    --cc=userm57@yahoo.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