lkml.org 
[lkml]   [2001]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch] cpqarray + cciss bugs/cleanups
Hi,

It's bothered me for some time that cpqarray and cciss release the
request command before completing the command. This doesn't really break
anything in 2.4, but it can later be a bug. Other things:

- Not releasing request on error
- Queueing loop cleanup and optimization
- io_request_lock dropped when not needed
- Don't calculate sector size of request, we already know it

--
Jens Axboe

diff -u /opt/kernel/linux-2.4.14-pre7/drivers/block/cciss.c linux/drivers/block/cciss.c
--- /opt/kernel/linux-2.4.14-pre7/drivers/block/cciss.c Wed Oct 31 09:39:12 2001
+++ linux/drivers/block/cciss.c Fri Nov 2 14:32:47 2001
@@ -1214,7 +1214,12 @@
status=0;
}
}
- complete_buffers(cmd->bh, status);
+ complete_buffers(cmd->rq->bh, status);
+
+#ifdef CCISS_DEBUG
+ printk("Done with %p\n", cmd->rq);
+#endif /* CCISS_DEBUG */
+ end_that_request_last(cmd->rq);
}


@@ -1269,7 +1274,7 @@
{
ctlr_info_t *h= q->queuedata;
CommandList_struct *c;
- int log_unit, start_blk, seg, sect;
+ int log_unit, start_blk, seg;
char *lastdataend;
struct buffer_head *bh;
struct list_head *queue_head = &q->queue_head;
@@ -1278,13 +1283,12 @@
struct my_sg tmp_sg[MAXSGENTRIES];
int i;

- // Loop till the queue is empty if or it is plugged
- while (1)
- {
- if (q->plugged || list_empty(queue_head)) {
- start_io(h);
- return;
- }
+ if (q->plugged)
+ goto startio;
+
+queue_next:
+ if (list_empty(queue_head))
+ goto startio;

creq = blkdev_entry_next_request(queue_head);
if (creq->nr_segments > MAXSGENTRIES)
@@ -1296,17 +1300,18 @@
h->ctlr, creq->rq_dev, creq);
blkdev_dequeue_request(creq);
complete_buffers(creq->bh, 0);
- start_io(h);
- return;
+ end_that_request_last(creq);
+ goto startio;
}

if (( c = cmd_alloc(h, 1)) == NULL)
- {
- start_io(h);
- return;
- }
+ goto startio;
+
+ spin_unlock_irq(&io_request_lock);
+
c->cmd_type = CMD_RWREQ;
- bh = c->bh = creq->bh;
+ bh = creq->bh;
+ c->rq = creq;

/* fill in the request */
log_unit = MINOR(creq->rq_dev) >> NWD_SHIFT;
@@ -1330,10 +1335,8 @@
#endif /* CCISS_DEBUG */
seg = 0;
lastdataend = NULL;
- sect = 0;
while(bh)
{
- sect += bh->b_size/512;
if (bh->b_data == lastdataend)
{ // tack it on to the last segment
tmp_sg[seg-1].len +=bh->b_size;
@@ -1367,7 +1370,7 @@
h->maxSG = seg;

#ifdef CCISS_DEBUG
- printk(KERN_DEBUG "cciss: Submitting %d sectors in %d segments\n", sect, seg);
+ printk(KERN_DEBUG "cciss: Submitting %d sectors in %d segments\n", creq->nr_sectors, seg);
#endif /* CCISS_DEBUG */

c->Header.SGList = c->Header.SGTotal = seg;
@@ -1377,28 +1380,26 @@
c->Request.CDB[4]= (start_blk >> 8) & 0xff;
c->Request.CDB[5]= start_blk & 0xff;
c->Request.CDB[6]= 0; // (sect >> 24) & 0xff; MSB
- c->Request.CDB[7]= (sect >> 8) & 0xff;
- c->Request.CDB[8]= sect & 0xff;
+ c->Request.CDB[7]= (creq->nr_sectors >> 8) & 0xff;
+ c->Request.CDB[8]= creq->nr_sectors & 0xff;
c->Request.CDB[9] = c->Request.CDB[11] = c->Request.CDB[12] = 0;

-
+ spin_lock_irq(&io_request_lock);
+
blkdev_dequeue_request(creq);

-
/*
* ehh, we can't really end the request here since it's not
* even started yet. for now it shouldn't hurt though
*/
-#ifdef CCISS_DEBUG
- printk("Done with %p\n", creq);
-#endif /* CCISS_DEBUG */
- end_that_request_last(creq);
-
addQ(&(h->reqQ),c);
h->Qdepth++;
if(h->Qdepth > h->maxQsinceinit)
h->maxQsinceinit = h->Qdepth;
- } // while loop
+
+ goto queue_next;
+startio:
+ start_io(h);
}

static void do_cciss_intr(int irq, void *dev_id, struct pt_regs *regs)
diff -u /opt/kernel/linux-2.4.14-pre7/drivers/block/cciss_cmd.h linux/drivers/block/cciss_cmd.h
--- /opt/kernel/linux-2.4.14-pre7/drivers/block/cciss_cmd.h Tue May 22 19:23:16 2001
+++ linux/drivers/block/cciss_cmd.h Fri Nov 2 14:24:35 2001
@@ -228,7 +228,7 @@
int cmd_type;
struct _CommandList_struct *prev;
struct _CommandList_struct *next;
- struct buffer_head * bh;
+ struct request * rq;
} CommandList_struct;

//Configuration Table Structure
diff -u /opt/kernel/linux-2.4.14-pre7/drivers/block/cpqarray.c linux/drivers/block/cpqarray.c
--- /opt/kernel/linux-2.4.14-pre7/drivers/block/cpqarray.c Wed Oct 31 09:39:12 2001
+++ linux/drivers/block/cpqarray.c Fri Nov 2 14:32:47 2001
@@ -911,21 +911,19 @@
{
ctlr_info_t *h = q->queuedata;
cmdlist_t *c;
- int seg, sect;
char *lastdataend;
struct list_head * queue_head = &q->queue_head;
struct buffer_head *bh;
struct request *creq;
struct my_sg tmp_sg[SG_MAX];
- int i;
+ int i, seg;

-// Loop till the queue is empty if or it is plugged
- while (1)
-{
- if (q->plugged || list_empty(queue_head)) {
- start_io(h);
- return;
- }
+ if (q->plugged)
+ goto startio;
+
+queue_next:
+ if (list_empty(queue_head))
+ goto startio;

creq = blkdev_entry_next_request(queue_head);
if (creq->nr_segments > SG_MAX)
@@ -937,15 +935,14 @@
h->ctlr, creq->rq_dev, creq);
blkdev_dequeue_request(creq);
complete_buffers(creq->bh, 0);
- start_io(h);
- return;
+ end_that_request_last(creq);
+ goto startio;
}

if ((c = cmd_alloc(h,1)) == NULL)
- {
- start_io(h);
- return;
- }
+ goto startio;
+
+ spin_unlock_irq(&io_request_lock);

bh = creq->bh;

@@ -955,7 +952,7 @@
c->size += sizeof(rblk_t);

c->req.hdr.blk = ida[(h->ctlr<<CTLR_SHIFT) + MINOR(creq->rq_dev)].start_sect + creq->sector;
- c->bh = bh;
+ c->rq = creq;
DBGPX(
if (bh == NULL)
panic("bh == NULL?");
@@ -963,9 +960,7 @@
printk("sector=%d, nr_sectors=%d\n", creq->sector, creq->nr_sectors);
);
seg = 0; lastdataend = NULL;
- sect = 0;
while(bh) {
- sect += bh->b_size/512;
if (bh->b_data == lastdataend) {
tmp_sg[seg-1].size += bh->b_size;
lastdataend += bh->b_size;
@@ -991,26 +986,12 @@
}
DBGPX( printk("Submitting %d sectors in %d segments\n", sect, seg); );
c->req.hdr.sg_cnt = seg;
- c->req.hdr.blk_cnt = sect;
+ c->req.hdr.blk_cnt = creq->nr_sectors;

- /*
- * Since we control our own merging, we know that this request
- * is now fully setup and there's nothing left.
- */
- if (creq->nr_sectors != sect) {
- printk("ida: %ld != %d sectors\n", creq->nr_sectors, sect);
- BUG();
- }
+ spin_lock_irq(&io_request_lock);

blkdev_dequeue_request(creq);

- /*
- * ehh, we can't really end the request here since it's not
- * even started yet. for now it shouldn't hurt though
- */
-DBGPX( printk("Done with %p\n", creq); );
- end_that_request_last(creq);
-
c->req.hdr.cmd = (creq->cmd == READ) ? IDA_READ : IDA_WRITE;
c->type = CMD_RWREQ;

@@ -1019,7 +1000,11 @@
h->Qdepth++;
if (h->Qdepth > h->maxQsinceinit)
h->maxQsinceinit = h->Qdepth;
- } // while loop
+
+ goto queue_next;
+
+startio:
+ start_io(h);
}

/*
@@ -1096,7 +1081,13 @@
cmd->req.sg[i].addr, cmd->req.sg[i].size,
(cmd->req.hdr.cmd == IDA_READ) ? PCI_DMA_FROMDEVICE : PCI_DMA_TODEVICE);
}
- complete_buffers(cmd->bh, ok);
+
+ complete_buffers(cmd->rq->bh, ok);
+
+ DBGPX(printk("Done with %p\n", cmd->rq););
+ end_that_request_last(cmd->rq);
+
+
}

/*
diff -u /opt/kernel/linux-2.4.14-pre7/drivers/block/ida_cmd.h linux/drivers/block/ida_cmd.h
--- /opt/kernel/linux-2.4.14-pre7/drivers/block/ida_cmd.h Wed Jul 25 23:12:01 2001
+++ linux/drivers/block/ida_cmd.h Fri Nov 2 14:24:20 2001
@@ -93,7 +93,7 @@
int ctlr;
struct cmdlist *prev;
struct cmdlist *next;
- struct buffer_head *bh;
+ struct request *rq;
int type;
} cmdlist_t;
\
 
 \ /
  Last update: 2005-03-22 13:12    [W:0.052 / U:1.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site