Skip to contentSkip to conversation

ibmvscsi.c: remove max-sectors module parm

PATCH In review Dave C Boutcher 2 participants 7 messages Dec 21, 2004 – Dec 31, 2004

Subsystems (3)
SCSI PowerPC IBM Power Virtual SCSI Device Drivers
lore
Share & tools
lore

Dave C Boutcher

author
#0 lore raw

PATCH 1/1 ibmvscsi.c: remove max-sectors module parm

Description: Based on comments from Jens Axboe, remove redundant module parm to set max sectors. This should be set through the /dev/block/xxx/queue/max_sectors_kb interface. This incremental patch fixes the only comments received to the previous set of patches submitted.

Signed-off-by Dave Boutcher <[email protected]>

---

 ibmvscsi.c |    2 --
 1 files changed, 2 deletions(-)
drivers/scsi/ibmvscsi/ibmvscsi.c +0 -2
Index: linux-2.6.10-rc3/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================
@@ -103,8 +103,6 @@
 MODULE_PARM_DESC(init_timeout, "Initialization timeout in seconds");
 module_param_named(max_requests, max_requests, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(max_requests, "Maximum requests for this adapter");
-module_param_named(max_sectors, max_sectors, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(max_sectors, "Maximum sectors per request for this adapter");
 
 /* ------------------------------------------------------------
  * Routines for the event pool and event structs
Signature · 1 line
Dave Boutcher
#1 lore raw
Quoted 5 lines · Dave C Boutcher

Description: Based on comments from Jens Axboe, remove redundant module parm to set max sectors. This should be set through the /dev/block/xxx/queue/max_sectors_kb interface. This incremental patch fixes the only comments received to the previous set of patches submitted.

This one doesn't apply to scsi-misc-2.6, could you reroll against what's there?

Thanks,

James

author
#2 lore raw
Quoted 9 lines · James Bottomley
Quoted 5 lines · Dave C Boutcher

Description: Based on comments from Jens Axboe, remove redundant module parm to set max sectors. This should be set through the /dev/block/xxx/queue/max_sectors_kb interface. This incremental patch fixes the only comments received to the previous set of patches submitted.

This one doesn't apply to scsi-misc-2.6, could you reroll against what's there?

The patches I submitted on Tuesday are incremental on top of the four patches from November 29th. Let me know if you would like the November 29th patches re-sent.

Signature · 1 line
Dave Boutcher
#3 lore raw
Quoted 3 lines · Dave C Boutcher

The patches I submitted on Tuesday are incremental on top of the four patches from November 29th. Let me know if you would like the November 29th patches re-sent.

Actually, just the limit size of I/O requests with the two patches rolled together (or is there nothing left of this now?)

James

author
#4 lore raw
Quoted 7 lines · James Bottomley
Quoted 3 lines · Dave C Boutcher

The patches I submitted on Tuesday are incremental on top of the four patches from November 29th. Let me know if you would like the November 29th patches re-sent.

Actually, just the limit size of I/O requests with the two patches rolled together (or is there nothing left of this now?)

I'm not sure I parse that...but in any case, one of the four patches from November limited the size of the I/O requests based on information from the SCSI target, with a default value in case the target doesn't tell us. I got overly clever and made the default value a module parameter. Jens pointed out that I was being redundant, since there are other ways to set max sectors. The patch I submitted on Tuesday leaves the basic function intact (setting max I/O size based on input from the target) and just removes the redundant module parameter.

Signature · 1 line
Dave Boutcher
#5 lore raw
Quoted 12 lines · Dave C Boutcher
Quoted 2 lines

Actually, just the limit size of I/O requests with the two patches rolled together (or is there nothing left of this now?)

I'm not sure I parse that...but in any case, one of the four patches from November limited the size of the I/O requests based on information from the SCSI target, with a default value in case the target doesn't tell us. I got overly clever and made the default value a module parameter. Jens pointed out that I was being redundant, since there are other ways to set max sectors. The patch I submitted on Tuesday leaves the basic function intact (setting max I/O size based on input from the target) and just removes the redundant module parameter.

What I mean is that after there's been argument over a patch I want an updated diff (like the one below) with a new change log containing the resolution, not a diff to a previous patch.

Also, in the diff below (which is why it's a good idea to do it this way) the static parameter max_sectors now looks to be superfluous, so you can get rid of it.

James

drivers/scsi/ibmvscsi/ibmvscsi.c +10 -3
@@ -86,8 +86,9 @@
 static int max_channel = 3;
 static int init_timeout = 5;
 static int max_requests = 50;
+static int max_sectors = 32 * 8; /* default max I/O 32 pages */
 
-#define IBMVSCSI_VERSION "1.5.1"
+#define IBMVSCSI_VERSION "1.5.2"
 
 MODULE_DESCRIPTION("IBM Virtual SCSI");
 MODULE_AUTHOR("Dave Boutcher");

@@ -641,11 +644,16 @@
 		       evt_struct->xfer_iu->mad.adapter_info.common.status);
 	} else {
 		printk("ibmvscsi: host srp version: %s, "
-		       "host partition %s (%d), OS %d\n",
+		       "host partition %s (%d), OS %d, max io %u\n",
 		       hostdata->madapter_info.srp_version,
 		       hostdata->madapter_info.partition_name,
 		       hostdata->madapter_info.partition_number,
-		       hostdata->madapter_info.os_type);
+		       hostdata->madapter_info.os_type,
+		       hostdata->madapter_info.port_max_txu[0]);
+
+		if (hostdata->madapter_info.port_max_txu[0]) 
+		    hostdata->host->max_sectors = 
+			hostdata->madapter_info.port_max_txu[0] >> 9;
 	}
 }
 

@@ -1295,6 +1303,7 @@
 	hostdata->host = host;
 	hostdata->dev = dev;
 	atomic_set(&hostdata->request_limit, -1);
+	hostdata->host->max_sectors = max_sectors; 
 
 	if (ibmvscsi_init_crq_queue(&hostdata->queue, hostdata,
 				    max_requests) != 0) {
author
#6 lore raw
Quoted 3 lines · James Bottomley

What I mean is that after there's been argument over a patch I want an updated diff (like the one below) with a new change log containing the resolution, not a diff to a previous patch.

OK, thanks for the guidance...new patch on its way. I have a number of subsequent patches which still apply fine (with some fuzz.) Would you like them resubmitted to get everything back in sync?

Signature · 1 line
Dave Boutcher