Re: [PATCH v2] scsi: core: Clear power-on UNIT ATTENTION on BIWIN UFS devices

[Alexey Charkov]

On Thu, Oct 29, 2026 at 3:12 PM <sashiko-bot@xxxxxxxxxx> wrote:
[...]
> [Severity: High]
> Can scsi_test_unit_ready() starve waiting for a request tag when retrying
> writes saturate the queue?
>
> scsi_test_unit_ready() calls scsi_execute_cmd() -> scsi_alloc_request() ->
> blk_mq_alloc_request() with flags == 0, which waits in blk_mq_get_tag() for a
> normal non-reserved tag from bitmap_tags.
>
> When write commands receive a power-on/reset unit attention,
> drivers/scsi/scsi_lib.c:scsi_io_completion_action() requeues them immediately:
>
>         case ACTION_RETRY:
>                 /* Retry the same command immediately */
>                 __scsi_queue_insert(cmd, SCSI_MLQUEUE_EH_RETRY, false);
>                 break;
>
> In __scsi_queue_insert() -> blk_mq_requeue_request() ->
> __blk_mq_requeue_request() -> blk_mq_put_driver_tag():
>
>         if (rq->tag == BLK_MQ_NO_TAG || rq->internal_tag == BLK_MQ_NO_TAG)
>                 return;
>
>         __blk_mq_put_driver_tag(rq->mq_hctx, rq);
>
> When an I/O scheduler is attached (such as the default mq-deadline on UFS,
> where dd_init_sched() sets q->async_depth = q->nr_requests so writes can
> consume all tags in sched_tags->bitmap_tags), __blk_mq_requeue_request()
> releases rq->tag but keeps the scheduler tag (rq->internal_tag) across all
> retries. When no I/O scheduler is configured, blk_mq_limit_depth() imposes no
> limit on writes and blk_mq_put_driver_tag() does not release rq->tag on
> requeue because rq->internal_tag == BLK_MQ_NO_TAG.
>
> If enough write requests are queued to fill all non-reserved tags across a
> reset or runtime resume, won't those writes endlessly fail and requeue while
> pinning every tag, blocking scsi_test_unit_ready() in blk_mq_get_tag() until
> a write exceeds the 180s retry window and fails with an I/O error?

Hmm, I've simulated this scenario and it is indeed the case. When a
lot of writes are in-flight at the moment of an injected UFS
error-handler reset, they end up holding every scheduler tag upon
resume, meaning the TUR queues behind them, and the recovery doesn't
happen until after the writes time out with an I/O error (which is
pretty much the original broken behavior this was meant to fix).

Given that, I'm wondering if dealing with this via the UFS devman
interface (as was done in v1) is a better option after all. Bart, what
would you say?

- sd.c looks like a wrong place, because sd never sees resets the UFS
layer does on its own, such as UFS error recovery
- The midlayer (like this version) doesn't seem to work either,
because it only learns about the reset under the host lock, where it
can't issue commands, and queuing them in a process context means they
wait for a tag from the same pool as the offending writes are holding
- The devman path in UFS, on the other hand, knows when UFS does all
of its resets, and has its own reserved tag, so the TUR can go out
inside the reset before LU queues restart (thus no deadlock of the
sorts referenced in edc0596cc04b)

I've run the same test on v1 (TUR through the devman slot): the TUR
went out inside the reset, before ufshcd_err_handler finished, and all
399599 WRITEs in the same window succeeded, with no UNIT ATTENTIONs,
stalls or I/O errors. If we go that way, v3 would be v1 plus setting
lrbp->lun from the request UPIU, as pointed out by Sashiko during the
review of v1.

Best regards,
Alexey