Re: [PATCH] adjust use of unplug in elevator code

[Jens Axboe]

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 <linas@xxxxxxxxxxxxxx>
> Cc: Jens Axboe <axboe@xxxxxxx>
> Cc: Chris Wright <chrisw@xxxxxxxxxxxx>
> 
> ----
>  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 <jens.axboe@xxxxxxxxxx>

I'll get this merged for 2.6.21.

-- 
Jens Axboe

-
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/