| Message ID | 20181129103709.nthxjmmmlb2pdr6g@kili.mountain (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | scsi: Fix a harmless double shift bug | expand |
On Thu, Nov 29, 2018 at 01:37:10PM +0300, Dan Carpenter wrote: > Smatch generates a warning: > > drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit number > > The problem is that SCMD_STATE_COMPLETE is supposed to be bit number 0 > and not a mask like "(1 << 0)". It is used like this: > > if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) > > The test_and_set_bit() has a shift built in so it's a double left shift > and uses bit number 1 instead of number 0. This bug is harmless because > it's done consistently and it doesn't clash with any other flags. > > Fixes: f1342709d18a ("scsi: Do not rely on blk-mq for double completions") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Nice catch, thanks for the fix. Reviewed-by: Keith Busch <keith.busch@intel.com>
Jens, This went in through your tree. Can you please pick this fix up? > On Thu, Nov 29, 2018 at 01:37:10PM +0300, Dan Carpenter wrote: >> Smatch generates a warning: >> >> drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit number >> >> The problem is that SCMD_STATE_COMPLETE is supposed to be bit number 0 >> and not a mask like "(1 << 0)". It is used like this: >> >> if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) >> >> The test_and_set_bit() has a shift built in so it's a double left shift >> and uses bit number 1 instead of number 0. This bug is harmless because >> it's done consistently and it doesn't clash with any other flags. >> >> Fixes: f1342709d18a ("scsi: Do not rely on blk-mq for double completions") >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Nice catch, thanks for the fix. > > Reviewed-by: Keith Busch <keith.busch@intel.com> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
On 12/7/18 8:20 PM, Martin K. Petersen wrote: > > Jens, > > This went in through your tree. Can you please pick this fix up? Yep, applied, thanks Dan.
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 3de905e205ce..d85e6befa26b 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -62,7 +62,7 @@ struct scsi_pointer { #define SCMD_PRESERVED_FLAGS (SCMD_UNCHECKED_ISA_DMA | SCMD_INITIALIZED) /* for scmd->state */ -#define SCMD_STATE_COMPLETE (1 << 0) +#define SCMD_STATE_COMPLETE 0 struct scsi_cmnd { struct scsi_request req;
Smatch generates a warning: drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit number The problem is that SCMD_STATE_COMPLETE is supposed to be bit number 0 and not a mask like "(1 << 0)". It is used like this: if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state)) The test_and_set_bit() has a shift built in so it's a double left shift and uses bit number 1 instead of number 0. This bug is harmless because it's done consistently and it doesn't clash with any other flags. Fixes: f1342709d18a ("scsi: Do not rely on blk-mq for double completions") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- include/scsi/scsi_cmnd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)