2008-07-01 21:02:59

by Mike Miller

[permalink] [raw]
Subject: [PATCH 1/1] cciss: read config to obtain max outstanding commands per controller

PATCH 1/1

This patch changes the way we determine the maximum number of outstanding
commands for each controller. Most Smart Array controllers can support up
to 1024 commands, the notable exceptions are the E200 and E200i.
The next generation of controllers which were just added support a mode of
operation called Zero Memory Raid (ZMR). In this mode they only support 64
outstanding commands. In Full Function Raid (FFR) mode they support 1024.
We have been setting the queue depth by arbitrarily assigning some value for
each controller. We needed a better way to set the queue depth to avoid lots
of annoying "fifo full" messages. So we made the driver a little smarter. We
now read the config table and subtract 4 from the returned value. The -4 is
to allow some room for ioctl calls which are not tracked the same way as io
commands are tracked.

Please consider this for inclusion.

Signed-off-by: Mike Miller <[email protected]>

--------------------------------------------------------------------------------
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b2b6fb1..0d5df43 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -106,35 +106,34 @@ MODULE_DEVICE_TABLE(pci, cciss_pci_device_id);
/* board_id = Subsystem Device ID & Vendor ID
* product = Marketing Name for the board
* access = Address of the struct of function pointers
- * nr_cmds = Number of commands supported by controller
*/
static struct board_type products[] = {
- {0x40700E11, "Smart Array 5300", &SA5_access, 512},
- {0x40800E11, "Smart Array 5i", &SA5B_access, 512},
- {0x40820E11, "Smart Array 532", &SA5B_access, 512},
- {0x40830E11, "Smart Array 5312", &SA5B_access, 512},
- {0x409A0E11, "Smart Array 641", &SA5_access, 512},
- {0x409B0E11, "Smart Array 642", &SA5_access, 512},
- {0x409C0E11, "Smart Array 6400", &SA5_access, 512},
- {0x409D0E11, "Smart Array 6400 EM", &SA5_access, 512},
- {0x40910E11, "Smart Array 6i", &SA5_access, 512},
- {0x3225103C, "Smart Array P600", &SA5_access, 512},
- {0x3223103C, "Smart Array P800", &SA5_access, 512},
- {0x3234103C, "Smart Array P400", &SA5_access, 512},
- {0x3235103C, "Smart Array P400i", &SA5_access, 512},
- {0x3211103C, "Smart Array E200i", &SA5_access, 120},
- {0x3212103C, "Smart Array E200", &SA5_access, 120},
- {0x3213103C, "Smart Array E200i", &SA5_access, 120},
- {0x3214103C, "Smart Array E200i", &SA5_access, 120},
- {0x3215103C, "Smart Array E200i", &SA5_access, 120},
- {0x3237103C, "Smart Array E500", &SA5_access, 512},
- {0x323D103C, "Smart Array P700m", &SA5_access, 512},
- {0x3241103C, "Smart Array P212", &SA5_access, 384},
- {0x3243103C, "Smart Array P410", &SA5_access, 384},
- {0x3245103C, "Smart Array P410i", &SA5_access, 384},
- {0x3247103C, "Smart Array P411", &SA5_access, 384},
- {0x3249103C, "Smart Array P812", &SA5_access, 384},
- {0xFFFF103C, "Unknown Smart Array", &SA5_access, 120},
+ {0x40700E11, "Smart Array 5300", &SA5_access},
+ {0x40800E11, "Smart Array 5i", &SA5B_access},
+ {0x40820E11, "Smart Array 532", &SA5B_access},
+ {0x40830E11, "Smart Array 5312", &SA5B_access},
+ {0x409A0E11, "Smart Array 641", &SA5_access},
+ {0x409B0E11, "Smart Array 642", &SA5_access},
+ {0x409C0E11, "Smart Array 6400", &SA5_access},
+ {0x409D0E11, "Smart Array 6400 EM", &SA5_access},
+ {0x40910E11, "Smart Array 6i", &SA5_access},
+ {0x3225103C, "Smart Array P600", &SA5_access},
+ {0x3223103C, "Smart Array P800", &SA5_access},
+ {0x3234103C, "Smart Array P400", &SA5_access},
+ {0x3235103C, "Smart Array P400i", &SA5_access},
+ {0x3211103C, "Smart Array E200i", &SA5_access},
+ {0x3212103C, "Smart Array E200", &SA5_access},
+ {0x3213103C, "Smart Array E200i", &SA5_access},
+ {0x3214103C, "Smart Array E200i", &SA5_access},
+ {0x3215103C, "Smart Array E200i", &SA5_access},
+ {0x3237103C, "Smart Array E500", &SA5_access},
+ {0x323D103C, "Smart Array P700m", &SA5_access},
+ {0x3241103C, "Smart Array P212", &SA5_access},
+ {0x3243103C, "Smart Array P410", &SA5_access},
+ {0x3245103C, "Smart Array P410i", &SA5_access},
+ {0x3247103C, "Smart Array P411", &SA5_access},
+ {0x3249103C, "Smart Array P812", &SA5_access},
+ {0xFFFF103C, "Unknown Smart Array", &SA5_access},
};

/* How long to wait (in milliseconds) for board to go into simple mode */
@@ -3086,11 +3085,20 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
print_cfg_table(c->cfgtable);
#endif /* CCISS_DEBUG */

+ /* Some controllers support Zero Memory Raid (ZMR).
+ * When configured in ZMR mode the number of supported
+ * commands drops to 64. So instead of just setting an
+ * arbitrary value we make the driver a little smarter.
+ * We read the config table to tell us how many commands
+ * are supported on the controller then subtract 4 to
+ * leave a little room for ioctl calls.
+ */
+ c->max_commands = readl(&(c->cfgtable->CmdsOutMax));
for (i = 0; i < ARRAY_SIZE(products); i++) {
if (board_id == products[i].board_id) {
c->product_name = products[i].product_name;
c->access = *(products[i].access);
- c->nr_cmds = products[i].nr_cmds;
+ c->nr_cmds = c->max_commands - 4;
break;
}
}
@@ -3110,7 +3118,7 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
if (subsystem_vendor_id == PCI_VENDOR_ID_HP) {
c->product_name = products[i-1].product_name;
c->access = *(products[i-1].access);
- c->nr_cmds = products[i-1].nr_cmds;
+ c->nr_cmds = c->max_commands - 4;
printk(KERN_WARNING "cciss: This is an unknown "
"Smart Array controller.\n"
"cciss: Please update to the latest driver "


2008-07-01 21:24:09

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/1] cciss: read config to obtain max outstanding commands per controller

On Tue, 1 Jul 2008 16:02:47 -0500
Mike Miller <[email protected]> wrote:

> This patch changes the way we determine the maximum number of outstanding
> commands for each controller. Most Smart Array controllers can support up
> to 1024 commands, the notable exceptions are the E200 and E200i.
> The next generation of controllers which were just added support a mode of
> operation called Zero Memory Raid (ZMR). In this mode they only support 64
> outstanding commands. In Full Function Raid (FFR) mode they support 1024.
> We have been setting the queue depth by arbitrarily assigning some value for
> each controller. We needed a better way to set the queue depth to avoid lots
> of annoying "fifo full" messages. So we made the driver a little smarter. We
> now read the config table and subtract 4 from the returned value. The -4 is
> to allow some room for ioctl calls which are not tracked the same way as io
> commands are tracked.

What would be the effect of _not_ having this patch upon users of the
new hardware?

If the answer is "bad" then we should get this change into 2.6.26 and
2.6.25.x, because people will surely be running those kernels on the
new hardware.

2008-07-07 15:12:59

by Mike Miller

[permalink] [raw]
Subject: RE: [PATCH 1/1] cciss: read config to obtain max outstanding commands per controller



> -----Original Message-----
> From: Andrew Morton [mailto:[email protected]]
> Sent: Tuesday, July 01, 2008 4:23 PM
> To: Miller, Mike (OS Dev)
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] cciss: read config to obtain max
> outstanding commands per controller
>
> On Tue, 1 Jul 2008 16:02:47 -0500
> Mike Miller <[email protected]> wrote:
>
> > This patch changes the way we determine the maximum number of
> > outstanding commands for each controller. Most Smart Array
> controllers
> > can support up to 1024 commands, the notable exceptions are
> the E200 and E200i.
> > The next generation of controllers which were just added support a
> > mode of operation called Zero Memory Raid (ZMR). In this mode they
> > only support 64 outstanding commands. In Full Function Raid
> (FFR) mode they support 1024.
> > We have been setting the queue depth by arbitrarily assigning some
> > value for each controller. We needed a better way to set the queue
> > depth to avoid lots of annoying "fifo full" messages. So we
> made the
> > driver a little smarter. We now read the config table and
> subtract 4
> > from the returned value. The -4 is to allow some room for
> ioctl calls
> > which are not tracked the same way as io commands are tracked.
>
> What would be the effect of _not_ having this patch upon
> users of the new hardware?
>
> If the answer is "bad" then we should get this change into
> 2.6.26 and 2.6.25.x, because people will surely be running
> those kernels on the new hardware.

Sorry for my late reply, I've been out of the office. Not having this patch would be bad. The performance at a queue depth of 64 is not very good anyway. If we hit the "fifo full" condition we spew annoying messages and spin for a bit and try again. This leads to very poor performance.

Thanks,
mikem