diff mbox series

[v6,02/12] scsi: Alter handling of RQF_DV requests

Message ID 20180809194149.15285-3-bart.vanassche@wdc.com (mailing list archive)
State New, archived
Headers show
Series blk-mq: Implement runtime power management | expand

Commit Message

Bart Van Assche Aug. 9, 2018, 7:41 p.m. UTC 
Process all requests in state SDEV_CREATED instead of only RQF_DV
requests. This does not change the behavior of the SCSI core because
the SCSI device state is modified into another state before SCSI
devices become visible in sysfs and before any device nodes are
created in /dev. Do not process RQF_DV requests in state SDEV_CANCEL
because only power management requests should be processed in this
state. Handle all SCSI device states explicitly in
scsi_prep_state_check() instead of using a default case in the
switch/case statement in scsi_prep_state_check(). This allows the
compiler to verify whether all states have been handled.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Jianchao Wang <jianchao.w.wang@oracle.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/scsi/scsi_lib.c | 78 +++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

Comments

Ming Lei Aug. 10, 2018, 1:20 a.m. UTC | #1 
On Thu, Aug 09, 2018 at 12:41:39PM -0700, Bart Van Assche wrote:
> Process all requests in state SDEV_CREATED instead of only RQF_DV
> requests. This does not change the behavior of the SCSI core because
> the SCSI device state is modified into another state before SCSI
> devices become visible in sysfs and before any device nodes are
> created in /dev. Do not process RQF_DV requests in state SDEV_CANCEL
> because only power management requests should be processed in this
> state.

Could you explain a bit why only PM requests should be processed in
SDEV_CANCEL?

Thanks,
Ming
Bart Van Assche Aug. 10, 2018, 3:07 p.m. UTC | #2 
On Fri, 2018-08-10 at 09:20 +0800, Ming Lei wrote:
> On Thu, Aug 09, 2018 at 12:41:39PM -0700, Bart Van Assche wrote:
> > Process all requests in state SDEV_CREATED instead of only RQF_DV
> > requests. This does not change the behavior of the SCSI core because
> > the SCSI device state is modified into another state before SCSI
> > devices become visible in sysfs and before any device nodes are
> > created in /dev. Do not process RQF_DV requests in state SDEV_CANCEL
> > because only power management requests should be processed in this
> > state.
> 
> Could you explain a bit why only PM requests should be processed in
> SDEV_CANCEL?

Hi Ming,

There is only one function that changes the device state into SDEV_CANCEL,
namely __scsi_remove_device(). I think that in the SDEV_CANCEL state all
newly queued requests should be ignored except the SYNCHRONIZE CACHE and
STOP commands submitted by sd_shutdown(). More information about the SCSI
device states is available in the commit message of 9b22a8fb0edd ("Updated
state model for SCSI devices").

Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a65a03e2bcc4..8685704f6c8b 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1331,49 +1331,43 @@  scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 	 * If the device is not in running state we will reject some
 	 * or all commands.
 	 */
-	if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
-		switch (sdev->sdev_state) {
-		case SDEV_OFFLINE:
-		case SDEV_TRANSPORT_OFFLINE:
-			/*
-			 * If the device is offline we refuse to process any
-			 * commands.  The device must be brought online
-			 * before trying any recovery commands.
-			 */
-			sdev_printk(KERN_ERR, sdev,
-				    "rejecting I/O to offline device\n");
-			ret = BLKPREP_KILL;
-			break;
-		case SDEV_DEL:
-			/*
-			 * If the device is fully deleted, we refuse to
-			 * process any commands as well.
-			 */
-			sdev_printk(KERN_ERR, sdev,
-				    "rejecting I/O to dead device\n");
-			ret = BLKPREP_KILL;
-			break;
-		case SDEV_BLOCK:
-		case SDEV_CREATED_BLOCK:
+	switch (sdev->sdev_state) {
+	case SDEV_RUNNING:
+	case SDEV_CREATED:
+		break;
+	case SDEV_OFFLINE:
+	case SDEV_TRANSPORT_OFFLINE:
+		/*
+		 * If the device is offline we refuse to process any commands.
+		 * The device must be brought online before trying any
+		 * recovery commands.
+		 */
+		sdev_printk(KERN_ERR, sdev,
+			    "rejecting I/O to offline device\n");
+		ret = BLKPREP_KILL;
+		break;
+	case SDEV_DEL:
+		/*
+		 * If the device is fully deleted, we refuse to process any
+		 * commands as well.
+		 */
+		sdev_printk(KERN_ERR, sdev, "rejecting I/O to dead device\n");
+		ret = BLKPREP_KILL;
+		break;
+	case SDEV_BLOCK:
+	case SDEV_CREATED_BLOCK:
+		ret = BLKPREP_DEFER;
+		break;
+	case SDEV_QUIESCE:
+		/* Only allow RQF_PM and RQF_DV requests. */
+		if (!(req->rq_flags & (RQF_PM | RQF_DV)))
 			ret = BLKPREP_DEFER;
-			break;
-		case SDEV_QUIESCE:
-			/*
-			 * If the devices is blocked we defer normal commands.
-			 */
-			if (req && !(req->rq_flags & (RQF_PM | RQF_DV)))
-				ret = BLKPREP_DEFER;
-			break;
-		default:
-			/*
-			 * For any other not fully online state we only allow
-			 * special commands.  In particular any user initiated
-			 * command is not allowed.
-			 */
-			if (req && !(req->rq_flags & (RQF_PM | RQF_DV)))
-				ret = BLKPREP_KILL;
-			break;
-		}
+		break;
+	case SDEV_CANCEL:
+		/* Only allow RQF_PM requests. */
+		if (!(req->rq_flags & RQF_PM))
+			ret = BLKPREP_KILL;
+		break;
 	}
 	return ret;
 }