Re: [PATCH UPDATED] block: In blk_execute_rq_nowait, init rq->end_iobefore checking for dead queue.

[Muthu Kumar]

On Fri, Jun 22, 2012 at 12:20 AM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
> On 06/21/12 21:40, Muthu Kumar wrote:
>
>> On Wed, Jun 20, 2012 at 10:40 PM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
>>> On 06/21/12 00:53, Muthu Kumar wrote:
>>>> This part already went in, its here only for testing - So in the final
>>>> patch, before applying this should be removed.
>>>
>>> When did it go in ? As far as I can see that patch is not yet present in
>>> Linus' tree nor in James' tree.
>>
>> My bad. I thought Jens/James took it already... may be not.
>
>
> Do you agree with combining both patches into one (see below) ?


Perfect. Thanks.

Regards,
Muthu.









>
> From: Muthukumar Ratty <muthur@xxxxxxxxx>
> Subject: [PATCH] block: Fix blk_execute_rq_nowait() dead queue handling
>
> If the queue is dead blk_execute_rq_nowait() doesn't invoke the done()
> callback function. That will result in blk_execute_rq() being stuck
> in wait_for_completion(). Avoid this by initializing rq->end_io to the
> done() callback before we check the queue state. Also, make sure the
> queue lock is held around the invocation of the done() callback. Found
> this through source code review.
>
> Signed-off-by: Muthukumar Ratty <muthur@xxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Jens Axboe <axboe@xxxxxxxxx>
> Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  block/blk-exec.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index fb2cbd5..8b6dc5b 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -43,6 +43,9 @@ static void blk_end_sync_rq(struct request *rq, int error)
>  * Description:
>  *    Insert a fully prepared request at the back of the I/O scheduler queue
>  *    for execution.  Don't wait for completion.
> + *
> + * Note:
> + *    This function will invoke @done directly if the queue is dead.
>  */
>  void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
>                           struct request *rq, int at_head,
> @@ -51,18 +54,20 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
>        int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
>
>        WARN_ON(irqs_disabled());
> +
> +       rq->rq_disk = bd_disk;
> +       rq->end_io = done;
> +
>        spin_lock_irq(q->queue_lock);
>
>        if (unlikely(blk_queue_dead(q))) {
> -               spin_unlock_irq(q->queue_lock);
>                rq->errors = -ENXIO;
>                if (rq->end_io)
>                        rq->end_io(rq, rq->errors);
> +               spin_unlock_irq(q->queue_lock);
>                return;
>        }
>
> -       rq->rq_disk = bd_disk;
> -       rq->end_io = done;
>        __elv_add_request(q, rq, where);
>        __blk_run_queue(q);
>        /* the queue is stopped so it won't be run */
> --
> 1.7.7
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/