From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762571AbXKOGPB (ORCPT ); Thu, 15 Nov 2007 01:15:01 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1758753AbXKOGLE (ORCPT ); Thu, 15 Nov 2007 01:11:04 -0500 Received: from pentafluge.infradead.org ([213.146.154.40]:36583 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753073AbXKOGLA (ORCPT ); Thu, 15 Nov 2007 01:11:00 -0500 Date: Wed, 14 Nov 2007 22:09:58 -0800 From: Greg KH To: linux-kernel@vger.kernel.org, stable@kernel.org Cc: Justin Forbes , Zwane Mwaikambo , "Theodore Ts'o" , Randy Dunlap , Dave Jones , Chuck Wolber , Chris Wedgwood , Michael Krufky , Chuck Ebbert , Domenico Andreoli , torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Jens Axboe Subject: [patch 13/13] BLOCK: Fix bad sharing of tag busy list on queues with shared tag maps Message-ID: <20071115060958.GN7602@kroah.com> References: <20071115042610.731859958@mini.kroah.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="block-fix-bad-sharing-of-tag-busy-list-on-queues-with-shared-tag-maps.patch" In-Reply-To: <20071115060544.GA7602@kroah.com> User-Agent: Mutt/1.5.16 (2007-06-09) X-Bad-Reply: References and In-Reply-To but no 'Re:' in Subject. Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org -stable review patch. If anyone has any objections, please let us know. ------------------ From: Jens Axboe 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 Signed-off-by: Greg Kroah-Hartman --- 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; --