LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jens Axboe <jens.axboe@oracle.com>
To: Linas Vepstas <linas@austin.ibm.com>
Cc: Jens Axboe <axboe@suse.de>, Chris Wright <chrisw@sous-sol.org>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nick Piggin <npiggin@suse.de>,
	Bino.Sebastian@Emulex.Com, James.Smart@Emulex.Com,
	rlary@us.ibm.com, Laurie.Barry@Emulex.Com, strosake@us.ibm.com,
	vaios.papadimitriou@Emulex.Com
Subject: Re: [PATCH] adjust use of unplug in elevator code
Date: Tue, 16 Jan 2007 11:27:33 +1100	[thread overview]
Message-ID: <20070116002733.GC4067@kernel.dk> (raw)
In-Reply-To: <20070115215117.GD13157@austin.ibm.com>

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@austin.ibm.com>
> Cc: Jens Axboe <axboe@suse.de>
> Cc: Chris Wright <chrisw@sous-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@oracle.com>

I'll get this merged for 2.6.21.

-- 
Jens Axboe


      reply	other threads:[~2007-01-16  0:40 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-15 21:51 Linas Vepstas
2007-01-16  0:27 ` Jens Axboe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070116002733.GC4067@kernel.dk \
    --to=jens.axboe@oracle.com \
    --cc=Bino.Sebastian@Emulex.Com \
    --cc=James.Smart@Emulex.Com \
    --cc=Laurie.Barry@Emulex.Com \
    --cc=axboe@suse.de \
    --cc=chrisw@sous-sol.org \
    --cc=linas@austin.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=npiggin@suse.de \
    --cc=rlary@us.ibm.com \
    --cc=strosake@us.ibm.com \
    --cc=vaios.papadimitriou@Emulex.Com \
    --subject='Re: [PATCH] adjust use of unplug in elevator code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).