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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20070116002733.GC4067@kernel.dk>
Date:	Tue, 16 Jan 2007 11:27:33 +1100
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Linas Vepstas <linas@...tin.ibm.com>
Cc:	Jens Axboe <axboe@...e.de>, Chris Wright <chrisw@...s-sol.org>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Nick Piggin <npiggin@...e.de>, Bino.Sebastian@...lex.Com,
	James.Smart@...lex.Com, rlary@...ibm.com, Laurie.Barry@...lex.Com,
	strosake@...ibm.com, vaios.papadimitriou@...lex.Com
Subject: Re: [PATCH] adjust use of unplug in elevator code

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@...tin.ibm.com>
> Cc: Jens Axboe <axboe@...e.de>
> Cc: Chris Wright <chrisw@...s-sol.org>
> 
> ----
>  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@...cle.com>

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/