Re: Degraded I/O performance, since 2.5.41

Re: Degraded I/O performance, since 2.5.41

[Dave Hansen]

Linus Torvalds wrote:
 > In article <3DA4D34F.3070106@us.ibm.com> you write:
 >
 >>When I run a certain large webserver benchmark, I prefer to warm the
 >>pagecache up with the file set first, to cheat a little :)  I grep
 >>through 20 different 500MB file sets in parallel to do this.  It is 
 >>a _lot_ slower in the BK snapshot than in plain 2.5.41.
 >
 > Can you pinpont where this started happening? There aren't that many
 > changes since 2.5.41 that I'd consider suspicious, and it would 
help > to try to pinpoint it a bit.

Using stupid brute force, I narrowed it down to 12 patches.  The only
one that touches generic SCSI code is the queue length one.
1.704.1.2, in my tree from dledford@redhat.com.  Backing it out fixed
my problem.  CC'ing him...

I would guess that it's just making stupid, er.... suboptimal,
decisions about the length for my hardware.  With a controller as
"smart" as the ServeRAID, I would imagine the best approach is to just
throw as many requests at it at one time as you can, then let the
controller sort it out.  Doug, I noticed that you changed a bunch of
code to the aic7xxx_old driver, is this just a lack of optimization in
the ServeRAID driver?

-- 
Dave Hansen
haveblue@us.ibm.com

Re: Degraded I/O performance, since 2.5.41

[James Bottomley]

haveblue@us.ibm.com said:
> I would guess that it's just making stupid, er.... suboptimal,
> decisions about the length for my hardware.  With a controller as
> "smart" as the ServeRAID, I would imagine the best approach is to just
> throw as many requests at it at one time as you can, then let the
> controller sort it out.  Doug, I noticed that you changed a bunch of
> code to the aic7xxx_old driver, is this just a lack of optimization in
> the ServeRAID driver?

OK, I bet this is because the ips driver now no longer actually does tagged 
command queueing.  I'll go an check.

James

Re: Degraded I/O performance, since 2.5.41

[James Bottomley]

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

OK, this patch should fix it.  Do your performance numbers for ips improve 
again with this?

James


[-- Attachment #2: tmp.diff --]
[-- Type: text/plain , Size: 687 bytes --]

===== drivers/scsi/scsi_scan.c 1.24 vs edited =====
--- 1.24/drivers/scsi/scsi_scan.c	Tue Oct  8 15:45:57 2002
+++ edited/drivers/scsi/scsi_scan.c	Thu Oct 10 17:40:53 2002
@@ -1477,11 +1477,14 @@
 		if (sdt->detect)
 			sdev->attached += (*sdt->detect) (sdev);
 
-	if (sdev->host->hostt->slave_attach != NULL)
+	if (sdev->host->hostt->slave_attach != NULL) {
 		if (sdev->host->hostt->slave_attach(sdev) != 0) {
 			printk(KERN_INFO "scsi_add_lun: failed low level driver attach, setting device offline");
 			sdev->online = FALSE;
 		}
+	} else if(sdev->host->cmd_per_lun) {
+		scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
+	}
 
 	if (sdevnew != NULL)
 		*sdevnew = sdev;

Re: Degraded I/O performance, since 2.5.41

[Doug Ledford]

On Thu, Oct 10, 2002 at 06:09:14PM -0700, James Bottomley wrote:
> OK, this patch should fix it.  Do your performance numbers for ips improve 
> again with this?

Well, it should help.  Most drivers set cmd_per_lun too low to really be 
good here (and well they should because you only want it big enough to 
keep a CD-R or tape streaming, no more, because it's only intended to be 
used on non-tagged devices, disks should have their depth set via 
scsi_adjust_queue_depth() by the low level driver).  This is in fact the 
right thing to do though, so I've added it to my code here as well since 
it's required in order for cmd_per_lun to do the right thing in regards to 
untagged devices.  The real answer is to update the ServeRAID driver to 
the new tagged queue adjustment mechanism I just wrote an intro to and 
sent to linux-scsi.

As a side note, the generic block layer tagged queueing mechanism may not 
be suitable for a lot of SCSI drivers (I know it's not suitable for mine 
at all).  How many of the drivers in the scsi tree today keep a shared tag 
table between all devices on a host and how many of them have separate tag 
tables for each device?  Show of hands anyone?  Justin, what about the 
modern aic7xxx driver?  Gerard?

> James
> 


-- 
  Doug Ledford <dledford@redhat.com>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606
  

block TCQ [was: Re: Degraded I/O performance, since 2.5.41]

[Luben Tuikov]

Doug Ledford wrote:
> 
> As a side note, the generic block layer tagged queueing mechanism may not
> be suitable for a lot of SCSI drivers (I know it's not suitable for mine
> at all).  How many of the drivers in the scsi tree today keep a shared tag
> table between all devices on a host and how many of them have separate tag
> tables for each device?  Show of hands anyone?  Justin, what about the
> modern aic7xxx driver?  Gerard?
> 

Ok. SAM-3 specifies that the initiator assigns task tags (4.9.1).
Who is the initiator?

But, the Abort Task and Abort Task Set (and friends) task management
functions are requested by the application client.
Futhermore, 6.2 Abort Task uses an I_T_L_Q nexus, that is, a tagged task.
So, to cancel an untagged task, one would use the 6.3 Abort Task Set,
which would be consistent since there should be only one untagged task
per initiator.

What are the implications and can we model around this?

Checking up on the definitions, 3.1.4 application client and 3.1.93
the initiator is your driver and the application client is the
SCSI core/block layer/sg/whatever.

AFAIR, IDE happened to support some type of tagging and all of a sudden
TCQ was moved up to the block layer, since SCSI has had it all along.

While this is alright, SAM-3 suggests that TCQ _generation_ is performed
by the SCSI initiator port (i.e. the SCSI LLDD) and for valid reasons...
(again those have to do with the underlying transport/interconnect...)

Thus, the only thing which the block layer would need to know is
_if_ TCQ is supported, but genereration of tags should be left to the
SCSI LLDD.

-- 
Luben

RE: Degraded I/O performance, since 2.5.41

[Jeffery, David]

> From: Dave Hansen 
> James Bottomley wrote:
> > OK, this patch should fix it.  Do your performance numbers 
> for ips improve 
> > again with this?
> 
> Yes, they are better, but still about 10% below what I was seeing 
> before.  Thank you for getting this out so quickly.  I can do 
> reasonable work with this.
> 
> Are the ServeRAID people aware of this situation?  Do they know that 
> their performance could be in the toliet if they don't 
> implement queue 
> resizing in the driver?
> -- 
> Dave Hansen
> haveblue@us.ibm.com
> 

Dave,

Thank you for for adding me to the CC list.  I didn't realize how the
queueing work would affect scsi drivers.

I'm using an older 2.5 kernel so I hadn't seen the performance drop.
I'll update my kernel and get to work on it next week when I have
time.

David Jeffery

Re: Degraded I/O performance, since 2.5.41

[Mike Anderson]

Sorry if this is a resend it looked this did not make it onto the list
Friday.

Jeffery, David [David_Jeffery@adaptec.com] wrote:
> > Yes, they are better, but still about 10% below what I was seeing 
> > before.  Thank you for getting this out so quickly.  I can do 
> > reasonable work with this.

Dave H,

The short term patch will not bring your queue_depth value back up all
the way. The ips drivers cmd_per_lun value is 16. The old method would
distribute the queue_depth among the devices on the adapter. I believe
your system has a ServeRAID 4M which means you could have had a
queue_depth as high as 95. This seems high but from your numbers a value
higher than 16 is most likely needed.

You can check your old and new values post booting the kernel if you have
sg configured in with:

cat /proc/scsi/sg/device_hdr /proc/scsi/sg/devices

> 
> I'm using an older 2.5 kernel so I hadn't seen the performance drop.
> I'll update my kernel and get to work on it next week when I have

David J,
Here is a quick patch for you to review. I tested and seems to bring my
system back up to per patch queue depth values. I used a enq value to
simulate past behavior in the attach function. I tested with one logical
and two logical devices.


Here is some sg output at different kernel and logical driver values.

2.5.40 1 logical drive:
host    chan    id      lun     type    opens   qdepth  busy    online
0       0       0       0       0       3       31      0       1

2.5.40 2 logical drives:
host    chan    id      lun     type    opens   qdepth  busy    online
0       0       0       0       0       3       15      0       1
0       0       1       0       0       0       15      0       1

2.5 current 2 logical drives: 
host    chan    id      lun     type    opens   qdepth  busy    online
0       0       0       0       0       3       1       0       1
0       0       1       0       0       0       1       0       1

2.5 current 2 logical drives + patch:
host    chan    id      lun     type    opens   qdepth  busy    online
0       0       0       0       0       3       15      0       1
0       0       1       0       0       0       7       0       1

2.5 current 2 logical drives + patch post some IO:
host    chan    id      lun     type    opens   qdepth  busy    online
0       0       0       0       0       3       15      0       1
0       0       1       0       0       0       15      0       1

-andmike
--
Michael Anderson
andmike@us.ibm.com

 ips.c |   47 ++++++++++++++++-------------------------------
 ips.h |    3 ++-
 2 files changed, 18 insertions(+), 32 deletions(-)
------

--- 1.26/drivers/scsi/ips.c	Tue Oct  8 00:51:39 2002
+++ edited/drivers/scsi/ips.c	Fri Oct 11 13:24:08 2002
@@ -481,7 +481,6 @@
 static void ips_free_flash_copperhead(ips_ha_t *ha);
 static void ips_get_bios_version(ips_ha_t *, int);
 static void ips_identify_controller(ips_ha_t *);
-static void ips_select_queue_depth(struct Scsi_Host *, Scsi_Device *);
 static void ips_chkstatus(ips_ha_t *, IPS_STATUS *);
 static void ips_enable_int_copperhead(ips_ha_t *);
 static void ips_enable_int_copperhead_memio(ips_ha_t *);
@@ -1087,7 +1086,6 @@
          sh->n_io_port = io_addr ? 255 : 0;
          sh->unique_id = (io_addr) ? io_addr : mem_addr;
          sh->irq = irq;
-         sh->select_queue_depths = ips_select_queue_depth;
          sh->sg_tablesize = sh->hostt->sg_tablesize;
          sh->can_queue = sh->hostt->can_queue;
          sh->cmd_per_lun = sh->hostt->cmd_per_lun;
@@ -1820,45 +1818,33 @@
 
 /****************************************************************************/
 /*                                                                          */
-/* Routine Name: ips_select_queue_depth                                     */
+/* Routine Name: ips_slave_attach                                           */
 /*                                                                          */
 /* Routine Description:                                                     */
 /*                                                                          */
-/*   Select queue depths for the devices on the contoller                   */
+/*   Configure the device we are attaching to this controller               */
 /*                                                                          */
 /****************************************************************************/
-static void
-ips_select_queue_depth(struct Scsi_Host *host, Scsi_Device *scsi_devs) {
-   Scsi_Device *device;
+int
+ips_slave_attach(Scsi_Device *scsi_dev) {
    ips_ha_t    *ha;
-   int          count = 0;
-   int          min;
+   int          queue_depth;
+   int          min, per_logical;
+
+   ha = (ips_ha_t *) scsi_dev->host->hostdata;;
 
-   ha = IPS_HA(host);
    min = ha->max_cmds / 4;
+   per_logical = ( ha->max_cmds -1 ) / ha->enq->ucLogDriveCount;
 
-   for (device = scsi_devs; device; device = device->next) {
-     if (device->host == host) {
-        if ((device->channel == 0) && (device->type == 0))
-           count++;
-     }
+   if ((scsi_dev->channel == 0) && (scsi_dev->type == 0)) {
+      queue_depth = max(per_logical, min);
+   } else {
+      queue_depth = 2;
    }
 
-   for (device = scsi_devs; device; device = device->next) {
-      if (device->host == host) {
-         if ((device->channel == 0) && (device->type == 0)) {
-            device->queue_depth = ( ha->max_cmds - 1 ) / count;
-            if (device->queue_depth < min) 
-               device->queue_depth = min;
-         }
-         else {
-            device->queue_depth = 2;
-         }
-
-         if (device->queue_depth < 2)
-            device->queue_depth = 2;
-      }
-   }
+   scsi_adjust_queue_depth(scsi_dev, 0, queue_depth);
+   
+   return 0;
 }
 
 /****************************************************************************/
@@ -7407,7 +7393,6 @@
     sh->n_io_port = io_addr ? 255 : 0;
     sh->unique_id = (io_addr) ? io_addr : mem_addr;
     sh->irq = irq;
-    sh->select_queue_depths = ips_select_queue_depth;
     sh->sg_tablesize = sh->hostt->sg_tablesize;
     sh->can_queue = sh->hostt->can_queue;
     sh->cmd_per_lun = sh->hostt->cmd_per_lun;
--- 1.9/drivers/scsi/ips.h	Tue Oct  8 00:51:39 2002
+++ edited/drivers/scsi/ips.h	Fri Oct 11 11:39:14 2002
@@ -62,6 +62,7 @@
    extern int ips_biosparam(Disk *, struct block_device *, int *);
    extern const char * ips_info(struct Scsi_Host *);
    extern void do_ips(int, void *, struct pt_regs *);
+   extern int ips_slave_attach(Scsi_Device *);
 
    /*
     * Some handy macros
@@ -481,7 +482,7 @@
     eh_host_reset_handler : ips_eh_reset, \
     abort : NULL,                         \
     reset : NULL,                         \
-    slave_attach : NULL,                  \
+    slave_attach : ips_slave_attach,      \
     bios_param : ips_biosparam,           \
     can_queue : 0,                        \
     this_id: -1,                          \