[PATCH] cleanup switch in scsi_adjust_queue_depth()

[PATCH] cleanup switch in scsi_adjust_queue_depth()

[Douglas Gilbert]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

While checking what scsi_adjust_queue_depth() did I
thought its switch statement could be clearer.

ChangeLog:
   - remove redundant assignment (to sdev->queue_depth)
   - re-order cases (thus removing the fall-through)

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

[-- Attachment #2: scsi_adjust_queue_depth1.patch --]
[-- Type: text/x-patch, Size: 873 bytes --]

--- linux-3.15/drivers/scsi/scsi.c3151	2014-06-08 21:36:43.815551129 -0400
+++ linux-3.15/drivers/scsi/scsi.c	2014-06-24 19:17:02.474464807 -0400
@@ -850,6 +850,10 @@ void scsi_adjust_queue_depth(struct scsi
 
 	sdev->queue_depth = tags;
 	switch (tagged) {
+		case 0:
+			sdev->ordered_tags = 0;
+			sdev->simple_tags = 0;
+			break;
 		case MSG_ORDERED_TAG:
 			sdev->ordered_tags = 1;
 			sdev->simple_tags = 1;
@@ -859,13 +863,11 @@ void scsi_adjust_queue_depth(struct scsi
 			sdev->simple_tags = 1;
 			break;
 		default:
+			sdev->ordered_tags = 0;
+			sdev->simple_tags = 0;
 			sdev_printk(KERN_WARNING, sdev,
 				    "scsi_adjust_queue_depth, bad queue type, "
 				    "disabled\n");
-		case 0:
-			sdev->ordered_tags = sdev->simple_tags = 0;
-			sdev->queue_depth = tags;
-			break;
 	}
  out:
 	spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);

Re: [PATCH] cleanup switch in scsi_adjust_queue_depth()

[Martin K. Petersen]

>>>>> "Doug" == Douglas Gilbert <dgilbert@interlog.com> writes:

Doug> While checking what scsi_adjust_queue_depth() did I thought its
Doug> switch statement could be clearer.

Doug> ChangeLog:
Doug>   - remove redundant assignment (to sdev->queue_depth)
Doug>   - re-order cases (thus removing the fall-through)

Doug> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

I agree that the original version is a bit misleading.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH] cleanup switch in scsi_adjust_queue_depth()

[Elliott, Robert (Server Storage)]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Douglas Gilbert
> Sent: Tuesday, 24 June, 2014 6:24 PM
> To: SCSI development list; Christoph Hellwig; James Bottomley
> Subject: [PATCH] cleanup switch in scsi_adjust_queue_depth()
> 
> While checking what scsi_adjust_queue_depth() did I
> thought its switch statement could be clearer.
> 
> ChangeLog:
>    - remove redundant assignment (to sdev->queue_depth)
>    - re-order cases (thus removing the fall-through)
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>

I've run tests with tagged set to both 0 and MSG_SIMPLE_TAG 
and they both work like before, so you may add:
Reviewed-by: Robert Elliott <elliott@hp.com>
Tested-by: Robert Elliott <elliott@hp.com>

I did notice that include/scsi/scsi_tcq.h scsi_set_tag_type() 
is another function with a switch statement with fallthroughs 
setting these variables.  It doesn't have default: before 0: 
or involve a print, so reordering is not as necessary.

However, that function doesn't clear ordered_tags in the 
MSG_SIMPLE_TAG case, which seems problematic if the device
is ever changed from ordered to simple.  You might want to 
address that in this patch too.


---
Rob Elliott    HP Server Storage