[PATCH v2 2/2] uas: Make uas work with blk-mq
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
Reported-by: Douglas Gilbert
Signed-off-by: Hans de Goede
Reviewed-by: Christoph Hellwig
--
Changes in v2:
-Remove ".disable_blk_mq = true" from uas_host_template
---
drivers/usb/storage/uas.c | 64 +--
1 file changed, 23 insertions(+), 41 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 89b2434..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;
- i
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
On Fri, Oct 03, 2014 at 12:08:57PM +0200, Hans de Goede wrote: > 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 > Reported-by: Douglas Gilbert > Signed-off-by: Hans de Goede > Reviewed-by: Christoph Hellwig This doesn't apply to my usb-next branch of usb.git, care to refresh it and resend? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
Hi, On 10/03/2014 11:47 PM, Greg Kroah-Hartman wrote: > On Fri, Oct 03, 2014 at 12:08:57PM +0200, Hans de Goede wrote: >> 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 >> Reported-by: Douglas Gilbert >> Signed-off-by: Hans de Goede >> Reviewed-by: Christoph Hellwig > > This doesn't apply to my usb-next branch of usb.git, care to refresh it > and resend? I did this v2 to resolve a conflict with a last minute fix for 3.17 which James Bottomley pushed out yesterday. So if this does not apply that likely is because that fix is not (yet) in next. See: https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/log/?h=fixes And specifically: https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?h=fixes&id=2c2d831c81ec75a7b0d8e28caa8e3d9c1fe546f9 I'm not sure how to proceed here, maybe the best thing is to cherry pick that fix into usb-next (should disappear on merge), and then apply this one on top ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
On Sat, 2014-10-04 at 10:53 +0200, Hans de Goede wrote: > Hi, > > On 10/03/2014 11:47 PM, Greg Kroah-Hartman wrote: > > On Fri, Oct 03, 2014 at 12:08:57PM +0200, Hans de Goede wrote: > >> 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 > >> Reported-by: Douglas Gilbert > >> Signed-off-by: Hans de Goede > >> Reviewed-by: Christoph Hellwig > > > > This doesn't apply to my usb-next branch of usb.git, care to refresh it > > and resend? > > I did this v2 to resolve a conflict with a last minute fix for 3.17 which > James Bottomley pushed out yesterday. So if this does not apply that likely > is because that fix is not (yet) in next. See: > > https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/log/?h=fixes > > And specifically: > > https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?h=fixes&id=2c2d831c81ec75a7b0d8e28caa8e3d9c1fe546f9 > > I'm not sure how to proceed here, maybe the best thing is to cherry pick > that fix into usb-next (should disappear on merge), and then apply > this one on top ? Depends: The fixes tree is based on -rc1, so you could just use that as the base for the topic branch uas goes in. That's the usual way. Or we could just do it via the SCSI tree, which will have the necessary base. Which would you prefer? James -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
Hi, On 10/04/2014 04:46 PM, James Bottomley wrote: > On Sat, 2014-10-04 at 10:53 +0200, Hans de Goede wrote: >> Hi, >> >> On 10/03/2014 11:47 PM, Greg Kroah-Hartman wrote: >>> On Fri, Oct 03, 2014 at 12:08:57PM +0200, Hans de Goede wrote: 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 Reported-by: Douglas Gilbert Signed-off-by: Hans de Goede Reviewed-by: Christoph Hellwig >>> >>> This doesn't apply to my usb-next branch of usb.git, care to refresh it >>> and resend? >> >> I did this v2 to resolve a conflict with a last minute fix for 3.17 which >> James Bottomley pushed out yesterday. So if this does not apply that likely >> is because that fix is not (yet) in next. See: >> >> https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/log/?h=fixes >> >> And specifically: >> >> https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?h=fixes&id=2c2d831c81ec75a7b0d8e28caa8e3d9c1fe546f9 >> >> I'm not sure how to proceed here, maybe the best thing is to cherry pick >> that fix into usb-next (should disappear on merge), and then apply >> this one on top ? > > Depends: The fixes tree is based on -rc1, so you could just use that as > the base for the topic branch uas goes in. That's the usual way. Or we > could just do it via the SCSI tree, which will have the necessary base. > > Which would you prefer? This patch sits on top of a lot of other uas fixes which are already in usb-next, so this needs to go in through usb-next. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
On Sun, Oct 05, 2014 at 11:06:00AM +0200, Hans de Goede wrote: > This patch sits on top of a lot of other uas fixes which are already > in usb-next, so this needs to go in through usb-next. What is the actual conflict? If it's just your revert of the disable_blk_mq flag you might want to skip that for this pull request and send it on after both scsi and usb trees make it to Linus. If there's more conflict around the host template you could cherry pick my patch for your tree. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
Hi, On 10/05/2014 11:10 AM, Christoph Hellwig wrote: > On Sun, Oct 05, 2014 at 11:06:00AM +0200, Hans de Goede wrote: >> This patch sits on top of a lot of other uas fixes which are already >> in usb-next, so this needs to go in through usb-next. > > What is the actual conflict? If it's just your revert of the > disable_blk_mq flag Yes, the conclict is just my revert of the disable_blk_mq flag > you might want to skip that for this pull request > and send it on after both scsi and usb trees make it to Linus. Since the patch with the conclict actually makes the flag unnecessary, to me this belongs in the same patch. > If there's more conflict around the host template you could cherry pick > my patch for your tree. Yes, Greg cherry picking the patch in question into his tree to resolv the conflict would be my preferred solution to this, but ultimately that is Greg's call. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] uas: Make uas work with blk-mq
On Sun, Oct 05, 2014 at 11:18:49AM +0200, Hans de Goede wrote: > Hi, > > On 10/05/2014 11:10 AM, Christoph Hellwig wrote: > > On Sun, Oct 05, 2014 at 11:06:00AM +0200, Hans de Goede wrote: > >> This patch sits on top of a lot of other uas fixes which are already > >> in usb-next, so this needs to go in through usb-next. > > > > What is the actual conflict? If it's just your revert of the > > disable_blk_mq flag > > Yes, the conclict is just my revert of the disable_blk_mq flag > > > you might want to skip that for this pull request > > and send it on after both scsi and usb trees make it to Linus. > > Since the patch with the conclict actually makes the flag unnecessary, > to me this belongs in the same patch. > > > If there's more conflict around the host template you could cherry pick > > my patch for your tree. > > Yes, Greg cherry picking the patch in question into his tree to resolv > the conflict would be my preferred solution to this, but ultimately > that is Greg's call. I'm not going to do that, just resend it to me after 3.18-rc1 is out, my tree is closed for the -rc1 merge window now anyway. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
