On Mon, Jan 15 2007, Linas Vepstas wrote:
> 
> Hi Chris, Jens,
> Can you look at this, and push upstream if this looks reasonable
> to you? It fixes a bug I've been tripping over.
> 
> --linas
> 
> 
> A flag was recently added to the elevator code to avoid
> performing an unplug when reuests are being re-queued.
> The goal of this flag was to avoid a deep recursion that
> can occur when re-queueing requests after a SCSI device/host 
> reset.  See http://lkml.org/lkml/2006/5/17/254
> 
> However, that fix added the flag near the bottom of a case
> statement, where an earlier break (in an if statement) could
> transport one out of the case, without setting the flag.
> This patch sets the flag earlier in the case statement.
> 
> I re-discovered the deep recursion recently during testing;
> I was told that it was a known problem, and the fix to it was
> in the kernel I was testing. Indeed it was ... but it didn't
> fix the bug. With the patch below, I no longer see the bug.
> 
> Signed-off by: Linas Vepstas <[EMAIL PROTECTED]>
> Cc: Jens Axboe <[EMAIL PROTECTED]>
> Cc: Chris Wright <[EMAIL PROTECTED]>
> 
> ----
>  block/elevator.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6.20-rc4/block/elevator.c
> ===================================================================
> --- linux-2.6.20-rc4.orig/block/elevator.c    2007-01-15 14:16:03.000000000 
> -0600
> +++ linux-2.6.20-rc4/block/elevator.c 2007-01-15 14:20:04.000000000 -0600
> @@ -590,6 +590,12 @@ void elv_insert(request_queue_t *q, stru
>                */
>               rq->cmd_flags |= REQ_SOFTBARRIER;
>  
> +             /*
> +              * Most requeues happen because of a busy condition,
> +              * don't force unplug of the queue for that case.
> +              */
> +             unplug_it = 0;
> +
>               if (q->ordseq == 0) {
>                       list_add(&rq->queuelist, &q->queue_head);
>                       break;
> @@ -604,11 +610,6 @@ void elv_insert(request_queue_t *q, stru
>               }
>  
>               list_add_tail(&rq->queuelist, pos);
> -             /*
> -              * most requeues happen because of a busy condition, don't
> -              * force unplug of the queue for that case.
> -              */
> -             unplug_it = 0;
>               break;

Ah, yes it definitely should be moved up, thanks for that!

Acked-by: Jens Axboe <[EMAIL PROTECTED]>

I'll get this merged for 2.6.21.

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to