[PATCH] Correct unplugs on nr_queued

From: Jens Axboe <axboe@suse.de>

There's a small discrepancy in when we decide to unplug a queue based on
q->unplug_thresh.  Basically it doesn't work for tagged queues, since
q->rq.count[READ] + q->rq.count[WRITE] is just the number of allocated
requests, not the number of requests stuck in the io scheduler.  We could
just change the nr_queued == to a nr_queued >=, however that is still
suboptimal.

This patch adds accounting for requests that have been dequeued from the io
scheduler, but not freed yet.  These are q->in_flight.  allocated_requests
- q->in_flight == requests_in_scheduler.  So the condition correctly
becomes

	if (requests_in_scheduler == q->unplug_thresh)

instead.  I did a quick round of testing, and for dbench on a SCSI disk the
number of timer induced unplugs was reduced from 13 to 5 :-).  Not a huge
number, but there might be cases where it's more significant.  Either way,
it gets ->unplug_thresh always right, which the old logic didn't.

diff --git a/drivers/block/elevator.c b/drivers/block/elevator.c
index 40377d4a030acfdc40d16d30fe8e1fbdcde23811..c42fd0ddd75f5fa4f5c9a7ebac57327240072575 100644 (file)
--- a/drivers/block/elevator.c
+++ b/drivers/block/elevator.c
@@ -149,6 +149,13 @@ void elv_merge_requests(request_queue_t *q, struct request *rq,
 
 void elv_requeue_request(request_queue_t *q, struct request *rq)
 {
+       /*
+        * it already went through dequeue, we need to decrement the
+        * in_flight count again
+        */
+       if (blk_account_rq(rq))
+               q->in_flight--;
+
        /*
         * if iosched has an explicit requeue hook, then use that. otherwise
         * just put the request at the front of the queue
@@ -232,6 +239,16 @@ void elv_remove_request(request_queue_t *q, struct request *rq)
 {
        elevator_t *e = &q->elevator;
 
+       /*
+        * the time frame between a request being removed from the lists
+        * and to it is freed is accounted as io that is in progress at
+        * the driver side. note that we only account requests that the
+        * driver has seen (REQ_STARTED set), to avoid false accounting
+        * for request-request merges
+        */
+       if (blk_account_rq(rq))
+               q->in_flight++;
+
        /*
         * the main clearing point for q->last_merge is on retrieval of
         * request by driver (it calls elv_next_request()), but it _can_
@@ -321,6 +338,12 @@ void elv_completed_request(request_queue_t *q, struct request *rq)
 {
        elevator_t *e = &q->elevator;
 
+       /*
+        * request is released from the driver, io must be done
+        */
+       if (blk_account_rq(rq))
+               q->in_flight--;
+
        if (e->elevator_completed_req_fn)
                e->elevator_completed_req_fn(q, rq);
 }

diff --git a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c
index 209fdef4d9868a1d602014c424d91036bb5c64ec..6b0ff2c5f092ec24dc96dc85a292fa131e082bca 100644 (file)
--- a/drivers/block/ll_rw_blk.c
+++ b/drivers/block/ll_rw_blk.c
@@ -2275,9 +2275,9 @@ out:
                __blk_put_request(q, freereq);
 
        if (blk_queue_plugged(q)) {
-               int nr_queued = q->rq.count[READ] + q->rq.count[WRITE];
+               int nrq = q->rq.count[READ] + q->rq.count[WRITE] - q->in_flight;
 
-               if (nr_queued == q->unplug_thresh || bio_sync(bio))
+               if (nrq == q->unplug_thresh || bio_sync(bio))
                        __generic_unplug_device(q);
        }
        spin_unlock_irq(q->queue_lock);

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 572f96e6940aee46ceb205d8a4f6d4e08f496fb4..44c722d4b67b87dbf686ecf486b563ce0706f038 100644 (file)
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -348,6 +348,8 @@ struct request_queue
 
        atomic_t                refcnt;
 
+       unsigned int            in_flight;
+
        /*
         * sg stuff
         */
@@ -377,6 +379,9 @@ struct request_queue
 #define blk_fs_request(rq)     ((rq)->flags & REQ_CMD)
 #define blk_pc_request(rq)     ((rq)->flags & REQ_BLOCK_PC)
 #define blk_noretry_request(rq)        ((rq)->flags & REQ_FAILFAST)
+#define blk_rq_started(rq)     ((rq)->flags & REQ_STARTED)
+
+#define blk_account_rq(rq)     (blk_rq_started(rq) && blk_fs_request(rq))
 
 #define blk_pm_suspend_request(rq)     ((rq)->flags & REQ_PM_SUSPEND)
 #define blk_pm_resume_request(rq)      ((rq)->flags & REQ_PM_RESUME)