All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: Justin Forbes <jmforbes@linuxtx.org>,
	Zwane Mwaikambo <zwane@arm.linux.org.uk>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Dave Jones <davej@redhat.com>,
	Chuck Wolber <chuckw@quantumlinux.com>,
	Chris Wedgwood <reviews@ml.cw.f00f.org>,
	Michael Krufky <mkrufky@linuxtv.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	Domenico Andreoli <cavokz@gmail.com>,
	torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, Jens Axboe <jens.axboe@oracle.com>
Subject: [patch 13/13] BLOCK: Fix bad sharing of tag busy list on queues with shared tag maps
Date: Wed, 14 Nov 2007 22:09:58 -0800	[thread overview]
Message-ID: <20071115060958.GN7602@kroah.com> (raw)
In-Reply-To: <20071115060544.GA7602@kroah.com>

[-- Attachment #1: block-fix-bad-sharing-of-tag-busy-list-on-queues-with-shared-tag-maps.patch --]
[-- Type: text/plain, Size: 3261 bytes --]

-stable review patch.  If anyone has any objections, please let us know.

------------------
From: Jens Axboe <jens.axboe@oracle.com>

patch 6eca9004dfcb274a502438a591df5b197690afb1 in mainline.

For the locking to work, only the tag map and tag bit map may be shared
(incidentally, I was just explaining this to Nick yesterday, but I
apparently didn't review the code well enough myself). But we also share
the busy list!  The busy_list must be queue private, or we need a
block_queue_tag covering lock as well.

So we have to move the busy_list to the queue. This'll work fine, and
it'll actually also fix a problem with blk_queue_invalidate_tags() which
will invalidate tags across all shared queues. This is a bit confusing,
the low level driver should call it for each queue seperately since
otherwise you cannot kill tags on just a single queue for eg a hard
drive that stops responding. Since the function has no callers
currently, it's not an issue.

This is fixed with commit 6eca9004dfcb274a502438a591df5b197690afb1 in
Linus' tree.

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 block/ll_rw_blk.c      |    8 +++-----
 include/linux/blkdev.h |    2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -819,7 +819,6 @@ static int __blk_free_tags(struct blk_qu
 	retval = atomic_dec_and_test(&bqt->refcnt);
 	if (retval) {
 		BUG_ON(bqt->busy);
-		BUG_ON(!list_empty(&bqt->busy_list));
 
 		kfree(bqt->tag_index);
 		bqt->tag_index = NULL;
@@ -931,7 +930,6 @@ static struct blk_queue_tag *__blk_queue
 	if (init_tag_map(q, tags, depth))
 		goto fail;
 
-	INIT_LIST_HEAD(&tags->busy_list);
 	tags->busy = 0;
 	atomic_set(&tags->refcnt, 1);
 	return tags;
@@ -982,6 +980,7 @@ int blk_queue_init_tags(struct request_q
 	 */
 	q->queue_tags = tags;
 	q->queue_flags |= (1 << QUEUE_FLAG_QUEUED);
+	INIT_LIST_HEAD(&q->tag_busy_list);
 	return 0;
 fail:
 	kfree(tags);
@@ -1152,7 +1151,7 @@ int blk_queue_start_tag(struct request_q
 	rq->tag = tag;
 	bqt->tag_index[tag] = rq;
 	blkdev_dequeue_request(rq);
-	list_add(&rq->queuelist, &bqt->busy_list);
+	list_add(&rq->queuelist, &q->tag_busy_list);
 	bqt->busy++;
 	return 0;
 }
@@ -1173,11 +1172,10 @@ EXPORT_SYMBOL(blk_queue_start_tag);
  **/
 void blk_queue_invalidate_tags(struct request_queue *q)
 {
-	struct blk_queue_tag *bqt = q->queue_tags;
 	struct list_head *tmp, *n;
 	struct request *rq;
 
-	list_for_each_safe(tmp, n, &bqt->busy_list) {
+	list_for_each_safe(tmp, n, &q->tag_busy_list) {
 		rq = list_entry_rq(tmp);
 
 		if (rq->tag == -1) {
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -356,7 +356,6 @@ enum blk_queue_state {
 struct blk_queue_tag {
 	struct request **tag_index;	/* map of busy tags */
 	unsigned long *tag_map;		/* bit map of free/busy tags */
-	struct list_head busy_list;	/* fifo list of busy tags */
 	int busy;			/* current depth */
 	int max_depth;			/* what we will send to device */
 	int real_max_depth;		/* what the array can hold */
@@ -451,6 +450,7 @@ struct request_queue
 	unsigned int		dma_alignment;
 
 	struct blk_queue_tag	*queue_tags;
+	struct list_head	tag_busy_list;
 
 	unsigned int		nr_sorted;
 	unsigned int		in_flight;

-- 

      parent reply	other threads:[~2007-11-15  6:15 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20071115042610.731859958@mini.kroah.org>
2007-11-15  6:05 ` [patch 00/13] 2.6.23-stable review, core kernel changes Greg KH
2007-11-15  6:09   ` [patch 01/13] lockdep: fix mismatched lockdep_depth/curr_chain_hash Greg KH
2007-11-15  6:09   ` [patch 02/13] locks: fix possible infinite loop in posix deadlock detection Greg KH
2007-11-15  6:09   ` [patch 03/13] Remove broken ptrace() special-case code from file mapping Greg KH
2007-11-15  6:09   ` [patch 04/13] param_sysfs_builtin memchr argument fix Greg KH
2007-11-15 16:11     ` Chuck Ebbert
2007-11-15 17:58       ` Greg KH
2007-11-15 20:46         ` Chuck Ebbert
2007-11-15 21:20           ` Jan Kiszka
2007-11-15 23:58             ` Greg KH
2007-11-15  6:09   ` [patch 05/13] HOWTO: update ja_JP/HOWTO with latest changes Greg KH
2007-11-15  6:09   ` [patch 06/13] SLUB: Fix memory leak by not reusing cpu_slab Greg KH
2007-11-15  6:09   ` [patch 07/13] writeback: dont propagate AOP_WRITEPAGE_ACTIVATE Greg KH
2007-11-15  6:09   ` [patch 08/13] splice: fix double kunmap() in vmsplice copy path Greg KH
2007-11-15  6:09   ` [patch 09/13] fix the softlockup watchdog to actually work Greg KH
2007-11-15  6:09   ` [patch 10/13] sched: keep utime/stime monotonic Greg KH
2007-11-15  6:09   ` [patch 11/13] Fix compat futex hangs Greg KH
2007-11-15  6:09   ` [patch 12/13] fix tmpfs BUG and AOP_WRITEPAGE_ACTIVATE Greg KH
2007-11-15  6:09   ` Greg KH [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=20071115060958.GN7602@kroah.com \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=cavokz@gmail.com \
    --cc=cebbert@redhat.com \
    --cc=chuckw@quantumlinux.com \
    --cc=davej@redhat.com \
    --cc=jens.axboe@oracle.com \
    --cc=jmforbes@linuxtx.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkrufky@linuxtv.org \
    --cc=rdunlap@xenotime.net \
    --cc=reviews@ml.cw.f00f.org \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=zwane@arm.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.