public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: jens.axboe@oracle.com, jesper.juhl@gmail.com,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: scsi_scan: WARNING: at include/linux/blkdev.h:431 blk_queue_init_tags+0x107/0x120()
Date: Mon, 12 May 2008 10:59:28 -0500	[thread overview]
Message-ID: <1210607968.3078.15.camel@localhost.localdomain> (raw)
In-Reply-To: <20080508125532W.fujita.tomonori@lab.ntt.co.jp>

On Thu, 2008-05-08 at 12:55 +0900, FUJITA Tomonori wrote: 
> On Wed, 07 May 2008 17:40:26 -0500
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> > > The problem is that the commit 75ad23b expects that we hold the queue
> > > lock for __blk_queue_free_tags, blk_queue_free_tags and
> > > blk_queue_init_tags but we haven't.
> > > 
> > > The simple fix is using queue_flag_set/clear_unlocked for them, then
> > > it should work as before. However, it would be better to hold the
> > > queue lock for blk_queue_free_tags and blk_queue_init_tags (we can
> > > hold the queue lock in scsi_activate_tcq and scsi_deactivate_tcq).
> > 
> > So is this the fix that everyone agrees on?  And if so, whose tree is it
> > going through (I tend to think block, since the original breakage came
> > from the block tree).
> 
> The patch is fine from the perspective of the SCSI mid-layer. But it
> would be not from the perspective of the block layer. For example,
> blk_queue_init_tags is expected to be called with holding the queue
> lock (since it could call blk_queue_resize_tags internally) though
> only the SCSI mid-layer uses those APIs for now.

There are two cases for blk_queue_init_tags; one is for a shared tag map
and the other is for individual tag maps.  If you look at the code and
its use, blk_queue_resize_tags is *only* called for the individual tag
map case (or for the first caller in a shared tag map case).

The blk_queue_resize_tags() also returns -EBUSY on shared tag maps,
except the first one; which really exposes a bug in SCSI: we want to set
the shared tag map to some host specific depth and then carve off pieces
of it in the devices, so we don't want to adjust the queue down to
whatever the first seen device wants, and we don't want all other
devices to fail to adjust the scsi_queue depth the mid-layer uses.

This should fix up scsi.

James

---
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 110e776..cc0af0f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -893,9 +893,11 @@ void scsi_adjust_queue_depth(struct scsi_device *sdev, int tagged, int tags)
 
 	spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
 
-	/* Check to see if the queue is managed by the block layer.
-	 * If it is, and we fail to adjust the depth, exit. */
+	/* Check to see if the queue is managed by the block layer,
+	 * and that we don't have a shared tag map.  If so, and we
+	 * fail to adjust the depth, exit. */
 	if (blk_queue_tagged(sdev->request_queue) &&
+	    !sdev->host->bqt &&
 	    blk_queue_resize_tags(sdev->request_queue, tags) != 0)
 		goto out;
 




  reply	other threads:[~2008-05-12 17:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-01 22:43 scsi_scan: WARNING: at include/linux/blkdev.h:431 blk_queue_init_tags+0x107/0x120() Jesper Juhl
2008-05-02  2:17 ` FUJITA Tomonori
2008-05-02 10:13   ` Jesper Juhl
2008-05-07 22:40   ` James Bottomley
2008-05-08  3:55     ` FUJITA Tomonori
2008-05-12 15:59       ` James Bottomley [this message]
2008-05-15 20:01         ` Jesper Juhl

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=1210607968.3078.15.camel@localhost.localdomain \
    --to=james.bottomley@hansenpartnership.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jens.axboe@oracle.com \
    --cc=jesper.juhl@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@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