public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: SCSI development list <linux-scsi@vger.kernel.org>,
	Douglas Gilbert <dgilbert@interlog.com>
Subject: Re: uas breakage when using scsi_mod.blk_mq=Y
Date: Wed, 01 Oct 2014 17:53:10 +0200	[thread overview]
Message-ID: <542C2366.30700@redhat.com> (raw)
In-Reply-To: <20141001124543.GB14872@infradead.org>

Hi,

On 10/01/2014 02:45 PM, Christoph Hellwig wrote:
> On Wed, Oct 01, 2014 at 10:17:41AM +0200, Hans de Goede wrote:
>> The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
>> is the tag number 33. uas.c does the following in slave_configure:
>>
>> 	scsi_activate_tcq(sdev, devinfo->qdepth - 2);
>>
>> Where qdepth is 32, so 30 gets passed in. uas.c stranslates scsi tags
>> to uas stream ids, which means it adds 2 (stream ids start at 1 not 0,
>> and 1 is reserved for untagged commands).
>>
>> So the tag 33 above, means that the scsi subsys has called uas.c with
>> a tagged command with a tag of 31, which should not happen when using
>> scsi_activate_tcq(sdev, 30).
>>
>> So should the uas.c code do something different with blk-mq to tell
>> it to only use tags 0-29, or is this a blk-mq bug ?
> 
> This is a mismatch between the (undocumented) existing behavior and
> what blk-mq implements.  In the old code unless you use host-shared
> maps you would never see a tag number greater than the queue depth
> when using block layer tags.  With blk-mq we also use host-wide maps,
> so you can easily see tag numbers bigger than the queue depth.
> 
> So far it seems uas is the only driver with this expectation.
> Given how it maps tags to a non-scsi concept it might be better to
> just use a separate bitmaps for the streams inside uas than reusing
> the tags.

So let me see if I understand this correctly, blk-mq will never queue
more then qdepth commands, but it will use higher tag numbers ?

If that is the case fixing this should be easy, uas already has
an array to map stream-ids to scsi cmnds so that when it gets a status
urb, it can find the scsi cmnd which belongs to that status urb. We
can just search for a free slot in that array.

> Is this mapping an implementation detail of the Linux uas
> driver or part of the spec?

The mapping is a Linux uas implementation detail, the spec is
silent on how to map things.

Regards,

Hans

  reply	other threads:[~2014-10-01 15:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-01  8:17 uas breakage when using scsi_mod.blk_mq=Y Hans de Goede
2014-10-01 12:45 ` Christoph Hellwig
2014-10-01 15:53   ` Hans de Goede [this message]
2014-10-01 17:31     ` Christoph Hellwig

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=542C2366.30700@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=dgilbert@interlog.com \
    --cc=hch@infradead.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