elevator.c: prevent flushing small requests to device

PATCH In review Miquel van Smoorenburg 2 participants 2 messages Oct 23, 2008 – Oct 31, 2008 Block layer

lore

Share & tools

lore

Miquel van Smoorenburg

author

2008-10-23 16:13 UTC #0 lore raw

PATCH 1/1 elevator.c: prevent flushing small requests to device

While tracing I/O patterns with blktrace (a great tool) a few weeks ago I identified a minor issue in elevator.c

[PATCH] elevator.c: prevent flushing small requests to device

When elv_insert() is called to insert a new request, and the device is plugged, bit of code at the end of the function
will unplug the device if the number of pending requests >= q->unplug_thresh.

This means the current request is also sent to the device immidiately while it is potentially mergeable with the next request. This has been observed when a lot of small sequential requests are made.

By unplugging the device before we add the new request to the queue this can be prevented.

Signed-off-by Miquel van Smoorenburg <[email protected]>

block/elevator.c +14 -0

diff -ruN linux-2.6.26.5.orig/block/elevator.c linux-2.6.26.5/block/elevator.c
@@ -602,6 +602,20 @@
 
 	case ELEVATOR_INSERT_SORT:
 		BUG_ON(!blk_fs_request(rq));
+		/*
+		 * If we're going to unplug the device, do it now before
+		 * we put a potentially small and mergeable new
+		 * request on the queue, instead of just after it.
+		 */
+		if (blk_queue_plugged(q)) {
+			int nrq = q->rq.count[READ] + q->rq.count[WRITE]
+				- q->in_flight;
+			if (nrq >= q->unplug_thresh)
+				__generic_unplug_device(q);
+			if (elv_queue_empty(q))
+				blk_plug_device(q);
+			unplug_it = 0;
+		}
 		rq->cmd_flags |= REQ_SORTED;
 		q->nr_sorted++;
 		if (rq_mergeable(rq)) {

Jens Axboe

2008-10-31 07:35 UTC #1 lore raw

Quoted 32 lines · Miquel van Smoorenburg

While tracing I/O patterns with blktrace (a great tool) a few weeks ago I identified a minor issue in elevator.c
[PATCH] elevator.c: prevent flushing small requests to device
When elv_insert() is called to insert a new request, and the device is plugged, bit of code at the end of the function
will unplug the device if the number of pending requests >= q->unplug_thresh.
This means the current request is also sent to the device immidiately while it is potentially mergeable with the next request. This has been observed when a lot of small sequential requests are made.
By unplugging the device before we add the new request to the queue this can be prevented.
Signed-off-by Miquel van Smoorenburg <[email protected]>
diff -ruN linux-2.6.26.5.orig/block/elevator.c linux-2.6.26.5/block/elevator.c
block/elevator.c +7 -0
@@ -602,6 +602,20 @@
 
 	case ELEVATOR_INSERT_SORT:
 		BUG_ON(!blk_fs_request(rq));
+		/*
+		 * If we're going to unplug the device, do it now before
+		 * we put a potentially small and mergeable new
+		 * request on the queue, instead of just after it.
+		 */
+		if (blk_queue_plugged(q)) {
+			int nrq = q->rq.count[READ] + q->rq.count[WRITE]

block/elevator.c line 621

Quoted 10 lines

block/elevator.c +7 -0

+				- q->in_flight;
+			if (nrq >= q->unplug_thresh)
+				__generic_unplug_device(q);
+			if (elv_queue_empty(q))
+				blk_plug_device(q);
+			unplug_it = 0;
+		}
 		rq->cmd_flags |= REQ_SORTED;
 		q->nr_sorted++;
 		if (rq_mergeable(rq)) {

This makes sense, I'll throw it into the 2.6.29 mix for some testing. I've slightly modified your patch, there's no need to check for queue empty and plug on your own, __make_request() will do that.

Signature · 1 line

Jens Axboe