[PATCH 0/1] Fix not fully initialized SCSI commands

[PATCH 0/1] Fix not fully initialized SCSI commands

[Anastasia Kovaleva]

We have encountered the following type of logs on initiators:

kernel: sd 16:0:1:84: [sdts] tag#405 timing out command, waited 720s
kernel: sd 16:0:1:84: [sdts] tag#405 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=66636s

The initiator uses dm-mpath for multipathing, the SCSI mid layer, and
the QLogic FC HBA driver (qla2xxx). After debugging, the following call
stack was identified:

blk_mq_sched_dispatch_requests()
  blk_mq_dispatch_rq_list()
    dm_mq_queue_rq()
      map_request()
        ti->type->clone_and_map_rq()    // New cloned request with tag 405
        blk_insert_cloned_request()
          scsi_queue_rq()
            qla2xxx_mqueuecommand()
              qla2xxx_dif_start_scsi_mq()

If qla2xxx_dif_start_scsi_mq() returns an error for any reason (e.g.,
due to extremely heavy traffic causing the driver to exhaust its
handles), scsi_done() -> scsi_end_request() is not called within
qla2xxx_mqueuecommand(). As a result, the SCMD_INITIALIZED flag
remains set. Next, map_request() releases the cloned request and
requeues the original request. While the cloned request is released, the
associated SCSI command retains stale data from the previous command.

If all I/O traffic stops for some extended period of time, and later
resumes, the following scenario may occur:

blk_mq_sched_dispatch_requests()
  blk_mq_dispatch_rq_list()
    dm_mq_queue_rq()
      map_request()
        ti->type->clone_and_map_rq()    // New cloned request uses tag 405 again
        blk_insert_cloned_request()
          scsi_queue_rq()


Within scsi_queue_rq(), the scsi_init_command() function does not call
scsi_initialize_rq() because the SCMD_INITIALIZED flag is already set.
Because of that, when the command completes in scsi_complete(), the
scsi_cmd_runtime_exceeded() check returns true, causing the command to
fail.

This issue appears after the commit 4abafdc4360d ("block: remove the
initialize_rq_fn blk_mq_ops method"). Before this change, the
initialize_rq_fn method forcibly initialized the SCSI command in
blk_get_request(). There may be other places where a command is queued
in scsi_queue_rq() but scsi_done() is not called.

Anastasia Kovaleva (1):
  scsi: uninit not completed scsi cmd

 drivers/scsi/scsi_lib.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--
2.40.3

[PATCH 1/1] scsi: uninit not completed scsi cmd

[Anastasia Kovaleva]

Scsi commands that have not been completed with scsi_done() do not clear
the SCMD_INITIALIZED flag and therefore will not be properly
reinitialized. Thus, the next time the scsi_cmnd structure is used, the
scsi command may fail in scsi_cmd_runtime_exceeced() due to the old
jiffies_at_alloc field of the scsi command:

 kernel: sd 16:0:1:84: [sdts] tag#405 timing out command, waited 720s
 kernel: sd 16:0:1:84: [sdts] tag#405 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_OK cmd_age=66636s

Clear the SCMD_INITIALIZED flag for scsi commands, that have not been
completed by SCSI, so that they can be initialised when queueing.

Fixes: 4abafdc4360d ("block: remove the initialize_rq_fn blk_mq_ops method")
Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com>
---
 drivers/scsi/scsi_lib.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 79a8fb317a2d..db4c0f07ea72 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1237,8 +1237,12 @@ EXPORT_SYMBOL_GPL(scsi_alloc_request);
  */
 static void scsi_cleanup_rq(struct request *rq)
 {
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
+
+	cmd->flags &= ~SCMD_INITIALIZED;
+
 	if (rq->rq_flags & RQF_DONTPREP) {
-		scsi_mq_uninit_cmd(blk_mq_rq_to_pdu(rq));
+		scsi_mq_uninit_cmd(cmd);
 		rq->rq_flags &= ~RQF_DONTPREP;
 	}
 }
-- 
2.40.3

Re: [PATCH 0/1] Fix not fully initialized SCSI commands

[James Bottomley]

On Mon, 2025-03-24 at 11:49 +0300, Anastasia Kovaleva wrote:
> We have encountered the following type of logs on initiators:

Hey, Anna,

Just so you know what's going on, Yadro is on the US SDN sanctions
list:

https://sanctionssearch.ofac.treas.gov/Details.aspx?id=39140

Which means that under the latest LF guidance:

https://www.linuxfoundation.org/blog/navigating-global-regulations-and-open-source-us-ofac-sanctions

We could take your patch as is, but we can't discuss changing it with
you (Point 3: avoid two way engagement).

Really sorry about this,

Regards,

James Bottomley