Skip to contentSkip to conversation

cleanup switch in scsi_adjust_queue_depth()

PATCH In review Douglas Gilbert 3 participants 3 messages Jun 24, 2014 – Jul 1, 2014 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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)

drivers/scsi/scsi.c renamed from drivers/scsi/scsi.c3151 +6 -4
@@ -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);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Doug" == Douglas Gilbert <[email protected]> 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 <[email protected]>

I agree that the original version is a bit misleading.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
Reviewed-by Tested-by on v1 1/1
#2 lore raw
In reply to Douglas Gilbert
Quoted 9 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] 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()

Quoted 8 lines

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)

I've run tests with tagged set to both 0 and MSG_SIMPLE_TAG and they both work like before, so you may add:

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