Re: [PATCH] blk-exec-assign-endio-before-queue-dead-check

[Muthu Kumar]

On Wed, Jun 6, 2012 at 10:53 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello,
>
> On Thu, Jun 7, 2012 at 2:42 PM, Muthu Kumar <muthu.lkml@xxxxxxxxx> wrote:
>>>> One more thing to consider is, the completion function is called from
>>>> the same calling context here. As far as my check, it looks ok. Let me
>>>> know if you think otherwise.
>>>
>>> Not sure what you mean.
>>
>> If there is no error, then the completion routine is called in a
>> different context - rq completion context. But here, we call the
>> completion routine in the same context of the caller.
>
> Ah, okay. I think the only problem there would be that the end_io
> callback is being called outside queue lock. Can you please take care
> of that too?
>

Revised patch (below as well as attached)

>> blk-exec.c: In blk_execute_rq_nowait(), if the queue is dead, call to
>> done() routine is not made. That will result in blk_execute_rq() stuck
>> in wait_for_completion(). Avoid this by initializing rq->end_io to
>> done() routine before we check for dead queue.
>
> Yeah, sounds about right.
>
> Thanks.
>
> --
> tejun


-----------------------
blk-exec.c: In blk_execute_rq_nowait(), if the queue is dead, call to
done() routine is not made. That will result in blk_execute_rq() stuck
in wait_for_completion(). Avoid this by initializing rq->end_io to
done() routine before we check for dead queue.

Signed-off-by: Muthukumar Ratty <muthur@xxxxxxxxx>
CC: Tejun Heo <tj@xxxxxxxxxx>
CC: Jens Axboe <axboe@xxxxxxxxx>
CC: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>

-----------------------

diff --git a/block/blk-exec.c b/block/blk-exec.c
index fb2cbd5..284bf56 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -53,16 +53,17 @@ void blk_execute_rq_nowait(struct request_queue
*q, struct gendisk *bd_disk,
        WARN_ON(irqs_disabled());
        spin_lock_irq(q->queue_lock);

+       rq->rq_disk = bd_disk;
+       rq->end_io = done;
+
        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 */
-----------------------
blk-exec.c: In blk_execute_rq_nowait(), if the queue is dead, call to 
done() routine is not made. That will result in blk_execute_rq() stuck
in wait_for_completion(). Avoid this by initializing rq->end_io to
done() routine before we check for dead queue.

Signed-off-by: Muthukumar Ratty <muthur@xxxxxxxxx>
CC: Tejun Heo <tj@xxxxxxxxxx>
CC: Jens Axboe <axboe@xxxxxxxxx>
CC: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>

-----------------------

diff --git a/block/blk-exec.c b/block/blk-exec.c
index fb2cbd5..284bf56 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -53,16 +53,17 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 	WARN_ON(irqs_disabled());
 	spin_lock_irq(q->queue_lock);
 
+	rq->rq_disk = bd_disk;
+	rq->end_io = done;
+
 	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 */