* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] [not found] ` <1094032735l.3189l.7l@traveler> @ 2004-09-01 10:09 ` Christoph Hellwig 2004-09-01 11:08 ` Miquel van Smoorenburg 0 siblings, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2004-09-01 10:09 UTC (permalink / raw) To: Miquel van Smoorenburg; +Cc: lkml, Timothy Miller, linux-kernel, linux-scsi 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] 2004-09-01 10:09 ` 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] Christoph Hellwig @ 2004-09-01 11:08 ` Miquel van Smoorenburg 2004-09-01 11:43 ` Christoph Hellwig 2004-09-01 19:43 ` Patrick Mansfield 0 siblings, 2 replies; 6+ messages in thread From: Miquel van Smoorenburg @ 2004-09-01 11:08 UTC (permalink / raw) To: Christoph Hellwig Cc: Miquel van Smoorenburg, lkml, Timothy Miller, linux-kernel, linux-scsi 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 <miquels@cistron.nl> --- 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" ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] 2004-09-01 11:08 ` Miquel van Smoorenburg @ 2004-09-01 11:43 ` Christoph Hellwig 2004-09-01 19:43 ` Patrick Mansfield 1 sibling, 0 replies; 6+ messages in thread From: Christoph Hellwig @ 2004-09-01 11:43 UTC (permalink / raw) To: Miquel van Smoorenburg Cc: Christoph Hellwig, lkml, Timothy Miller, linux-kernel, linux-scsi 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] 2004-09-01 11:08 ` Miquel van Smoorenburg 2004-09-01 11:43 ` Christoph Hellwig @ 2004-09-01 19:43 ` Patrick Mansfield 2004-09-01 22:23 ` Miquel van Smoorenburg 1 sibling, 1 reply; 6+ messages in thread From: Patrick Mansfield @ 2004-09-01 19:43 UTC (permalink / raw) To: Miquel van Smoorenburg Cc: Christoph Hellwig, lkml, Timothy Miller, Jens Axboe, linux-kernel, linux-scsi 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] 2004-09-01 19:43 ` Patrick Mansfield @ 2004-09-01 22:23 ` Miquel van Smoorenburg 2004-09-04 10:10 ` Jens Axboe 0 siblings, 1 reply; 6+ messages in thread From: Miquel van Smoorenburg @ 2004-09-01 22:23 UTC (permalink / raw) To: Patrick Mansfield Cc: Miquel van Smoorenburg, Christoph Hellwig, lkml, Timothy Miller, Jens Axboe, linux-kernel, linux-scsi 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] 2004-09-01 22:23 ` Miquel van Smoorenburg @ 2004-09-04 10:10 ` Jens Axboe 0 siblings, 0 replies; 6+ messages in thread From: Jens Axboe @ 2004-09-04 10:10 UTC (permalink / raw) To: Miquel van Smoorenburg Cc: Patrick Mansfield, Christoph Hellwig, lkml, Timothy Miller, linux-kernel, linux-scsi 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-09-04 10:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200408021602.34320.swsnyder@insightbb.com>
[not found] ` <1094030083l.3189l.2l@traveler>
[not found] ` <1094030194l.3189l.3l@traveler>
[not found] ` <200409010233.31643.lkml@lpbproductions.com>
[not found] ` <1094032735l.3189l.7l@traveler>
2004-09-01 10:09 ` 3ware queue depth [was: Re: HIGHMEM4G config for 1GB RAM on desktop?] Christoph Hellwig
2004-09-01 11:08 ` Miquel van Smoorenburg
2004-09-01 11:43 ` Christoph Hellwig
2004-09-01 19:43 ` Patrick Mansfield
2004-09-01 22:23 ` Miquel van Smoorenburg
2004-09-04 10:10 ` Jens Axboe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox