Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

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

On 9/22/26 11:09 PM, Peter Wang wrote:
> But whether the subsequent re-queue (DID_REQUEUE) violates, I
> haven't seen any prohibition?
While this is not written down explicitly anywhere as far as I
know, requeuing from inside the host error handler is not allowed
at all.
> Because currently the UFS error handler (ufshcd_err_handler)
> directly re-queues (By set DID_REQUEUE) the unfinish command,
> and err handler might be triggered in many places, for example,
> like UIC errors, directly resetting and re-queueing should be
> reasonable and can be handled quickly.

Have you noticed the following code in drivers/scsi/scsi_error.c?

	scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);

> If it cannot directly re-queue, then ufshcd_eh_host_reset_handler
> would need another similar function that does not handle re-queueing.
> But this looks unnecessary, direct re-queuing seems safe?

Requeuing from the .eh_host_reset_handler() is not safe at all. It may
trigger list corruption as follows:
1. The .eh_host_reset_handler() callback sets scmd->result to
    DID_REQUEUE << 16 and calls scsi_done(scmd). This causes
    blk_mq_requeue_request() to insert the SCSI command into
    &q->requeue_list. The SCSI command stays there because the request
    queue is not run immediately because the SCSI host is in the state
    SHOST_RECOVERY.
2. The .eh_host_reset_handler() callback finishes and returns SUCCESS.
3. The SCSI EH thread in scsi_eh_host_reset() sees that the reset
    succeeded and moves cmd to done_q.
4. The SCSI EH thread executes scsi_eh_flush_done_q(done_q).
5. scsi_eh_flush_done_q() invokes
    scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY) for the same scmd
    that is already present on the request queue requeue list.
6. scsi_queue_insert() calls __scsi_queue_insert(), which invokes
    blk_mq_requeue_request() a second time. This causes list corruption.
    Code that examines the corrupted requeue list will either crash or
    trigger an infinite loop.

Bart.