| Message ID | 20180318205904.28541-7-dgilbert@interlog.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
On Sun, 2018-03-18 at 21:59 +0100, Douglas Gilbert wrote: > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -1048,13 +1048,20 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) > scsi_req(req->next_rq)->resid_len = scsi_in(cmd)->resid; > if (scsi_end_request(req, BLK_STS_OK, blk_rq_bytes(req), > blk_rq_bytes(req->next_rq))) > - BUG(); > + WARN(true, "Bidi command with remaining bytes"); > return; > } > } > > /* no bidi support yet, other than in pass-through */ > - BUG_ON(blk_bidi_rq(req)); > + if (unlikely(blk_bidi_rq(req))) { > + WARN_ONCE(true, "Only support bidi command in passthrough"); > + scmd_printk(KERN_ERR, cmd, "Killing bidi command\n"); > + if (scsi_end_request(req, BLK_STS_IOERR, blk_rq_bytes(req), > + blk_rq_bytes(req->next_rq))) > + WARN(true, "Bidi command with remaining bytes"); > + return; > + } > > /* > * Next deal with any sectors which we were able to correctly > @@ -1077,7 +1084,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) > /* Kill remainder if no retrys */ > if (unlikely(blk_stat && scsi_noretry_cmd(cmd))) { > if (scsi_end_request(req, blk_stat, blk_rq_bytes(req), 0)) > - BUG(); > + WARN(true, > + "Bytes remaining after failed, no-retry command"); > return; > } > Shouldn't the WARN() calls be changed into WARN_ONCE() calls to avoid that a large number of complaints appears on the console if these conditions are hit repeatedly? Thanks, Bart.
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 63f79b0dbbf3..be8836d984f5 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1048,13 +1048,20 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) scsi_req(req->next_rq)->resid_len = scsi_in(cmd)->resid; if (scsi_end_request(req, BLK_STS_OK, blk_rq_bytes(req), blk_rq_bytes(req->next_rq))) - BUG(); + WARN(true, "Bidi command with remaining bytes"); return; } } /* no bidi support yet, other than in pass-through */ - BUG_ON(blk_bidi_rq(req)); + if (unlikely(blk_bidi_rq(req))) { + WARN_ONCE(true, "Only support bidi command in passthrough"); + scmd_printk(KERN_ERR, cmd, "Killing bidi command\n"); + if (scsi_end_request(req, BLK_STS_IOERR, blk_rq_bytes(req), + blk_rq_bytes(req->next_rq))) + WARN(true, "Bidi command with remaining bytes"); + return; + } /* * Next deal with any sectors which we were able to correctly @@ -1077,7 +1084,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) /* Kill remainder if no retrys */ if (unlikely(blk_stat && scsi_noretry_cmd(cmd))) { if (scsi_end_request(req, blk_stat, blk_rq_bytes(req), 0)) - BUG(); + WARN(true, + "Bytes remaining after failed, no-retry command"); return; }
The scsi_io_completion function contains three BUG() and BUG_ON() calls. Replace them with WARN variants. Signed-off-by: Douglas Gilbert <dgilbert@interlog.com> --- This was done at a reviewer's request. This patchset did not change those BUG() lines but the git-diff suggested it did. Then the checkpatch.pl warned "BUG() calls considered harmful". BUG() calls are simpler than WARNs because the programmer doesn't have to worry about a continuation after them. The continuations given here may not be ideal. For these reasons the author considers this patch optional. drivers/scsi/scsi_lib.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)