> index 5d83a162d03b..c1d5e4e36125 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1635,8 +1635,11 @@ static blk_status_t scsi_mq_prep_fn(struct request *req)
>
> static void scsi_mq_done(struct scsi_cmnd *cmd)
> {
> + if (unlikely(test_and_set_bit(__SCMD_COMPLETE, &cmd->flags)))
> + return;
> trace_scsi_dispatch_cmd_done(cmd);
> - blk_mq_complete_request(cmd->request);
> + if (unlikely(!blk_mq_complete_request(cmd->request)))
> + clear_bit(__SCMD_COMPLETE, &cmd->flags);
> }
This looks a little odd to me. If we didn't complete the command
someone else did. Why would we clear the bit in this case?
> static void scsi_mq_put_budget(struct blk_mq_hw_ctx *hctx)
> @@ -1701,6 +1704,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
> goto out_dec_host_busy;
> req->rq_flags |= RQF_DONTPREP;
> } else {
> + cmd->flags &= ~SCMD_COMPLETE;
> blk_mq_start_request(req);
> }
>
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index d6fd2aba0380..ded7c7194a28 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -58,6 +58,9 @@ struct scsi_pointer {
> #define SCMD_TAGGED (1 << 0)
> #define SCMD_UNCHECKED_ISA_DMA (1 << 1)
> #define SCMD_INITIALIZED (1 << 2)
> +
> +#define __SCMD_COMPLETE 3
> +#define SCMD_COMPLETE (1 << __SCMD_COMPLETE)
This mixing of atomic and non-atomic bitops looks rather dangerous
to me. Can you add a new atomic_flags just for the completed flag,
and always use the bitops on it for now? I think we can eventually
kill most of the existing flags except for SCMD_TAGGED over the
next merge window or two and then move that over as well.
Otherwise the concept looks fine to me.
