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

This is a discussion on [PATCH] elevator.c: prevent flushing small requests to device - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 2 of 2

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

  1. [PATCH] 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

    diff -ruN linux-2.6.26.5.orig/block/elevator.c linux-2.6.26.5/block/elevator.c
    --- linux-2.6.26.5.orig/block/elevator.c 2008-09-08 19:40:20.000000000 +0200
    +++ linux-2.6.26.5/block/elevator.c 2008-10-23 00:33:21.000000000 +0200
    @@ -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)) {
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    On Thu, Oct 23 2008, Miquel van Smoorenburg wrote:
    > 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
    >
    > diff -ruN linux-2.6.26.5.orig/block/elevator.c linux-2.6.26.5/block/elevator.c
    > --- linux-2.6.26.5.orig/block/elevator.c 2008-09-08 19:40:20.000000000 +0200
    > +++ linux-2.6.26.5/block/elevator.c 2008-10-23 00:33:21.000000000 +0200
    > @@ -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)) {


    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.

    --
    Jens Axboe

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread