Skip to content

Commit

Permalink
Revert "usb/uas: make sure data urb is gone if we receive status befo…
Browse files Browse the repository at this point in the history 
…re that"

This reverts commit e4d8318.

This patch makes uas.c call usb_unlink_urb on data urbs.  The data urbs
get freed in the completion callback.  This is illegal according to the
usb_unlink_urb documentation.

This patch also makes the code expect the data completion callback
being called before the status completion callback.  This isn't
guaranteed to be the case, even though the actual data transfer should
be finished by the time the status is received.

Background:  The ehci irq handler for example only know that there are
finished transfers, it then has go check the QHs & TDs to see which
transfers did actually finish.  It has no way to figure in which order
the transfers did complete.  The xhci driver can call the callbacks in
completion order thanks to the event queue.  This does nicely explain
why the driver is solid on a (usb2) xhci port whereas it goes crazy on
ehci in my testing.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Gerd Hoffmann authored and Greg Kroah-Hartman committed Jun 25, 2012
1 parent 889e552 commit c621a81
Showing 1 changed file with 15 additions and 75 deletions.
90 changes: 15 additions & 75 deletions drivers/usb/storage/uas.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ enum {
SUBMIT_DATA_OUT_URB = (1 << 5),
ALLOC_CMD_URB = (1 << 6),
SUBMIT_CMD_URB = (1 << 7),
COMPLETED_DATA_IN = (1 << 8),
COMPLETED_DATA_OUT = (1 << 9),
DATA_COMPLETES_CMD = (1 << 10),
};

/* Overrides scsi_pointer */
Expand Down Expand Up @@ -114,7 +111,6 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu *sense_iu = urb->transfer_buffer;
struct scsi_device *sdev = cmnd->device;
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;

if (urb->actual_length > 16) {
unsigned len = be16_to_cpup(&sense_iu->len);
Expand All @@ -132,15 +128,13 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
}

cmnd->result = sense_iu->status;
if (!(cmdinfo->state & DATA_COMPLETES_CMD))
cmnd->scsi_done(cmnd);
cmnd->scsi_done(cmnd);
}

static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
{
struct sense_iu_old *sense_iu = urb->transfer_buffer;
struct scsi_device *sdev = cmnd->device;
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;

if (urb->actual_length > 8) {
unsigned len = be16_to_cpup(&sense_iu->len) - 2;
Expand All @@ -158,8 +152,7 @@ static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
}

cmnd->result = sense_iu->status;
if (!(cmdinfo->state & DATA_COMPLETES_CMD))
cmnd->scsi_done(cmnd);
cmnd->scsi_done(cmnd);
}

static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
Expand All @@ -184,7 +177,6 @@ static void uas_stat_cmplt(struct urb *urb)
struct Scsi_Host *shost = urb->context;
struct uas_dev_info *devinfo = (void *)shost->hostdata[0];
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
u16 tag;
int ret;

Expand All @@ -210,32 +202,12 @@ static void uas_stat_cmplt(struct urb *urb)
dev_err(&urb->dev->dev, "failed submit status urb\n");
return;
}
cmdinfo = (void *)&cmnd->SCp;

switch (iu->iu_id) {
case IU_ID_STATUS:
if (devinfo->cmnd == cmnd)
devinfo->cmnd = NULL;

if (!(cmdinfo->state & COMPLETED_DATA_IN) &&
cmdinfo->data_in_urb) {
if (devinfo->use_streams) {
cmdinfo->state |= DATA_COMPLETES_CMD;
usb_unlink_urb(cmdinfo->data_in_urb);
} else {
usb_free_urb(cmdinfo->data_in_urb);
}
}
if (!(cmdinfo->state & COMPLETED_DATA_OUT) &&
cmdinfo->data_out_urb) {
if (devinfo->use_streams) {
cmdinfo->state |= DATA_COMPLETES_CMD;
usb_unlink_urb(cmdinfo->data_in_urb);
} else {
usb_free_urb(cmdinfo->data_out_urb);
}
}

if (urb->actual_length < 16)
devinfo->uas_sense_old = 1;
if (devinfo->uas_sense_old)
Expand Down Expand Up @@ -264,59 +236,27 @@ static void uas_stat_cmplt(struct urb *urb)
dev_err(&urb->dev->dev, "failed submit status urb\n");
}

static void uas_data_out_cmplt(struct urb *urb)
{
struct scsi_cmnd *cmnd = urb->context;
struct scsi_data_buffer *sdb = scsi_out(cmnd);
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;

cmdinfo->state |= COMPLETED_DATA_OUT;

sdb->resid = sdb->length - urb->actual_length;
usb_free_urb(urb);

if (cmdinfo->state & DATA_COMPLETES_CMD)
cmnd->scsi_done(cmnd);
}

static void uas_data_in_cmplt(struct urb *urb)
static void uas_data_cmplt(struct urb *urb)
{
struct scsi_cmnd *cmnd = urb->context;
struct scsi_data_buffer *sdb = scsi_in(cmnd);
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;

cmdinfo->state |= COMPLETED_DATA_IN;

struct scsi_data_buffer *sdb = urb->context;
sdb->resid = sdb->length - urb->actual_length;
usb_free_urb(urb);

if (cmdinfo->state & DATA_COMPLETES_CMD)
cmnd->scsi_done(cmnd);
}

static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp,
unsigned int pipe, struct scsi_cmnd *cmnd,
enum dma_data_direction dir)
unsigned int pipe, u16 stream_id,
struct scsi_data_buffer *sdb,
enum dma_data_direction dir)
{
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
struct usb_device *udev = devinfo->udev;
struct urb *urb = usb_alloc_urb(0, gfp);
struct scsi_data_buffer *sdb;
usb_complete_t complete_fn;
u16 stream_id = cmdinfo->stream;

if (!urb)
goto out;
if (dir == DMA_FROM_DEVICE) {
sdb = scsi_in(cmnd);
complete_fn = uas_data_in_cmplt;
} else {
sdb = scsi_out(cmnd);
complete_fn = uas_data_out_cmplt;
}
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length,
complete_fn, cmnd);
urb->stream_id = stream_id;
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt,
sdb);
if (devinfo->use_streams)
urb->stream_id = stream_id;
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
urb->sg = sdb->table.sgl;
out:
Expand Down Expand Up @@ -418,8 +358,8 @@ 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, cmnd,
DMA_FROM_DEVICE);
devinfo->data_in_pipe, cmdinfo->stream,
scsi_in(cmnd), DMA_FROM_DEVICE);
if (!cmdinfo->data_in_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_IN_URB;
Expand All @@ -436,8 +376,8 @@ 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, cmnd,
DMA_TO_DEVICE);
devinfo->data_out_pipe, cmdinfo->stream,
scsi_out(cmnd), DMA_TO_DEVICE);
if (!cmdinfo->data_out_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_OUT_URB;
Expand Down

0 comments on commit c621a81

Please sign in to comment.