Hello, Why do cciss/cpqarray directly call "bio_endio" and "disk_stat_add" when request completion instead of using "end_that_request_first"? Are there any problems in using "end_that_request_first"?
BACKGROUND: I'm working on refining request completion procedures like: Before: 2 steps completion using "end_that_request_{first/chunk}" and "end_that_request_last" After: 1 step completion using generic block-layer helper which I'm calling "blk_end_request" "blk_end_request" works like calling "__end_that_request_first" and "end_that_request_last". <http://marc.info/?l=linux-kernel&m=116846938806220&w=2> However, while auditing various drivers, I found some drivers are not using "end_that_request_{first/chunk}". So I have to be careful not to break such drivers. In the case of cciss/cpqarray, they call block layer functions directly instead of using "end_that_request_first". o. Call blk_trace_add_rq() (cpqarray doesn't call it though) o. Call disk_stat_add() o. Complete bios in a request using bio_endio() But "__end_that_request_first" does all of them. So I can't understand why cciss/cpqarray don't use it. Hence the question above. Are there any problems in using "end_that_request_first" like the patch below? If no problem, testing the patch would be very helpful because I have no hardware for those device drivers. (I have done only build test.) If it causes problems, some tricky work may be needed to skip "__end_that_request_first" in "blk_end_request". Thanks, Kiyoshi Ueda Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]> Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]> diff -rupN 2.6.23-rc1-mm1/drivers/block/cciss.c refine/drivers/block/cciss.c --- 2.6.23-rc1-mm1/drivers/block/cciss.c 2007-07-25 14:00:10.000000000 -0400 +++ refine/drivers/block/cciss.c 2007-07-26 11:29:32.000000000 -0400 @@ -1250,6 +1250,7 @@ static void cciss_softirq_done(struct re unsigned long flags; u64bit temp64; int i, ddir; + int uptodate = rq->errors ? 0 : 1; if (cmd->Request.Type.Direction == XFER_READ) ddir = PCI_DMA_FROMDEVICE; @@ -1264,13 +1265,7 @@ static void cciss_softirq_done(struct re pci_unmap_page(h->pdev, temp64.val, cmd->SG[i].Len, ddir); } - complete_buffers(rq->bio, (rq->errors == 0)); - - if (blk_fs_request(rq)) { - const int rw = rq_data_dir(rq); - - disk_stat_add(rq->rq_disk, sectors[rw], rq->nr_sectors); - } + end_that_request_first(rq, uptodate, rq->hard_nr_sectors); #ifdef CCISS_DEBUG printk("Done with %p\n", rq); @@ -1278,7 +1273,7 @@ static void cciss_softirq_done(struct re add_disk_randomness(rq->rq_disk); spin_lock_irqsave(&h->lock, flags); - end_that_request_last(rq, (rq->errors == 0)); + end_that_request_last(rq, uptodate); cmd_free(h, cmd, 1); cciss_check_queues(h); spin_unlock_irqrestore(&h->lock, flags); @@ -2504,7 +2499,6 @@ after_error_processing: } cmd->rq->data_len = 0; cmd->rq->completion_data = cmd; - blk_add_trace_rq(cmd->rq->q, cmd->rq, BLK_TA_COMPLETE); blk_complete_request(cmd->rq); } diff -rupN 2.6.23-rc1-mm1/drivers/block/cpqarray.c refine/drivers/block/cpqarray.c --- 2.6.23-rc1-mm1/drivers/block/cpqarray.c 2007-07-25 14:00:10.000000000 -0400 +++ refine/drivers/block/cpqarray.c 2007-07-26 11:30:23.000000000 -0400 @@ -1030,13 +1030,7 @@ static inline void complete_command(cmdl pci_unmap_page(hba[cmd->ctlr]->pci_dev, cmd->req.sg[i].addr, cmd->req.sg[i].size, ddir); - complete_buffers(rq->bio, ok); - - if (blk_fs_request(rq)) { - const int rw = rq_data_dir(rq); - - disk_stat_add(rq->rq_disk, sectors[rw], rq->nr_sectors); - } + end_that_request_first(rq, ok, rq->hard_nr_sectors); add_disk_randomness(rq->rq_disk); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/