Discussion:
[PATCH 1/2] uas: Reduce number of function arguments for uas_alloc_foo functions
Hans de Goede
2014-10-02 10:04:39 UTC
Permalink
The stream_id and pipe are already present in uas_cmd_info resp uas_dev_info,
so there is no need to pass a copy along.

Signed-off-by: Hans de Goede <***@redhat.com>
---
drivers/usb/storage/uas.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index b27fe21..d1dbe88 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -412,20 +412,22 @@ static void uas_cmd_cmplt(struct urb *urb)
}

static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
- unsigned int pipe, u16 stream_id,
struct scsi_cmnd *cmnd,
enum dma_data_direction dir)
{
struct usb_device *udev = devinfo->udev;
+ struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct urb *urb = usb_alloc_urb(0, gfp);
struct scsi_data_buffer *sdb = (dir == DMA_FROM_DEVICE)
? scsi_in(cmnd) : scsi_out(cmnd);
+ unsigned int pipe = (dir == DMA_FROM_DEVICE)
+ ? devinfo->data_in_pipe : devinfo->data_out_pipe;

if (!urb)
goto out;
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length,
uas_data_cmplt, cmnd);
- urb->stream_id = stream_id;
+ urb->stream_id = cmdinfo->stream;
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
urb->sg = sdb->table.sgl;
out:
@@ -433,9 +435,10 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
}

static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp,
- struct Scsi_Host *shost, u16 stream_id)
+ struct scsi_cmnd *cmnd)
{
struct usb_device *udev = devinfo->udev;
+ struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct urb *urb = usb_alloc_urb(0, gfp);
struct sense_iu *iu;

@@ -447,8 +450,8 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp,
goto free;

usb_fill_bulk_urb(urb, udev, devinfo->status_pipe, iu, sizeof(*iu),
- uas_stat_cmplt, shost);
- urb->stream_id = stream_id;
+ uas_stat_cmplt, cmnd->device->host);
+ urb->stream_id = cmdinfo->stream;
urb->transfer_flags |= URB_FREE_BUFFER;
out:
return urb;
@@ -500,15 +503,13 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
* daft to me.
*/

-static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd,
- gfp_t gfp, unsigned int stream)
+static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp)
{
- struct Scsi_Host *shost = cmnd->device->host;
- struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata;
+ struct uas_dev_info *devinfo = cmnd->device->hostdata;
struct urb *urb;
int err;

- urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream);
+ urb = uas_alloc_sense_urb(devinfo, gfp, cmnd);
if (!urb)
return NULL;
usb_anchor_urb(urb, &devinfo->sense_urbs);
@@ -531,7 +532,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,

lockdep_assert_held(&devinfo->lock);
if (cmdinfo->state & SUBMIT_STATUS_URB) {
- urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream);
+ urb = uas_submit_sense_urb(cmnd, gfp);
if (!urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~SUBMIT_STATUS_URB;
@@ -539,8 +540,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,

if (cmdinfo->state & ALLOC_DATA_IN_URB) {
cmdinfo->data_in_urb = uas_alloc_data_urb(devinfo, gfp,
- devinfo->data_in_pipe, cmdinfo->stream,
- cmnd, DMA_FROM_DEVICE);
+ cmnd, DMA_FROM_DEVICE);
if (!cmdinfo->data_in_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_IN_URB;
@@ -560,8 +560,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,

if (cmdinfo->state & ALLOC_DATA_OUT_URB) {
cmdinfo->data_out_urb = uas_alloc_data_urb(devinfo, gfp,
- devinfo->data_out_pipe, cmdinfo->stream,
- cmnd, DMA_TO_DEVICE);
+ cmnd, DMA_TO_DEVICE);
if (!cmdinfo->data_out_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_OUT_URB;
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-02 10:04:40 UTC
Permalink
With uas over usb-3 the tags inside the uas iu-s must match the usb-3 stream
ids, and those go from 1 - qdepth.

Before blk-mq calling scsi_activate_tcq(sdev, qdepth) guaranteed that we would
only get cmnd->request->tag from 0 - (qdepth - 1), and we used those as
uas-tags / stream-ids.

With blk-mq however we are guaranteed to never get more then qdepth commands
queued at the same time, but the cmnd->request->tag values may be much larger,
which breaks uas.

This commit fixes this by generating uas tags in the 1 - qdepth range ourselves
instead of using cmnd->request->tag.

While touching all involved code anyways also rename the uas_cmd_info stream
field to uas_tag, because when using uas over usb-2 streams are not used.

Cc: Christoph Hellwig <hch-***@public.gmane.org>
Reported-by: Douglas Gilbert <dgilbert-qazKcTl6WRFWk0Htik3J/***@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
drivers/usb/storage/uas.c | 57 +++++++++++++++++++----------------------------
1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d1dbe88..004ebc1 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -66,7 +66,7 @@ enum {
/* Overrides scsi_pointer */
struct uas_cmd_info {
unsigned int state;
- unsigned int stream;
+ unsigned int uas_tag;
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
@@ -173,30 +173,15 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
cmnd->result = sense_iu->status;
}

-/*
- * scsi-tags go from 0 - (nr_tags - 1), uas tags need to match stream-ids,
- * which go from 1 - nr_streams. And we use 1 for untagged commands.
- */
-static int uas_get_tag(struct scsi_cmnd *cmnd)
-{
- int tag;
-
- if (blk_rq_tagged(cmnd->request))
- tag = cmnd->request->tag + 2;
- else
- tag = 1;
-
- return tag;
-}
-
static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix,
int status)
{
struct uas_cmd_info *ci = (void *)&cmnd->SCp;
+ struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;

scmd_printk(KERN_INFO, cmnd,
- "%s %d tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ",
- prefix, status, uas_get_tag(cmnd),
+ "%s %d uas-tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ",
+ prefix, status, cmdinfo->uas_tag,
(ci->state & SUBMIT_STATUS_URB) ? " s-st" : "",
(ci->state & ALLOC_DATA_IN_URB) ? " a-in" : "",
(ci->state & SUBMIT_DATA_IN_URB) ? " s-in" : "",
@@ -242,7 +227,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
DATA_OUT_URB_INFLIGHT |
COMMAND_ABORTED))
return -EBUSY;
- devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL;
+ devinfo->cmnd[cmdinfo->uas_tag - 1] = NULL;
uas_free_unsubmitted_urbs(cmnd);
cmnd->scsi_done(cmnd);
return 0;
@@ -289,7 +274,7 @@ static void uas_stat_cmplt(struct urb *urb)
idx = be16_to_cpup(&iu->tag) - 1;
if (idx >= MAX_CMNDS || !devinfo->cmnd[idx]) {
dev_err(&urb->dev->dev,
- "stat urb: no pending cmd for tag %d\n", idx + 1);
+ "stat urb: no pending cmd for uas-tag %d\n", idx + 1);
goto out;
}

@@ -427,7 +412,8 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
goto out;
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length,
uas_data_cmplt, cmnd);
- urb->stream_id = cmdinfo->stream;
+ if (devinfo->use_streams)
+ urb->stream_id = cmdinfo->uas_tag;
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
urb->sg = sdb->table.sgl;
out:
@@ -451,7 +437,8 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp,

usb_fill_bulk_urb(urb, udev, devinfo->status_pipe, iu, sizeof(*iu),
uas_stat_cmplt, cmnd->device->host);
- urb->stream_id = cmdinfo->stream;
+ if (devinfo->use_streams)
+ urb->stream_id = cmdinfo->uas_tag;
urb->transfer_flags |= URB_FREE_BUFFER;
out:
return urb;
@@ -465,6 +452,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
{
struct usb_device *udev = devinfo->udev;
struct scsi_device *sdev = cmnd->device;
+ struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct urb *urb = usb_alloc_urb(0, gfp);
struct command_iu *iu;
int len;
@@ -481,7 +469,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
goto free;

iu->iu_id = IU_ID_COMMAND;
- iu->tag = cpu_to_be16(uas_get_tag(cmnd));
+ iu->tag = cpu_to_be16(cmdinfo->uas_tag);
iu->prio_attr = UAS_SIMPLE_TAG;
iu->len = len;
int_to_scsilun(sdev->lun, &iu->lun);
@@ -608,8 +596,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo = sdev->hostdata;
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
unsigned long flags;
- unsigned int stream;
- int err;
+ int idx, err;

BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer));

@@ -635,8 +622,12 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
return 0;
}

- stream = uas_get_tag(cmnd);
- if (devinfo->cmnd[stream - 1]) {
+ /* Find a free uas-tag */
+ for (idx = 0; idx < devinfo->qdepth; idx++) {
+ if (!devinfo->cmnd[idx])
+ break;
+ }
+ if (idx == devinfo->qdepth) {
spin_unlock_irqrestore(&devinfo->lock, flags);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
@@ -644,7 +635,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmnd->scsi_done = done;

memset(cmdinfo, 0, sizeof(*cmdinfo));
- cmdinfo->stream = stream;
+ cmdinfo->uas_tag = idx + 1; /* uas-tag == usb-stream-id, so 1 based */
cmdinfo->state = SUBMIT_STATUS_URB | ALLOC_CMD_URB | SUBMIT_CMD_URB;

switch (cmnd->sc_data_direction) {
@@ -659,10 +650,8 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
break;
}

- if (!devinfo->use_streams) {
+ if (!devinfo->use_streams)
cmdinfo->state &= ~(SUBMIT_DATA_IN_URB | SUBMIT_DATA_OUT_URB);
- cmdinfo->stream = 0;
- }

err = uas_submit_urbs(cmnd, devinfo, GFP_ATOMIC);
if (err) {
@@ -674,7 +663,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
uas_add_work(cmdinfo);
}

- devinfo->cmnd[stream - 1] = cmnd;
+ devinfo->cmnd[idx] = cmnd;
spin_unlock_irqrestore(&devinfo->lock, flags);
return 0;
}
@@ -702,7 +691,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
cmdinfo->state |= COMMAND_ABORTED;

/* Drop all refs to this cmnd, kill data urbs to break their ref */
- devinfo->cmnd[uas_get_tag(cmnd) - 1] = NULL;
+ devinfo->cmnd[cmdinfo->uas_tag - 1] = NULL;
if (cmdinfo->state & DATA_IN_URB_INFLIGHT)
data_in_urb = usb_get_urb(cmdinfo->data_in_urb);
if (cmdinfo->state & DATA_OUT_URB_INFLIGHT)
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-10-02 11:26:27 UTC
Permalink
Looks fine to me (and actually removes code..)

Reviewed-by: Christoph Hellwig <***@lst.de>

It's fairly late in 3.17 to get something like this in, but would you
consider Ccing it to stable so we can get it into the first 3.17 stable
release?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-02 12:35:10 UTC
Permalink
Hi,
Post by Christoph Hellwig
Looks fine to me (and actually removes code..)
It's fairly late in 3.17 to get something like this in, but would you
consider Ccing it to stable so we can get it into the first 3.17 stable
release?
This patch sits on top of a whole bunch of uas cleanups / fixes which
are going into 3.18, and which are to big of a change to backport.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-10-02 13:06:55 UTC
Permalink
Post by Hans de Goede
This patch sits on top of a whole bunch of uas cleanups / fixes which
are going into 3.18, and which are to big of a change to backport.
Can add a patch to set disable_blk_mq for uas on 3.17 then?

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-02 13:08:40 UTC
Permalink
Hi,
Post by Christoph Hellwig
Post by Hans de Goede
This patch sits on top of a whole bunch of uas cleanups / fixes which
are going into 3.18, and which are to big of a change to backport.
Can add a patch to set disable_blk_mq for uas on 3.17 then?
Yes, although that likely would need to go through stable, without
going into 3.18 .

Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-10-02 13:12:59 UTC
Permalink
Post by Hans de Goede
Yes, although that likely would need to go through stable, without
going into 3.18 .
Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?
This is a trivial one liner and 3.17 isn't released yet, so can you just
send it to Linus now?

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-02 13:18:14 UTC
Permalink
Hi,
Post by Christoph Hellwig
Post by Hans de Goede
Yes, although that likely would need to go through stable, without
going into 3.18 .
Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?
This is a trivial one liner and 3.17 isn't released yet, so can you just
send it to Linus now?
I don't think it is that urgent, given that block-mq is disabled by default
in 3.17 AFAIK. But feel free to send such a patch directly to Linus with my:

Acked-by: Hans de Goede <***@redhat.com>

Added, if that gets in, I guess I then need to do a v2 of the patch
from $subject, reverting it.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
James Bottomley
2014-10-02 13:25:50 UTC
Permalink
Post by Hans de Goede
Hi,
Post by Christoph Hellwig
Post by Hans de Goede
Yes, although that likely would need to go through stable, without
going into 3.18 .
Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?
This is a trivial one liner and 3.17 isn't released yet, so can you just
send it to Linus now?
I don't think it is that urgent, given that block-mq is disabled by default
Added, if that gets in, I guess I then need to do a v2 of the patch
from $subject, reverting it.
OK, so 3.17 should be on sunday giving three days or so to sort this
out. If you're happy with blk-mq being disabled so the bug never
triggers unless the user activates it, doing nothing is my preferred
outcome ... but that also means no need to backport to stable.

If you want a it fixed in 3.17, then we need a patch from you now.
Ultimately you have to judge the necessity for us, since it's your
driver.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-02 13:29:15 UTC
Permalink
Hi,
Post by James Bottomley
Post by Hans de Goede
Hi,
Post by Christoph Hellwig
Post by Hans de Goede
Yes, although that likely would need to go through stable, without
going into 3.18 .
Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?
This is a trivial one liner and 3.17 isn't released yet, so can you just
send it to Linus now?
I don't think it is that urgent, given that block-mq is disabled by default
Added, if that gets in, I guess I then need to do a v2 of the patch
from $subject, reverting it.
OK, so 3.17 should be on sunday giving three days or so to sort this
out. If you're happy with blk-mq being disabled so the bug never
triggers unless the user activates it, doing nothing is my preferred
outcome ... but that also means no need to backport to stable.
If you want a it fixed in 3.17, then we need a patch from you now.
Ultimately you have to judge the necessity for us, since it's your
driver.
Fair enough. Given that this only triggers if the user enables an experimental
option, and then mixes that with uas usage, my preferred way to deal with this
is also doing nothing.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-10-03 08:47:09 UTC
Permalink
Post by James Bottomley
OK, so 3.17 should be on sunday giving three days or so to sort this
out. If you're happy with blk-mq being disabled so the bug never
triggers unless the user activates it, doing nothing is my preferred
outcome ... but that also means no need to backport to stable.
If you want a it fixed in 3.17, then we need a patch from you now.
Ultimately you have to judge the necessity for us, since it's your
driver.
scsi-mq might be new and not default, but I wouldn't really consider it
"experimental" in the sense that people should expect crashes if they
attach a usb device.

Please consider applying the patch below for 3.17, as there is a cxgb
patch pending anyway:

---
From 8a42ea1369701960a821b7fa9a3da5c68e7e4366 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <***@lst.de>
Date: Fri, 3 Oct 2014 10:45:10 +0200
Subject: uas: disable use of blk-mq I/O path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The uas driver uses the block layer tag for USB3 stream IDs. With
blk-mq we can get larger tag numbers that the queue depth, which breaks
this assumption. A fix is under way for 3.18, but sits on top of
large changes so can't easily be backported. Set the disable_blk_mq
path so that a uas device can't easily crash the system when using
blk-mq for SCSI.

Signed-off-by: Christoph Hellwig <***@lst.de>
Acked-by: Hans de Goede <***@redhat.com>
---
drivers/usb/storage/uas.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..9bfa725 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -970,6 +970,13 @@ static struct scsi_host_template uas_host_template = {
.cmd_per_lun = 1, /* until we override it */
.skip_settle_delay = 1,
.ordered_tag = 1,
+
+ /*
+ * The uas drivers expects tags not to be bigger than the maximum
+ * per-device queue depth, which is not true with the blk-mq tag
+ * allocator.
+ */
+ .disable_blk_mq = true,
};

#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans de Goede
2014-10-03 09:25:00 UTC
Permalink
Hi,
Post by Christoph Hellwig
Post by James Bottomley
OK, so 3.17 should be on sunday giving three days or so to sort this
out. If you're happy with blk-mq being disabled so the bug never
triggers unless the user activates it, doing nothing is my preferred
outcome ... but that also means no need to backport to stable.
If you want a it fixed in 3.17, then we need a patch from you now.
Ultimately you have to judge the necessity for us, since it's your
driver.
scsi-mq might be new and not default, but I wouldn't really consider it
"experimental" in the sense that people should expect crashes if they
attach a usb device.
Please consider applying the patch below for 3.17, as there is a cxgb
For the record, and since I said before that I'm fine with leaving this as
is. As Christoph believes it is important to get this fixed for 3.17, lets
try to get this fix in.

So my Acked-by which is included below still stands.

Regards,

Hans
Post by Christoph Hellwig
---
From 8a42ea1369701960a821b7fa9a3da5c68e7e4366 Mon Sep 17 00:00:00 2001
Date: Fri, 3 Oct 2014 10:45:10 +0200
Subject: uas: disable use of blk-mq I/O path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The uas driver uses the block layer tag for USB3 stream IDs. With
blk-mq we can get larger tag numbers that the queue depth, which breaks
this assumption. A fix is under way for 3.18, but sits on top of
large changes so can't easily be backported. Set the disable_blk_mq
path so that a uas device can't easily crash the system when using
blk-mq for SCSI.
---
drivers/usb/storage/uas.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..9bfa725 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -970,6 +970,13 @@ static struct scsi_host_template uas_host_template = {
.cmd_per_lun = 1, /* until we override it */
.skip_settle_delay = 1,
.ordered_tag = 1,
+
+ /*
+ * The uas drivers expects tags not to be bigger than the maximum
+ * per-device queue depth, which is not true with the blk-mq tag
+ * allocator.
+ */
+ .disable_blk_mq = true,
};
#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman
2014-10-02 13:27:59 UTC
Permalink
Post by Christoph Hellwig
Post by Hans de Goede
Yes, although that likely would need to go through stable, without
going into 3.18 .
Greg, is this ok with you and how do I get a patch in stable which
is not in upstream (because upstream will get a better fix) ?
This is a trivial one liner and 3.17 isn't released yet, so can you just
send it to Linus now?
Let's just wait for these patches to get into 3.18-rc1, and then, Hans,
you can send me a patch just for 3.17-stable with a note saying why it
is different from what is in Linus's tree.

Or send me that patch now and I can queue it up at the proper time.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-10-02 11:24:13 UTC
Permalink
Post by Hans de Goede
The stream_id and pipe are already present in uas_cmd_info resp uas_dev_info,
so there is no need to pass a copy along.
Looks good,

Reviewed-by: Christoph Hellwig <***@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...