Re: [PATCH RFC] scsi: core: Clear cmd->flags for terminal queuing error

[Bart Van Assche <bvanassche@acm.org>]

On 9/8/26 5:08 AM, John Garry wrote:
> If a request can't be queued in scsi_queue_rq(), then we error out with
> a blk_status_t code. Such a scenario may be the scsi_device has its
> transport offline.
> 
> For BLK_STS_RESOURCE or BLK_STS_AGAIN, the request will be requeued in the
> block layer. For other errors, the block layer ends the request and so
> we won't see it again (until recycled).
> 
> Normally when a request completes scmd->flags is cleared in
> scsi_end_request().
> 
> However for the error scenario mentioned above, scsi_end_request() is
> not called and so scmd->flags is not cleared. As such, when the request
> is recycled we see the old value in scmd->flags.
> 
> Fix this issue by clearing scmd->flags in the error scenario mentioned.
> 
> Signed-off-by: John Garry <john.garry@linux.dev>
> ---
> Setting as an RFC as I am not 100% sure that this is correct as there are
> lots of paths involved. Sashiko pointed out a related issue in
> https://lore.kernel.org/linux-scsi/20260729161243.C918D1F00A3A@smtp.kernel.org/,
> but blk_mq_cleanup_rq() -> scsi_cleanup_rq() -> cmd->flags = 0 is only
> called for by DM code.
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index af27fd3df8d4..295f8b4afaa7 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1967,6 +1967,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>   		 */
>   		if (req->rq_flags & RQF_DONTPREP)
>   			scsi_mq_uninit_cmd(cmd);
> +		cmd->flags = 0;
>   		scsi_run_queue_async(sdev);
>   		break;
>   	}

Is there agreement that all block drivers are affected that define a
.cleanup_rq() callback? Would this be a valid alternative (untested)?

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a26a11c73ee3..0be933e26d67 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2126,6 +2126,12 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx 
*hctx, struct list_head *list,
  			blk_mq_handle_dev_resource(rq, list);
  			goto out;
  		default:
+			/*
+			 * Make sure to release all allocated resources when
+			 * we hit an error, as we will never see this command
+			 * again.
+			 */
+			blk_mq_cleanup_rq(rq);
  			blk_mq_end_request(rq, ret);
  		}
  	} while (!list_empty(list));
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 48aab0df30b7..636854bffd1e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1960,13 +1960,6 @@ static blk_status_t scsi_queue_rq(struct 
blk_mq_hw_ctx *hctx,
  			cmd->result = DID_NO_CONNECT << 16;
  		else
  			cmd->result = DID_ERROR << 16;
-		/*
-		 * Make sure to release all allocated resources when
-		 * we hit an error, as we will never see this command
-		 * again.
-		 */
-		if (req->rq_flags & RQF_DONTPREP)
-			scsi_mq_uninit_cmd(cmd);
  		scsi_run_queue_async(sdev);
  		break;
  	}

Thanks,

Bart.