Skip to contentSkip to conversation

ibmvscsi.c: limit size of I/O requests

PATCH NAK Dave C Boutcher 3 participants 7 messages Nov 29, 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 2/4 ibmvscsi.c: limit size of I/O requests

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

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

---

 ibmvscsi.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)
drivers/scsi/ibmvscsi/ibmvscsi.c +12 -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");

@@ -102,6 +103,8 @@
 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

@@ -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) {
Signature · 1 line
Dave Boutcher
NAK
#1 lore raw
Quoted 3 lines · Dave C Boutcher

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

Nack, why can't you use /dev/block/xxx/queue/max_sectors_kb to control this?

Signature · 1 line
Jens Axboe
author
#2 lore raw
Quoted 7 lines · Jens Axboe
Quoted 3 lines · Dave C Boutcher

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

Nack, why can't you use /dev/block/xxx/queue/max_sectors_kb to control this?

The basic function of this patch is to set host->max_sectors based on information from the SCSI target.

There is a default value because two of the existing target implementations (there are three) don't set the value.

I confess to making the default value a module parameter because I took a SWAG at the correct value. I COULD remove the module parameter part of this patch, though if someone actually cares enough about performance to tweak it, modprobe.conf seems like a nice place to set it.

Signature · 1 line
Dave Boutcher
#3 lore raw
Quoted 12 lines · Dave C Boutcher
Quoted 7 lines · Jens Axboe
Quoted 3 lines · Dave C Boutcher

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

Nack, why can't you use /dev/block/xxx/queue/max_sectors_kb to control this?

The basic function of this patch is to set host->max_sectors based on information from the SCSI target.

Quoted 8 lines

There is a default value because two of the existing target implementations (there are three) don't set the value.

I confess to making the default value a module parameter because I took a SWAG at the correct value. I COULD remove the module parameter part of this patch, though if someone actually cares enough about performance to tweak it, modprobe.conf seems like a nice place to set it.

I think it's much better to set host->max_sectors to a sane high value, then change it per-target if you need to. This is how other SCSI host adapters work, what makes ibmvscsi different?

Signature · 1 line
Jens Axboe
author
#4 lore raw

PATCH 1/1 ibmvscsi.c: limit size of I/O requests, updated

In reply to Dave C Boutcher

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

Updated based on comments from Jens Axboe and James Bottomley to not specify max I/O sectors as a module parameter, and subsequently not needlessly store the value as a static variable.

Signed-off-by Dave Boutcher <[email protected]>
drivers/scsi/ibmvscsi/ibmvscsi.c +9 -3
Index: linux-2.6.10-rc3/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================
@@ -87,7 +87,7 @@
 static int init_timeout = 5;
 static int max_requests = 50;
 
-#define IBMVSCSI_VERSION "1.5.1"
+#define IBMVSCSI_VERSION "1.5.2"
 
 MODULE_DESCRIPTION("IBM Virtual SCSI");
 MODULE_AUTHOR("Dave Boutcher");

@@ -641,11 +641,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 +1300,7 @@
 	hostdata->host = host;
 	hostdata->dev = dev;
 	atomic_set(&hostdata->request_limit, -1);
+	hostdata->host->max_sectors = 32 * 8; /* default max I/O 32 pages */
 
 	if (ibmvscsi_init_crq_queue(&hostdata->queue, hostdata,
 				    max_requests) != 0) {
Signature · 1 line
Dave Boutcher
#5 lore raw

Re: [patch] ibmvscsi.c: limit size of I/O requests, updated

Quoted 8 lines · Dave C Boutcher

Description: Limit the size of I/O requests sent by the ibmvscsi adapter. With better I/O scheduling (and thus larger requests) we were breaking some servers.

Updated based on comments from Jens Axboe and James Bottomley to not specify max I/O sectors as a module parameter, and subsequently not needlessly store the value as a static variable.

Got it, thanks (you don't actually have to summarise the argument, merely describe the current patch).

Can you look at the scsi-misc-2.6 tree (or the current BK head plus the scsi-misc-2.6 patch from http://parisc-linux.org/~jejb/scsi_diffs/) and tell me what (if anything) I'm still missing?

Thanks,

James

author
#6 lore raw

Re: [patch] ibmvscsi.c: limit size of I/O requests, updated

Quoted 3 lines · James Bottomley

Can you look at the scsi-misc-2.6 tree (or the current BK head plus the scsi-misc-2.6 patch from http://parisc-linux.org/~jejb/scsi_diffs/) and tell me what (if anything) I'm still missing?

Thanks, scsi-misc-2.6 has the patch in question. There are three other patches floating around the ether somewhere that move the driver up to version 1.5.5. I'll resend those.

Signature · 1 line
Dave Boutcher