Skip to content

Commit

Permalink
usb/uas: make sure data urb is gone if we receive status before that
Browse files Browse the repository at this point in the history 
Just run into the following:
- new disk arrived in the system
- udev couldn't wait to get its hands on to to run ata_id /dev/sda
- this sent the cdb 0xa1 to the device.
- my UAS-gadget recevied the cdb and had no idea what to do with it. It
  decided to send a status URB back with sense set to invalid opcode.
- the host side received it status and completed the scsi command.
- the host sent another scsi with 4kib data buffer
- Now I was confused why the data transfer is only 512 bytes instead of
  4kib since the host is always allocating the complete transfer in one
  go.
- Finally the system crashed while walking through the sg list.

This patch adds three new flags in order to distinguish between DATA
URB completed and outstanding. If we receive status before data, we
cancel data and let data complete the command.
This solves the problem for IN and OUT transfers but does not work for
BIDI.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
  • Loading branch information
Sebastian Andrzej Siewior authored and Sarah Sharp committed Feb 2, 2012
1 parent ee398b5 commit e4d8318
Showing 1 changed file with 75 additions and 15 deletions.
90 changes: 75 additions & 15 deletions drivers/usb/storage/uas.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ 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 @@ -111,6 +114,7 @@ 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 @@ -128,13 +132,15 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd)
}

cmnd->result = sense_iu->status;
cmnd->scsi_done(cmnd);
if (!(cmdinfo->state & DATA_COMPLETES_CMD))
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 @@ -152,7 +158,8 @@ static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
}

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

static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
Expand All @@ -177,6 +184,7 @@ 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 @@ -202,12 +210,32 @@ 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 @@ -236,27 +264,59 @@ static void uas_stat_cmplt(struct urb *urb)
dev_err(&urb->dev->dev, "failed submit status urb\n");
}

static void uas_data_cmplt(struct urb *urb)
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)
{
struct scsi_data_buffer *sdb = urb->context;
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;

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, u16 stream_id,
struct scsi_data_buffer *sdb,
enum dma_data_direction dir)
unsigned int pipe, struct scsi_cmnd *cmnd,
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;
usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt,
sdb);
if (devinfo->use_streams)
urb->stream_id = stream_id;
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;
urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0;
urb->sg = sdb->table.sgl;
out:
Expand Down Expand Up @@ -358,8 +418,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, cmdinfo->stream,
scsi_in(cmnd), DMA_FROM_DEVICE);
devinfo->data_in_pipe, cmnd,
DMA_FROM_DEVICE);
if (!cmdinfo->data_in_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
cmdinfo->state &= ~ALLOC_DATA_IN_URB;
Expand All @@ -376,8 +436,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, cmdinfo->stream,
scsi_out(cmnd), DMA_TO_DEVICE);
devinfo->data_out_pipe, 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 e4d8318

Please sign in to comment.