This patch adds per logical device queues to the HP cciss driver. It currently only implements a single lock but when time permits I will provide that funtionality. Thanks to Jeff Garzik for providing some sample code.
This patch built against 2.6.5. Please consider this for inclusion.
mikem
-------------------------------------------------------------------------------
diff -burpN lx265.orig/drivers/block/cciss.c lx265.save/drivers/block/cciss.c
--- lx265.orig/drivers/block/cciss.c 2004-04-03 21:38:20.000000000 -0600
+++ lx265.save/drivers/block/cciss.c 2004-04-06 10:22:08.000000000 -0500
@@ -988,7 +988,7 @@ static int revalidate_allvol(ctlr_info_t
drive_info_struct *drv = &(host->drv[i]);
if (!drv->nr_blocks)
continue;
- blk_queue_hardsect_size(host->queue, drv->block_size);
+ blk_queue_hardsect_size(drv->queue, drv->block_size);
set_capacity(disk, drv->nr_blocks);
add_disk(disk);
}
@@ -2013,7 +2013,7 @@ static irqreturn_t do_cciss_intr(int irq
CommandList_struct *c;
unsigned long flags;
__u32 a, a1;
-
+ int j;
/* Is this interrupt for us? */
if (( h->access.intr_pending(h) == 0) || (h->interrupts_enabled == 0))
@@ -2059,11 +2059,18 @@ static irqreturn_t do_cciss_intr(int irq
}
}
}
-
/*
* See if we can queue up some more IO
+ * check every disk that exists on this controller
+ * and start it's IO
*/
- blk_start_queue(h->queue);
+ for(j=0;j < NWD; j++) {
+ /* make sure the disk has been added and the drive is real */
+ /* because this can be called from the middle of init_one */
+ if(!(h->gendisk[j]->queue) || !(h->drv[j].nr_blocks) )
+ continue;
+ blk_start_queue(h->gendisk[j]->queue);
+ }
spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
return IRQ_HANDLED;
}
@@ -2510,7 +2517,6 @@ static void free_hba(int i)
static int __devinit cciss_init_one(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
- request_queue_t *q;
int i;
int j;
@@ -2568,13 +2574,6 @@ static int __devinit cciss_init_one(stru
}
spin_lock_init(&hba[i]->lock);
- q = blk_init_queue(do_cciss_request, &hba[i]->lock);
- if (!q)
- goto clean4;
-
- q->backing_dev_info.ra_pages = READ_AHEAD;
- hba[i]->queue = q;
- q->queuedata = hba[i];
/* Initialize the pdev driver private data.
have it point to hba[i]. */
@@ -2596,6 +2595,19 @@ static int __devinit cciss_init_one(stru
cciss_procinit(i);
+ for(j=0; j<NWD; j++) {
+ drive_info_struct *drv = &(hba[i]->drv[j]);
+ struct gendisk *disk = hba[i]->gendisk[j];
+ request_queue_t *q;
+
+ q = blk_init_queue(do_cciss_request, &hba[i]->lock);
+ if (!q) {
+ printk(KERN_ERR
+ "cciss: unable to allocate queue for disk %d\n",
+ j);
+ break;
+ }
+ drv->queue = q;
blk_queue_bounce_limit(q, hba[i]->pdev->dma_mask);
/* This is a hardware imposed limit. */
@@ -2606,21 +2618,17 @@ static int __devinit cciss_init_one(stru
blk_queue_max_sectors(q, 512);
-
- for(j=0; j<NWD; j++) {
- drive_info_struct *drv = &(hba[i]->drv[j]);
- struct gendisk *disk = hba[i]->gendisk[j];
-
+ q->queuedata = hba[i];
sprintf(disk->disk_name, "cciss/c%dd%d", i, j);
sprintf(disk->devfs_name, "cciss/host%d/target%d", i, j);
disk->major = COMPAQ_CISS_MAJOR + i;
disk->first_minor = j << NWD_SHIFT;
disk->fops = &cciss_fops;
- disk->queue = hba[i]->queue;
+ disk->queue = q;
disk->private_data = drv;
if( !(drv->nr_blocks))
continue;
- blk_queue_hardsect_size(hba[i]->queue, drv->block_size);
+ blk_queue_hardsect_size(q, drv->block_size);
set_capacity(disk, drv->nr_blocks);
add_disk(disk);
}
@@ -2690,9 +2698,9 @@ static void __devexit cciss_remove_one (
struct gendisk *disk = hba[i]->gendisk[j];
if (disk->flags & GENHD_FL_UP)
del_gendisk(disk);
+ blk_cleanup_queue(disk->queue);
}
- blk_cleanup_queue(hba[i]->queue);
pci_free_consistent(hba[i]->pdev, NR_CMDS * sizeof(CommandList_struct),
hba[i]->cmd_pool, hba[i]->cmd_pool_dhandle);
pci_free_consistent(hba[i]->pdev, NR_CMDS * sizeof( ErrorInfo_struct),
diff -burpN lx265.orig/drivers/block/cciss.h lx265.save/drivers/block/cciss.h
--- lx265.orig/drivers/block/cciss.h 2004-04-03 21:36:13.000000000 -0600
+++ lx265.save/drivers/block/cciss.h 2004-04-06 10:22:08.000000000 -0500
@@ -27,6 +27,7 @@ typedef struct _drive_info_struct
{
__u32 LunID;
int usage_count;
+ struct request_queue *queue;
sector_t nr_blocks;
int block_size;
int heads;
@@ -69,7 +70,6 @@ struct ctlr_info
unsigned int maxQsinceinit;
unsigned int maxSG;
spinlock_t lock;
- struct request_queue *queue;
//* pointers to command and error info pool */
CommandList_struct *cmd_pool;
@@ -252,7 +252,7 @@ struct board_type {
struct access_method *access;
};
-#define CCISS_LOCK(i) (hba[i]->queue->queue_lock)
+#define CCISS_LOCK(i) (&(hba[i]->lock))
#endif /* CCISS_H */
[email protected] wrote:
> This patch adds per logical device queues to the HP cciss driver. It currently only implements a single lock but when time permits I will provide that funtionality. Thanks to Jeff Garzik for providing some sample code.
> This patch built against 2.6.5. Please consider this for inclusion.
I appreciate the credit but I don't see that it addressed my original
objection -- the starvation issue.
Do you cap the number of per-array requests a "1024 / n_arrays", or
something like that? You mentioned that the hardware has a maximum of
1024 outstanding commands, for all devices. The two typical solutions
are a round-robin queue (see carmel.c) or limiting each array such that
if all arrays are full of commands, the total outstanding never exceeds
1024.
This patch may be OK for -mm, I would rather not see it go upstream --
it seems to me you are choosing to decrease stability to obtain a
performance increase. I think you can increase performance without
decreasing stability.
Jeff
Yep, you're right. I just regurgitated the same code. I'll pull my head out and try again :(
mikem
-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Wednesday, April 07, 2004 11:15 AM
To: Miller, Mike (OS Dev)
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: cciss updates for 2.6.6xxx [1/2]
[email protected] wrote:
> This patch adds per logical device queues to the HP cciss driver. It currently only implements a single lock but when time permits I will provide that funtionality. Thanks to Jeff Garzik for providing some sample code.
> This patch built against 2.6.5. Please consider this for inclusion.
I appreciate the credit but I don't see that it addressed my original
objection -- the starvation issue.
Do you cap the number of per-array requests a "1024 / n_arrays", or
something like that? You mentioned that the hardware has a maximum of
1024 outstanding commands, for all devices. The two typical solutions
are a round-robin queue (see carmel.c) or limiting each array such that
if all arrays are full of commands, the total outstanding never exceeds
1024.
This patch may be OK for -mm, I would rather not see it go upstream --
it seems to me you are choosing to decrease stability to obtain a
performance increase. I think you can increase performance without
decreasing stability.
Jeff
Miller, Mike (OS Dev) wrote:
> Yep, you're right. I just regurgitated the same code. I'll pull my head out and try again :(
The easiest thing to do may be to take your patch #1, and then add code
to clamp the per-queue outstanding-command (tag) depth to
1024 / n_arrays_found
at initialization time. Or perhaps s/n_arrays_found/max_arrays_per_hba/
I bet that's just a few additional lines of code, and should work...
Regards,
Jeff
I like the idea of capping max commands based on the number of arrays. One problem is that we can add or remove a logical drive during runtime. How would Linux handle us reshuffling the max commands for each queue?
mikem
-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Wednesday, April 07, 2004 11:34 AM
To: Miller, Mike (OS Dev)
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: cciss updates for 2.6.6xxx [1/2]
Miller, Mike (OS Dev) wrote:
> Yep, you're right. I just regurgitated the same code. I'll pull my head out and try again :(
The easiest thing to do may be to take your patch #1, and then add code
to clamp the per-queue outstanding-command (tag) depth to
1024 / n_arrays_found
at initialization time. Or perhaps s/n_arrays_found/max_arrays_per_hba/
I bet that's just a few additional lines of code, and should work...
Regards,
Jeff
Miller, Mike (OS Dev) wrote:
> I like the idea of capping max commands based on the number of arrays. One problem is that we can add or remove a logical drive during runtime. How would Linux handle us reshuffling the max commands for each queue?
What is the maximum number of logical drives?
If you always assume the maximum number of logical drives are configured
(let's say 32), then you can calculate based on that:
1024 / 32 logical drives == 32 commands per logical drive
It certainly gets a bit more complex, if you truly want to handle
something other than "hardcode max # of logical drives"... I would
suggest going to a round-robin anti-starvation approach at that point,
like what I do in carmel.c. The Carmel hardware has a single hardware
queue for all of its 8 SATA ports, which is similar to cciss having 1024
commands for all of its logical drives (if I understand things correctly).
Jeff