On 2004.09.01 11:33, Matt Heler wrote:
>
> I have a 3ware 7000-2 card. And I noticed the same problem.
>
> Actually what I just did now was change the max luns from 254 to 64.
> Recompiled and booted up. This seems to fix all my problems, and the speed
> seems to be quite faster then before.
Yes, that is because the queue_depth parameter gets set from
TW_MAX_CMDS_PER_LUN by the 3w-xxxx.c driver ...
I found the 3ware patch. The patch below makes the queue depth
an optional module parameter, makes sure that the initial
nr_requests is twice the size of the queue_depth, and
makes queue_depth writable for the 3ware driver.
Mike.
--- linux-2.6.5-rc2/drivers/scsi/3w-xxxx.c 2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.5-rc2-dmcong-tw/drivers/scsi/3w-xxxx.c 2004-03-23 14:56:41.000000000 +0100
@@ -13,6 +13,12 @@
Further tiny build fixes and trivial hoovering Alan Cox
+ Parameters (and default):
+
+ 3w-xxxx.queue_depth Queue depth per connected device (254)
+ 3w-xxxx.reverse_scan Set to "1" if you want the driver to detect
+ the 3ware cards in reverse order (0).
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; version 2 of the License.
@@ -179,6 +185,10 @@
1.02.00.036 - Increase character ioctl timeout to 60 seconds.
1.02.00.037 - Fix tw_ioctl() to handle all non-data ATA passthru cmds
for 'smartmontools' support.
+ 1.02.00.XXX - Miquel van Smoorenburg - add command line parameters
+ to set queue_depth/reverse_scan, make queue_depth
+ sysfs parameter writable, adjust queue nr_requests.
+
*/
#include <linux/module.h>
@@ -205,6 +215,7 @@
#include <linux/reboot.h>
#include <linux/spinlock.h>
#include <linux/interrupt.h>
+#include <linux/moduleparam.h>
#include <asm/errno.h>
#include <asm/io.h>
@@ -246,6 +257,13 @@
TW_Device_Extension *tw_device_extension_list[TW_MAX_SLOT];
int tw_device_extension_count = 0;
static int twe_major = -1;
+static int reverse_scan;
+static int queue_depth;
+
+module_param(reverse_scan, int, 0);
+MODULE_PARM_DESC(reverse_scan, "Scan PCI bus in reverse for 3ware cards");
+module_param(queue_depth, int, 0);
+MODULE_PARM_DESC(queue_depth, "Queue depth per device");
/* Functions */
@@ -1029,7 +1047,15 @@
dprintk(KERN_NOTICE "3w-xxxx: tw_findcards()\n");
for (i=0;i<TW_NUMDEVICES;i++) {
- while ((tw_pci_dev = pci_find_device(TW_VENDOR_ID, device[i], tw_pci_dev))) {
+ while (1) {
+ if (reverse_scan)
+ tw_pci_dev = pci_find_device_reverse(
+ TW_VENDOR_ID, device[i], tw_pci_dev);
+ else
+ tw_pci_dev = pci_find_device(
+ TW_VENDOR_ID, device[i], tw_pci_dev);
+ if (!tw_pci_dev)
+ break;
j++;
if (pci_enable_device(tw_pci_dev))
continue;
@@ -1141,14 +1167,6 @@
/* Set card status as online */
tw_dev->online = 1;
-#ifdef CONFIG_3W_XXXX_CMD_PER_LUN
- tw_host->cmd_per_lun = CONFIG_3W_XXXX_CMD_PER_LUN;
- if (tw_host->cmd_per_lun > TW_MAX_CMDS_PER_LUN)
- tw_host->cmd_per_lun = TW_MAX_CMDS_PER_LUN;
-#else
- /* Use SHT cmd_per_lun here */
- tw_host->cmd_per_lun = TW_MAX_CMDS_PER_LUN;
-#endif
tw_dev->free_head = TW_Q_START;
tw_dev->free_tail = TW_Q_START;
tw_dev->free_wrap = TW_Q_LENGTH - 1;
@@ -3379,21 +3397,17 @@
return 0;
} /* End tw_shutdown_device() */
-/* This function will configure individual target parameters */
+/* This function configures individual target parameters */
int tw_slave_configure(Scsi_Device *SDptr)
{
- int max_cmds;
-
- dprintk(KERN_WARNING "3w-xxxx: tw_slave_configure()\n");
-
-#ifdef CONFIG_3W_XXXX_CMD_PER_LUN
- max_cmds = CONFIG_3W_XXXX_CMD_PER_LUN;
- if (max_cmds > TW_MAX_CMDS_PER_LUN)
- max_cmds = TW_MAX_CMDS_PER_LUN;
-#else
- max_cmds = TW_MAX_CMDS_PER_LUN;
-#endif
- scsi_adjust_queue_depth(SDptr, MSG_ORDERED_TAG, max_cmds);
+ /* Set SCSI queue depth to kerne/module param, or default. */
+ if (queue_depth < 1 || queue_depth > TW_MAX_CMDS_PER_LUN)
+ queue_depth = TW_MAX_CMDS_PER_LUN;
+ scsi_adjust_queue_depth(SDptr, 0, queue_depth);
+
+ /* make sure blockdev queue depth is at least 2 * scsi depth */
+ if (SDptr->request_queue->nr_requests < 2 * queue_depth)
+ SDptr->request_queue->nr_requests = 2 * queue_depth;
return 0;
} /* End tw_slave_configure() */
@@ -3478,6 +3492,34 @@
outl(control_reg_value, control_reg_addr);
} /* End tw_unmask_command_interrupt() */
+static ssize_t
+tw_store_queue_depth(struct device *dev, const char *buf, size_t count)
+{
+ int depth;
+
+ struct scsi_device *SDp = to_scsi_device(dev);
+ if (sscanf(buf, "%d", &depth) != 1)
+ return -EINVAL;
+ if (depth < 1 || depth > TW_MAX_CMDS_PER_LUN)
+ return -EINVAL;
+ scsi_adjust_queue_depth(SDp, 0, depth);
+
+ return count;
+}
+
+static struct device_attribute tw_queue_depth_attr = {
+ .attr = {
+ .name = "queue_depth",
+ .mode = S_IWUSR,
+ },
+ .store = tw_store_queue_depth,
+};
+
+static struct device_attribute *tw_dev_attrs[] = {
+ &tw_queue_depth_attr,
+ NULL,
+};
+
static Scsi_Host_Template driver_template = {
.proc_name = "3w-xxxx",
.proc_info = tw_scsi_proc_info,
@@ -3488,12 +3530,14 @@
.eh_abort_handler = tw_scsi_eh_abort,
.eh_host_reset_handler = tw_scsi_eh_reset,
.bios_param = tw_scsi_biosparam,
+ .slave_configure = tw_slave_configure,
.can_queue = TW_Q_LENGTH-2,
.this_id = -1,
.sg_tablesize = TW_MAX_SGL_LENGTH,
.max_sectors = TW_MAX_SECTORS,
.cmd_per_lun = TW_MAX_CMDS_PER_LUN,
.use_clustering = ENABLE_CLUSTERING,
+ .sdev_attrs = tw_dev_attrs,
.emulated = 1
};
#include "scsi_module.c"
On Wed, Sep 01, 2004 at 09:58:55AM +0000, Miquel van Smoorenburg wrote:
> On 2004.09.01 11:33, Matt Heler wrote:
> >
> > I have a 3ware 7000-2 card. And I noticed the same problem.
> >
> > Actually what I just did now was change the max luns from 254 to 64.
> > Recompiled and booted up. This seems to fix all my problems, and the speed
> > seems to be quite faster then before.
>
> Yes, that is because the queue_depth parameter gets set from
> TW_MAX_CMDS_PER_LUN by the 3w-xxxx.c driver ...
>
> I found the 3ware patch. The patch below makes the queue depth
> an optional module parameter, makes sure that the initial
> nr_requests is twice the size of the queue_depth, and
> makes queue_depth writable for the 3ware driver.
- the writeable queue_depth sysfs attr is fine,
- the reverse_scan option is vetoed because it can't be supported when
the driver will be converted to the pci_driver interface (soon)
- I'm not so sure about the module parameter, what's the problem of beeing
able to only change the queue depth once sysfs is mounted?
On 2004.09.01 12:09, Christoph Hellwig wrote:
> On Wed, Sep 01, 2004 at 09:58:55AM +0000, Miquel van Smoorenburg wrote:
> > On 2004.09.01 11:33, Matt Heler wrote:
> > >
> > > I have a 3ware 7000-2 card. And I noticed the same problem.
> > >
> > > Actually what I just did now was change the max luns from 254 to 64.
> > > Recompiled and booted up. This seems to fix all my problems, and the speed
> > > seems to be quite faster then before.
> >
> > Yes, that is because the queue_depth parameter gets set from
> > TW_MAX_CMDS_PER_LUN by the 3w-xxxx.c driver ...
> >
> > I found the 3ware patch. The patch below makes the queue depth
> > an optional module parameter, makes sure that the initial
> > nr_requests is twice the size of the queue_depth, and
> > makes queue_depth writable for the 3ware driver.
>
> - the writeable queue_depth sysfs attr is fine,
> - the reverse_scan option is vetoed because it can't be supported when
> the driver will be converted to the pci_driver interface (soon)
Sure. That was more an experiment (the BIOS of the Tyan mobo I use
detects PCI cards in the reverse order from the kernel ...)
> - I'm not so sure about the module parameter, what's the problem of beeing
> able to only change the queue depth once sysfs is mounted?
Nothing much, I guess. Just ease of use, or "there's more than one
way to do it". Hey wait, that tunable is already a module parameter
in at least 2.6.9-rc1, only there it's called 'cmds_per_lun'.
Ofcourse cmds_per_lun and queue_depth are the same.
Anyway here's the minimal patch against 2.6.9-rc1
[PATCH] 3w-xxxx.c queue depth
make 3w-xxxx.c queue_depth sysfs parameter writable, adjust initial
queue nr_requests to 2*queue_depth
Signed-off-by: Miquel van Smoorenburg <[email protected]>
--- linux-2.6.9-rc1/drivers/scsi/3w-xxxx.c.orig 2004-08-17 22:07:49.000000000 +0200
+++ linux-2.6.9-rc1/drivers/scsi/3w-xxxx.c 2004-09-01 13:07:32.000000000 +0200
@@ -184,6 +184,8 @@
1.26.00.039 - Fix bug in tw_chrdev_ioctl() polling code.
Fix data_buffer_length usage in tw_chrdev_ioctl().
Update contact information.
+ 1.02.00.XXX - Miquel van Smoorenburg - make queue_depth sysfs parameter
+ writable, adjust initial queue nr_requests to 2*queue_depth
*/
#include <linux/module.h>
@@ -3388,8 +3390,6 @@
{
int max_cmds;
- dprintk(KERN_WARNING "3w-xxxx: tw_slave_configure()\n");
-
if (cmds_per_lun) {
max_cmds = cmds_per_lun;
if (max_cmds > TW_MAX_CMDS_PER_LUN)
@@ -3399,6 +3399,10 @@
}
scsi_adjust_queue_depth(SDptr, MSG_ORDERED_TAG, max_cmds);
+ /* make sure blockdev queue depth is at least 2 * scsi depth */
+ if (SDptr->request_queue->nr_requests < 2 * max_cmds)
+ SDptr->request_queue->nr_requests = 2 * max_cmds;
+
return 0;
} /* End tw_slave_configure() */
@@ -3482,6 +3486,34 @@
outl(control_reg_value, control_reg_addr);
} /* End tw_unmask_command_interrupt() */
+static ssize_t
+tw_store_queue_depth(struct device *dev, const char *buf, size_t count)
+{
+ int depth;
+
+ struct scsi_device *SDp = to_scsi_device(dev);
+ if (sscanf(buf, "%d", &depth) != 1)
+ return -EINVAL;
+ if (depth < 1 || depth > TW_MAX_CMDS_PER_LUN)
+ return -EINVAL;
+ scsi_adjust_queue_depth(SDp, MSG_ORDERED_TAG, depth);
+
+ return count;
+}
+
+static struct device_attribute tw_queue_depth_attr = {
+ .attr = {
+ .name = "queue_depth",
+ .mode = S_IWUSR,
+ },
+ .store = tw_store_queue_depth,
+};
+
+static struct device_attribute *tw_dev_attrs[] = {
+ &tw_queue_depth_attr,
+ NULL,
+};
+
static Scsi_Host_Template driver_template = {
.proc_name = "3w-xxxx",
.proc_info = tw_scsi_proc_info,
@@ -3499,6 +3531,7 @@
.max_sectors = TW_MAX_SECTORS,
.cmd_per_lun = TW_MAX_CMDS_PER_LUN,
.use_clustering = ENABLE_CLUSTERING,
+ .sdev_attrs = tw_dev_attrs,
.emulated = 1
};
#include "scsi_module.c"
On Wed, Sep 01, 2004 at 11:08:39AM +0000, Miquel van Smoorenburg wrote:
> Anyway here's the minimal patch against 2.6.9-rc1
>
> [PATCH] 3w-xxxx.c queue depth
>
> make 3w-xxxx.c queue_depth sysfs parameter writable, adjust initial
> queue nr_requests to 2*queue_depth
Looks good to me.
On Wed, Sep 01, 2004 at 11:08:39AM +0000, Miquel van Smoorenburg wrote:
> + /* make sure blockdev queue depth is at least 2 * scsi depth */
> + if (SDptr->request_queue->nr_requests < 2 * max_cmds)
> + SDptr->request_queue->nr_requests = 2 * max_cmds;
Why would you want nr_requests different (and larger) only for this
driver?
Is modifying nr_requests allowed?
-- Patrick Mansfield
On Wed, 01 Sep 2004 21:43:25, Patrick Mansfield wrote:
> On Wed, Sep 01, 2004 at 11:08:39AM +0000, Miquel van Smoorenburg wrote:
>
> > + /* make sure blockdev queue depth is at least 2 * scsi depth */
> > + if (SDptr->request_queue->nr_requests < 2 * max_cmds)
> > + SDptr->request_queue->nr_requests = 2 * max_cmds;
>
> Why would you want nr_requests different (and larger) only for this
> driver?
Because for the Linux I/O scheduler to work, nr_requests needs to
be at least twice as big as the scsi queue depth.
For all other scsi drivers, the scsi queue depth is somewhere between
0 and 63. Most are between 1 and 8.
Default nr_requests is 128, so this problem exists only with the
3ware driver/controller that has a queue depth of 254 ..
It's more complicated than that though when you have more than
one scsi device attached to the 3ware controller (multiple raid
arrays or JBODs defined), since the total queue depth of the
controller is 254. In that case one scsi device can starve
others on the same controller, so you want to tune down the
queue depth per device .. e.g. with 8 JBODs set queue_depth
per device to 32, set nr_requests to 128.
Perhaps the initial queue_depth per device should be set to
254 / tw_dev->tw_num_units, that would be optimal.
Something like
max_cmds = tw_host->can_queue / tw_dev->tw_num_units;
if (max_cmds > TW_MAX_CMDS_PER_LUN)
max_cmds = TW_MAX_CMDS_PER_LUN;
I think such a change should be submitted through the people
at 3ware, though.
> Is modifying nr_requests allowed?
Well we need to do the same things that ll_rw_blk::queue_requests_store()
does, only we don't need to worry about locking or existing queue
contents since the queue has been instantiated but the scsi device
is not active yet.
I do notice now however, that between 2.6.4 and 2.6.9-rc1
blk_queue_congestion_threshold() has been added which we should
probably call after adjusting nr_requests. Unfortunately it's
a static function in ll_rw_blk.c ..
Perhaps we should export the functionality of queue_requests_store()
as, say, queue_adjust_nr_requests() (like scsi_adjust_queue_depth) ?
Jens ?
Anyway, for now, perhaps the mucking with nr_requests should be
taken out and a change like the above should be sent to the
people at 3ware.
I'll submit the sysfs code for inclusion in -mm and the nr_requests
stuff to 3ware.
Mike.
On Wed, Sep 01 2004, Miquel van Smoorenburg wrote:
> On Wed, 01 Sep 2004 21:43:25, Patrick Mansfield wrote:
> >On Wed, Sep 01, 2004 at 11:08:39AM +0000, Miquel van Smoorenburg wrote:
> >
> >> + /* make sure blockdev queue depth is at least 2 * scsi depth */
> >> + if (SDptr->request_queue->nr_requests < 2 * max_cmds)
> >> + SDptr->request_queue->nr_requests = 2 * max_cmds;
> >
> >Why would you want nr_requests different (and larger) only for this
> >driver?
>
> Because for the Linux I/O scheduler to work, nr_requests needs to
> be at least twice as big as the scsi queue depth.
Well, basically if you want to have a chance to do any io scheduling
anywhere, you need to have more than 1 request to play with really. And
if the drive is swallowing all your requests all the time, you are
screwed. I do think the best option (as some people mentioned in this
thread) is to limit the 3ware queue depth, not increase the io scheduler
depth. At least for most of the current io schedulers, this will kill
your latency quite a bit.
> >Is modifying nr_requests allowed?
>
> Well we need to do the same things that ll_rw_blk::queue_requests_store()
> does, only we don't need to worry about locking or existing queue
> contents since the queue has been instantiated but the scsi device
> is not active yet.
>
> I do notice now however, that between 2.6.4 and 2.6.9-rc1
> blk_queue_congestion_threshold() has been added which we should
> probably call after adjusting nr_requests. Unfortunately it's
> a static function in ll_rw_blk.c ..
>
> Perhaps we should export the functionality of queue_requests_store()
> as, say, queue_adjust_nr_requests() (like scsi_adjust_queue_depth) ?
> Jens ?
Yes, if you want to do this, we need to export a function to do it that
takes care of updating the block layer congestion (etc) data.
> Anyway, for now, perhaps the mucking with nr_requests should be
> taken out and a change like the above should be sent to the
> people at 3ware.
Indeed.
> I'll submit the sysfs code for inclusion in -mm and the nr_requests
> stuff to 3ware.
Great!
--
Jens Axboe