Re: XFS vs Elevators (was Re: [PATCH RFC] nilfs2: continuous snapshotting file system)

Aaron Carroll
2008-08-21T08:11:14Z

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

Dave Chinner wrote:
> One thing I just found out - my old *laptop* is 4-5x faster than the
> 10krpm scsi disk behind an old cciss raid controller. I'm wondering
> if the long delays in dispatch is caused by an interaction with CTQ
> but I can't change it on the cciss raid controllers. Are you using
> ctq/ncq on your machine? If so, can you reduce the depth to
> something less than 4 and see what difference that makes?

I've been benchmarking on a cciss card, and patched the driver to
control the queue depth via sysfs. Maybe you'll find it useful...

The original patch was for 2.6.24, but that won't apply on git head.
I fixed it for 2.6.27, and it seems to work fine. Both are attached.

-- Aaron

[-- Attachment #2: cciss_qdepth-2.6.24.patch --]
[-- Type: text/plain, Size: 4025 bytes --]

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 55bd35c..709c419 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -474,7 +474,7 @@ static CommandList_struct *cmd_alloc(ctlr_info_t *h, int get_from_pool)

do {
i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
- if (i == h->nr_cmds)
+ if (i >= h->qdepth_max)
return NULL;
} while (test_and_set_bit
(i & (BITS_PER_LONG - 1),
@@ -1257,7 +1257,7 @@ static void cciss_check_queues(ctlr_info_t *h)
* in case the interrupt we serviced was from an ioctl and did not
* free any new commands.
*/
- if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) == h->nr_cmds)
+ if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) >= h->qdepth_max)
return;

/* We have room on the queue for more commands. Now we need to queue
@@ -1276,7 +1276,7 @@ static void cciss_check_queues(ctlr_info_t *h)
/* check to see if we have maxed out the number of commands
* that can be placed on the queue.
*/
- if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) == h->nr_cmds) {
+ if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) >= h->qdepth_max) {
if (curr_queue == start_queue) {
h->next_to_run =
(start_queue + 1) % (h->highest_lun + 1);
@@ -3075,6 +3075,7 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
c->product_name = products[i].product_name;
c->access = *(products[i].access);
c->nr_cmds = products[i].nr_cmds;
+ c->qdepth_max = products[i].nr_cmds;
break;
}
}
@@ -3095,6 +3096,7 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
c->product_name = products[i-1].product_name;
c->access = *(products[i-1].access);
c->nr_cmds = products[i-1].nr_cmds;
+ c->qdepth_max = products[i-1].nr_cmds;
printk(KERN_WARNING "cciss: This is an unknown "
"Smart Array controller.\n"
"cciss: Please update to the latest driver "
@@ -3346,6 +3348,44 @@ static void free_hba(int i)
kfree(p);
}

+static inline ctlr_info_t *cciss_get_ctlr_info(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ return pci_get_drvdata(pdev);
+}
+
+static ssize_t cciss_show_queue_depth(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ctlr_info_t *ctlr = cciss_get_ctlr_info(dev);
+ BUG_ON(!ctlr);
+
+ return sprintf(buf, "%u\n", ctlr->qdepth_max);
+}
+
+static ssize_t cciss_store_queue_depth(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ ctlr_info_t *ctlr = cciss_get_ctlr_info(dev);
+ unsigned long qdepth_max;
+
+ BUG_ON(!ctlr);
+ qdepth_max = simple_strtoul(buf, NULL, 10);
+
+ if (qdepth_max < 1)
+ qdepth_max = 1;
+ else if (qdepth_max > ctlr->nr_cmds)
+ qdepth_max = ctlr->nr_cmds;
+
+ ctlr->qdepth_max = (unsigned)qdepth_max;
+ return count;
+}
+
+static struct device_attribute cciss_queue_depth =
+ __ATTR(queue_depth, S_IRUGO | S_IWUSR,
+ &cciss_show_queue_depth,
+ &cciss_store_queue_depth);
+
/*
* This is it. Find all the controllers and register them. I really hate
* stealing all these major device numbers.
@@ -3450,6 +3490,11 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
((hba[i]->nr_cmds + BITS_PER_LONG -
1) / BITS_PER_LONG) * sizeof(unsigned long));

+ /* Setup queue_depth sysfs entry */
+ rc = device_create_file(&pdev->dev, &cciss_queue_depth);
+ if (rc)
+ goto clean4;
+
#ifdef CCISS_DEBUG
printk(KERN_DEBUG "Scanning for drives on controller cciss%d\n", i);
#endif /* CCISS_DEBUG */
diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index b70988d..6a4a38a 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -60,6 +60,7 @@ struct ctlr_info
void __iomem *vaddr;
unsigned long paddr;
int nr_cmds; /* Number of commands allowed on this controller */
+ unsigned qdepth_max; /* userspace queue depth limit */
CfgTable_struct __iomem *cfgtable;
int interrupts_enabled;
int major;

[-- Attachment #3: cciss_qdepth-2.6.27.patch --]
[-- Type: text/plain, Size: 3974 bytes --]

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b73116e..066577f 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -480,7 +480,7 @@ static CommandList_struct *cmd_alloc(ctlr_info_t *h, int get_from_pool)

do {
i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
- if (i == h->nr_cmds)
+ if (i >= h->qdepth_max)
return NULL;
} while (test_and_set_bit
(i & (BITS_PER_LONG - 1),
@@ -1259,7 +1259,7 @@ static void cciss_check_queues(ctlr_info_t *h)
* in case the interrupt we serviced was from an ioctl and did not
* free any new commands.
*/
- if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) == h->nr_cmds)
+ if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) >= h->qdepth_max)
return;

/* We have room on the queue for more commands. Now we need to queue
@@ -1278,7 +1278,7 @@ static void cciss_check_queues(ctlr_info_t *h)
/* check to see if we have maxed out the number of commands
* that can be placed on the queue.
*/
- if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) == h->nr_cmds) {
+ if ((find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds)) >= h->qdepth_max) {
if (curr_queue == start_queue) {
h->next_to_run =
(start_queue + 1) % (h->highest_lun + 1);
@@ -3253,6 +3253,7 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
c->product_name = products[i].product_name;
c->access = *(products[i].access);
c->nr_cmds = c->max_commands - 4;
+ c->qdepth_max = c->nr_cmds;
break;
}
}
@@ -3273,6 +3274,7 @@ static int __devinit cciss_pci_init(ctlr_info_t *c, struct pci_dev *pdev)
c->product_name = products[i-1].product_name;
c->access = *(products[i-1].access);
c->nr_cmds = c->max_commands - 4;
+ c->qdepth_max = c->nr_cmds;
printk(KERN_WARNING "cciss: This is an unknown "
"Smart Array controller.\n"
"cciss: Please update to the latest driver "
@@ -3392,6 +3394,44 @@ static void free_hba(int i)
kfree(p);
}

+static inline ctlr_info_t *cciss_get_ctlr_info(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ return pci_get_drvdata(pdev);
+}
+
+static ssize_t cciss_show_queue_depth(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ctlr_info_t *ctlr = cciss_get_ctlr_info(dev);
+ BUG_ON(!ctlr);
+
+ return sprintf(buf, "%u\n", ctlr->qdepth_max);
+}
+
+static ssize_t cciss_store_queue_depth(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ ctlr_info_t *ctlr = cciss_get_ctlr_info(dev);
+ unsigned long qdepth_max;
+
+ BUG_ON(!ctlr);
+ qdepth_max = simple_strtoul(buf, NULL, 10);
+
+ if (qdepth_max < 1)
+ qdepth_max = 1;
+ else if (qdepth_max > ctlr->nr_cmds)
+ qdepth_max = ctlr->nr_cmds;
+
+ ctlr->qdepth_max = (unsigned)qdepth_max;
+ return count;
+}
+
+static struct device_attribute cciss_queue_depth =
+ __ATTR(queue_depth, S_IRUGO | S_IWUSR,
+ &cciss_show_queue_depth,
+ &cciss_store_queue_depth);
+
/*
* This is it. Find all the controllers and register them. I really hate
* stealing all these major device numbers.
@@ -3496,6 +3536,11 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
((hba[i]->nr_cmds + BITS_PER_LONG -
1) / BITS_PER_LONG) * sizeof(unsigned long));

+ /* Setup queue_depth sysfs entry */
+ rc = device_create_file(&pdev->dev, &cciss_queue_depth);
+ if (rc)
+ goto clean4;
+
hba[i]->num_luns = 0;
hba[i]->highest_lun = -1;
for (j = 0; j < CISS_MAX_LUN; j++) {
diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index 24a7efa..91dcac6 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -62,6 +62,7 @@ struct ctlr_info
void __iomem *vaddr;
unsigned long paddr;
int nr_cmds; /* Number of commands allowed on this controller */
+ unsigned qdepth_max; /* userspace queue depth limit */
CfgTable_struct __iomem *cfgtable;
int interrupts_enabled;
int major;