Skip to content

Commit

Permalink
ub: remove BUG() after __blk_end_request and fix the condition causin…
Browse files Browse the repository at this point in the history
…g it

When __blk_end_request returns nonzero, it means that the request was
not completely processed and some BIOs are still attached. Since we
have dequeued it by that time, it means leaking requests and hanging
processes, which is why BUG() was in there. In ub this happens if
a packet request ends normally, but with residue (e.g. when scsi_id
issues INQUIRY).

The fix is to make sure that arguments passed to __blk_end_request
are correct: the full request length and not just transferred length.
The transferred length is indicated to applications by adjusting
rq->data_len with old, unchanged code outside of this patch.

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>
Cc: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Cc: Greg KH <greg@kroah.com>
Cc: Boaz Harrosh <bharrosh@panasas.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Pete Zaitcev authored and Linus Torvalds committed Apr 9, 2008
1 parent 7180c4c commit ef45cb6
Showing 1 changed file with 29 additions and 9 deletions.
38 changes: 29 additions & 9 deletions drivers/block/ub.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* and is not licensed separately. See file COPYING for details.
*
* TODO (sorted by decreasing priority)
* -- Return sense now that rq allows it (we always auto-sense anyway).
* -- set readonly flag for CDs, set removable flag for CF readers
* -- do inquiry and verify we got a disk and not a tape (for LUN mismatch)
* -- verify the 13 conditions and do bulk resets
Expand Down Expand Up @@ -359,7 +360,8 @@ static void ub_cmd_build_block(struct ub_dev *sc, struct ub_lun *lun,
static void ub_cmd_build_packet(struct ub_dev *sc, struct ub_lun *lun,
struct ub_scsi_cmd *cmd, struct ub_request *urq);
static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
static void ub_end_rq(struct request *rq, unsigned int status);
static void ub_end_rq(struct request *rq, unsigned int status,
unsigned int cmd_len);
static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,
struct ub_request *urq, struct ub_scsi_cmd *cmd);
static int ub_submit_scsi(struct ub_dev *sc, struct ub_scsi_cmd *cmd);
Expand Down Expand Up @@ -642,13 +644,13 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)

if (atomic_read(&sc->poison)) {
blkdev_dequeue_request(rq);
ub_end_rq(rq, DID_NO_CONNECT << 16);
ub_end_rq(rq, DID_NO_CONNECT << 16, blk_rq_bytes(rq));
return 0;
}

if (lun->changed && !blk_pc_request(rq)) {
blkdev_dequeue_request(rq);
ub_end_rq(rq, SAM_STAT_CHECK_CONDITION);
ub_end_rq(rq, SAM_STAT_CHECK_CONDITION, blk_rq_bytes(rq));
return 0;
}

Expand Down Expand Up @@ -701,7 +703,7 @@ static int ub_request_fn_1(struct ub_lun *lun, struct request *rq)

drop:
ub_put_cmd(lun, cmd);
ub_end_rq(rq, DID_ERROR << 16);
ub_end_rq(rq, DID_ERROR << 16, blk_rq_bytes(rq));
return 0;
}

Expand Down Expand Up @@ -770,6 +772,7 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
struct ub_request *urq = cmd->back;
struct request *rq;
unsigned int scsi_status;
unsigned int cmd_len;

rq = urq->rq;

Expand All @@ -779,8 +782,18 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)
rq->data_len = 0;
else
rq->data_len -= cmd->act_len;
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;
}
}
scsi_status = 0;
} else {
if (blk_pc_request(rq)) {
/* UB_SENSE_SIZE is smaller than SCSI_SENSE_BUFFERSIZE */
Expand All @@ -801,23 +814,30 @@ static void ub_rw_cmd_done(struct ub_dev *sc, struct ub_scsi_cmd *cmd)

urq->rq = NULL;

cmd_len = cmd->len;
ub_put_cmd(lun, cmd);
ub_end_rq(rq, scsi_status);
ub_end_rq(rq, scsi_status, cmd_len);
blk_start_queue(lun->disk->queue);
}

static void ub_end_rq(struct request *rq, unsigned int scsi_status)
static void ub_end_rq(struct request *rq, unsigned int scsi_status,
unsigned int cmd_len)
{
int error;
long rqlen;

if (scsi_status == 0) {
error = 0;
} else {
error = -EIO;
rq->errors = scsi_status;
}
if (__blk_end_request(rq, error, blk_rq_bytes(rq)))
BUG();
rqlen = blk_rq_bytes(rq); /* Oddly enough, this is the residue. */
if (__blk_end_request(rq, error, cmd_len)) {
printk(KERN_WARNING DRV_NAME
": __blk_end_request blew, %s-cmd total %u rqlen %ld\n",
blk_pc_request(rq)? "pc": "fs", cmd_len, rqlen);
}
}

static int ub_rw_cmd_retry(struct ub_dev *sc, struct ub_lun *lun,
Expand Down

0 comments on commit ef45cb6

Please sign in to comment.