From 2c51ae70ede5a90d8ccb67d965c1b4e20fc4e110 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Sat, 19 Apr 2008 14:32:18 -0700 Subject: ub: Fix timeouts The wodim says: "close track/session scsi sendcmd: cmd timeout after 5.000 (480) s" This happened because we ignored the supplied timeout and used 5s. It's not completely correct to apply a timeout meant for the complete command to any single URB, but we don't have many URBs per command, so this is simple and works. Signed-off-by: Pete Zaitcev Signed-off-by: Greg Kroah-Hartman --- drivers/block/ub.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ub.c b/drivers/block/ub.c index e322cce8c12..b87ad77e5bb 100644 --- a/drivers/block/ub.c +++ b/drivers/block/ub.c @@ -205,6 +205,7 @@ struct ub_scsi_cmd { unsigned char key, asc, ascq; /* May be valid if error==-EIO */ int stat_count; /* Retries getting status. */ + unsigned int timeo; /* jiffies until rq->timeout changes */ unsigned int len; /* Requested length */ unsigned int current_sg; @@ -764,6 +765,12 @@ static void ub_cmd_build_packet(struct ub_dev *sc, struct ub_lun *lun, cmd->cdb_len = rq->cmd_len; cmd->len = rq->data_len; + + /* + * To reapply this to every URB is not as incorrect as it looks. + * In return, we avoid any complicated tracking calculations. + */ + cmd->timeo = rq->timeout; } static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd) @@ -1336,7 +1343,10 @@ static void ub_data_start(struct ub_dev *sc, struct ub_scsi_cmd *cmd) return; } - sc->work_timer.expires = jiffies + UB_DATA_TIMEOUT; + if (cmd->timeo) + sc->work_timer.expires = jiffies + cmd->timeo; + else + sc->work_timer.expires = jiffies + UB_DATA_TIMEOUT; add_timer(&sc->work_timer); cmd->state = UB_CMDST_DATA; @@ -1376,7 +1386,10 @@ static int __ub_state_stat(struct ub_dev *sc, struct ub_scsi_cmd *cmd) return -1; } - sc->work_timer.expires = jiffies + UB_STAT_TIMEOUT; + if (cmd->timeo) + sc->work_timer.expires = jiffies + cmd->timeo; + else + sc->work_timer.expires = jiffies + UB_STAT_TIMEOUT; add_timer(&sc->work_timer); return 0; } -- cgit v1.2.3 From 82fe26ba7a21d9bcc77e6142c941683eede32940 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Sat, 19 Apr 2008 14:35:30 -0700 Subject: ub: Tune retries Make ub to fail faster in hopeless cases. Signed-off-by: Pete Zaitcev Signed-off-by: Greg Kroah-Hartman --- drivers/block/ub.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ub.c b/drivers/block/ub.c index b87ad77e5bb..5c6a6e89d2f 100644 --- a/drivers/block/ub.c +++ b/drivers/block/ub.c @@ -792,10 +792,6 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd) scsi_status = 0; } else { if (cmd->act_len != cmd->len) { - if ((cmd->key == MEDIUM_ERROR || - cmd->key == UNIT_ATTENTION) && - ub_rw_cmd_retry(sc, lun, urq, cmd) == 0) - return; scsi_status = SAM_STAT_CHECK_CONDITION; } else { scsi_status = 0; @@ -811,7 +807,10 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd) else scsi_status = DID_ERROR << 16; } else { - if (cmd->error == -EIO) { + if (cmd->error == -EIO && + (cmd->key == 0 || + cmd->key == MEDIUM_ERROR || + cmd->key == UNIT_ATTENTION)) { if (ub_rw_cmd_retry(sc, lun, urq, cmd) == 0) return; } -- cgit v1.2.3 From 0da13c8c3dfb1ab6c56f2a70fadfddd57e0d7c42 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Sat, 19 Apr 2008 14:42:49 -0700 Subject: ub: Ignore bad residue I hoped to continue to ignore this problem or use libusual, but these days it's simpler to work around than to deal with it. Let's attempt to use bad residue devices and hope that upper level integrity checks catch any problems (e.g. please use sha1sum on your backups). Signed-off-by: Pete Zaitcev Signed-off-by: Greg Kroah-Hartman --- drivers/block/ub.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ub.c b/drivers/block/ub.c index 5c6a6e89d2f..e2c3ebd8db2 100644 --- a/drivers/block/ub.c +++ b/drivers/block/ub.c @@ -319,6 +319,7 @@ struct ub_dev { int openc; /* protected by ub_lock! */ /* kref is too implicit for our taste */ int reset; /* Reset is running */ + int bad_resid; unsigned int tagcnt; char name[12]; struct usb_device *dev; @@ -1265,14 +1266,19 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd) return; } - len = le32_to_cpu(bcs->Residue); - if (len != cmd->len - cmd->act_len) { - /* - * It is all right to transfer less, the caller has - * to check. But it's not all right if the device - * counts disagree with our counts. - */ - goto Bad_End; + if (!sc->bad_resid) { + len = le32_to_cpu(bcs->Residue); + if (len != cmd->len - cmd->act_len) { + /* + * Only start ignoring if this cmd ended well. + */ + if (cmd->len == cmd->act_len) { + printk(KERN_NOTICE "%s: " + "bad residual %d of %d, ignoring\n", + sc->name, len, cmd->len); + sc->bad_resid = 1; + } + } } switch (bcs->Status) { -- cgit v1.2.3 From 9029b174ba22918d0a0aa3b71859854bd50c39cc Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Sat, 19 Apr 2008 14:45:24 -0700 Subject: ub: Cosmetics Fix a few comments and printk statements. Signed-off-by: Pete Zaitcev Signed-off-by: Greg Kroah-Hartman --- drivers/block/ub.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ub.c b/drivers/block/ub.c index e2c3ebd8db2..3a281ef11ff 100644 --- a/drivers/block/ub.c +++ b/drivers/block/ub.c @@ -1309,8 +1309,7 @@ static void ub_scsi_urb_compl(struct ub_dev *sc, struct ub_scsi_cmd *cmd) ub_state_done(sc, cmd, -EIO); } else { - printk(KERN_WARNING "%s: " - "wrong command state %d\n", + printk(KERN_WARNING "%s: wrong command state %d\n", sc->name, cmd->state); ub_state_done(sc, cmd, -EINVAL); return; @@ -1533,8 +1532,7 @@ static void ub_top_sense_done(struct ub_dev *sc, struct ub_scsi_cmd *scmd) return; } if (cmd->state != UB_CMDST_SENSE) { - printk(KERN_WARNING "%s: " - "sense done with bad cmd state %d\n", + printk(KERN_WARNING "%s: sense done with bad cmd state %d\n", sc->name, cmd->state); return; } @@ -1738,7 +1736,7 @@ static int ub_bd_ioctl(struct inode *inode, struct file *filp, } /* - * This is called once a new disk was seen by the block layer or by ub_probe(). + * This is called by check_disk_change if we reported a media change. * The main onjective here is to discover the features of the media such as * the capacity, read-only status, etc. USB storage generally does not * need to be spun up, but if we needed it, this would be the place. @@ -2154,8 +2152,7 @@ static int ub_get_pipes(struct ub_dev *sc, struct usb_device *dev, } if (ep_in == NULL || ep_out == NULL) { - printk(KERN_NOTICE "%s: failed endpoint check\n", - sc->name); + printk(KERN_NOTICE "%s: failed endpoint check\n", sc->name); return -ENODEV; } @@ -2372,7 +2369,7 @@ static void ub_disconnect(struct usb_interface *intf) spin_unlock_irqrestore(&ub_lock, flags); /* - * Fence stall clearnings, operations triggered by unlinkings and so on. + * Fence stall clearings, operations triggered by unlinkings and so on. * We do not attempt to unlink any URBs, because we do not trust the * unlink paths in HC drivers. Also, we get -84 upon disconnect anyway. */ @@ -2435,7 +2432,7 @@ static void ub_disconnect(struct usb_interface *intf) spin_unlock_irqrestore(sc->lock, flags); /* - * There is virtually no chance that other CPU runs times so long + * There is virtually no chance that other CPU runs a timeout so long * after ub_urb_complete should have called del_timer, but only if HCD * didn't forget to deliver a callback on unlink. */ -- cgit v1.2.3