xfs
[Top] [All Lists]

Re: XFS vs Elevators (was Re: [PATCH RFC] nilfs2: continuous snapshottin

To: david@xxxxxxxxxxxxx
Subject: Re: XFS vs Elevators (was Re: [PATCH RFC] nilfs2: continuous snapshotting file system)
From: Aaron Carroll <aaronc@xxxxxxxxxxxxxxxxxx>
Date: Thu, 21 Aug 2008 18:07:38 +1000
Cc: Szabolcs Szakacsits <szaka@xxxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <20080821060418.GC5706@disturbed>
References: <20080820004326.519405a2.akpm@xxxxxxxxxxxxxxxxxxxx> <200808201613.AA00212@xxxxxxxxxxxxxxxxxxxxxx> <Pine.LNX.4.61.0808202352450.4532@dhcppc2> <20080820143916.1a7eddab.akpm@xxxxxxxxxxxxxxxxxxxx> <20080821021259.GA5706@disturbed> <Pine.LNX.4.62.0808210535450.25448@xxxxxxxxxxxxxxxxxxx> <20080821051508.GB5706@disturbed> <20080821060418.GC5706@disturbed>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.14 (X11/20080618)
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

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;
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;
<Prev in Thread] Current Thread [Next in Thread>