linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: Jan Beulich <JBeulich@novell.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org
Subject: [PATCH v2.6.38-rc5 2/2] block: blk-flush shouldn't call directly into q->request_fn() __blk_run_queue()
Date: Thu, 17 Feb 2011 12:16:19 +0100	[thread overview]
Message-ID: <20110217111619.GR19830@htj.dyndns.org> (raw)
In-Reply-To: <20110217111511.GQ19830@htj.dyndns.org>

blk-flush decomposes a flush into sequence of multiple requests.  On
completion of a request, the next one is queued; however, block layer
must not implicitly call into q->request_fn() directly from completion
path.  This makes the queue behave unexpectedly when seen from the
drivers and violates the assumption that q->request_fn() is called
with process context + queue_lock.

This patch makes blk-flush the following two changes to make sure
q->request_fn() is not called directly from request completion path.

- blk_flush_complete_seq_end_io() now asks __blk_run_queue() to always
  use kblockd instead of calling directly into q->request_fn().

- queue_next_fseq() uses ELEVATOR_INSERT_REQUEUE instead of
  ELEVATOR_INSERT_FRONT so that elv_insert() doesn't try to unplug the
  request queue directly.

Reported by Jan in the following threads.

 http://thread.gmane.org/gmane.linux.ide/48778
 http://thread.gmane.org/gmane.linux.ide/48786

stable: applicable to v2.6.37.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Jan Beulich <JBeulich@novell.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: stable@kernel.org
---
Jens, this is applicable to v2.6.37 and 38.  The new implementation
for 39 would need similar fix but I couldn't find where the tree is.
Which branch is it?

Thanks.

 block/blk-flush.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: work/block/blk-flush.c
===================================================================
--- work.orig/block/blk-flush.c
+++ work/block/blk-flush.c
@@ -66,10 +66,12 @@ static void blk_flush_complete_seq_end_i
 
 	/*
 	 * Moving a request silently to empty queue_head may stall the
-	 * queue.  Kick the queue in those cases.
+	 * queue.  Kick the queue in those cases.  This function is called
+	 * from request completion path and calling directly into
+	 * request_fn may confuse the driver.  Always use kblockd.
 	 */
 	if (was_empty && next_rq)
-		__blk_run_queue(q, false);
+		__blk_run_queue(q, true);
 }
 
 static void pre_flush_end_io(struct request *rq, int error)
@@ -130,7 +132,7 @@ static struct request *queue_next_fseq(s
 		BUG();
 	}
 
-	elv_insert(q, rq, ELEVATOR_INSERT_FRONT);
+	elv_insert(q, rq, ELEVATOR_INSERT_REQUEUE);
 	return rq;
 }
 

  reply	other threads:[~2011-02-17 11:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-17 11:15 [PATCH v2.6.38-rc5 1/2] block: add @force_kblockd to __blk_run_queue() Tejun Heo
2011-02-17 11:16 ` Tejun Heo [this message]
2011-02-17 15:01   ` [PATCH v2.6.38-rc5 2/2] block: blk-flush shouldn't call directly into q->request_fn() __blk_run_queue() Mike Snitzer
2011-02-18  9:49     ` Tejun Heo
2011-03-02 13:46       ` Jens Axboe
2011-03-04 18:25         ` Tejun Heo
2011-03-07  8:41           ` Jens Axboe
2011-03-07 19:33           ` Jeff Moyer
2011-03-07 19:36             ` Jens Axboe
2011-03-07 19:39               ` Jeff Moyer
2011-03-07 19:41                 ` Jens Axboe
2011-03-07 19:50                   ` Jeff Moyer
2011-03-08  7:54                     ` Tejun Heo
2011-03-02 12:53 ` [PATCH v2.6.38-rc5 1/2] block: add @force_kblockd to __blk_run_queue() Jan Beulich

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=20110217111619.GR19830@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=JBeulich@novell.com \
    --cc=axboe@kernel.dk \
    --cc=davem@davemloft.net \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).