[PATCH v2 6/6] scsi_io_completion convert BUGs to WARNs

[Douglas Gilbert]

The scsi_io_completion function contains three BUG() and BUG_ON() calls.
Replace them with WARN variants.

Signed-off-by: Douglas Gilbert 
---
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(-)

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;
}
 
-- 
2.14.1

Re: [PATCH v2 6/6] scsi_io_completion convert BUGs to WARNs

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 6/6] scsi_io_completion convert BUGs to WARNs

[Bart Van Assche]

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.