[patch 2/4] ibmvscsi.c: limit size of I/O requests

[patch 2/4] ibmvscsi.c: limit size of I/O requests

[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.

Signed-off-by: Dave Boutcher <boutcher@us.ibm.com>
---

 ibmvscsi.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

--- linux-2.6.10-rc1orig/drivers/scsi/ibmvscsi/ibmvscsi.c	2004-12-01 10:46:58.413032856 -0600
+++ linux-2.6.10-rc1/drivers/scsi/ibmvscsi/ibmvscsi.c	2004-12-01 10:46:34.616058720 -0600
@@ -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) {
-- 
Dave Boutcher

Re: [patch 2/4] ibmvscsi.c: limit size of I/O requests

[Jens Axboe]

On Mon, Nov 29 2004, Dave C Boutcher wrote:
> 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?

-- 
Jens Axboe

Re: [patch 2/4] ibmvscsi.c: limit size of I/O requests

[Dave C Boutcher]

On Tue, Nov 30, 2004 at 08:54:58AM +0100, Jens Axboe wrote:
> On Mon, Nov 29 2004, Dave C Boutcher wrote:
> > 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.

-- 
Dave Boutcher

Re: [patch 2/4] ibmvscsi.c: limit size of I/O requests

[Jens Axboe]

On Tue, Nov 30 2004, Dave C Boutcher wrote:
> On Tue, Nov 30, 2004 at 08:54:58AM +0100, Jens Axboe wrote:
> > On Mon, Nov 29 2004, Dave C Boutcher wrote:
> > > 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.

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?

-- 
Jens Axboe

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

[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 <boutcher@us.ibm.com>
Index: linux-2.6.10-rc3/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================
--- linux-2.6.10-rc3.orig/drivers/scsi/ibmvscsi/ibmvscsi.c	2004-12-31 10:41:07.430930080 -0600
+++ linux-2.6.10-rc3/drivers/scsi/ibmvscsi/ibmvscsi.c	2004-12-31 10:42:37.518945736 -0600
@@ -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) {
-- 
Dave Boutcher

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

[James Bottomley]

On Fri, 2004-12-31 at 11:47 -0600, Dave C Boutcher wrote:
> 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

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

[Dave C Boutcher]

On Fri, Dec 31, 2004 at 01:10:40PM -0600, James Bottomley wrote:
> 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.

-- 
Dave Boutcher