Re: [PATCH V6 14/24] ublk: fix batch I/O recovery -ENODEV error

[Jens Axboe <axboe@kernel.dk>]

Claude says:

ublk: fix batch I/O recovery -ENODEV error

This commit changes the recovery flow to track per-queue readiness and
clear ubq->canceling when individual queues become ready, rather than
waiting for all queues. This fixes UBLK_U_IO_FETCH_IO_CMDS failing with
-ENODEV during batch I/O recovery.

> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index e6e746cd369e..346596c1e319 100644

[ ... ]

> @@ -2791,37 +2797,52 @@ static void ublk_stop_dev(struct ublk_device *ub)
>  	ublk_cancel_dev(ub);
>  }
>
> -/* reset ublk io_uring queue & io flags */
> -static void ublk_reset_io_flags(struct ublk_device *ub)
> +/* reset per-queue io flags */
> +static void ublk_queue_reset_io_flags(struct ublk_queue *ubq)
>  {
> -	int i, j;
> +	int j;
>
> -	for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
> -		struct ublk_queue *ubq = ublk_get_queue(ub, i);
> -
> -		/* UBLK_IO_FLAG_CANCELED can be cleared now */
> -		spin_lock(&ubq->cancel_lock);
> -		for (j = 0; j < ubq->q_depth; j++)
> -			ubq->ios[j].flags &= ~UBLK_IO_FLAG_CANCELED;
> -		spin_unlock(&ubq->cancel_lock);
> -		ubq->fail_io = false;
> -	}
> -	mutex_lock(&ub->cancel_mutex);
> -	ublk_set_canceling(ub, false);
> -	mutex_unlock(&ub->cancel_mutex);
> +	/* UBLK_IO_FLAG_CANCELED can be cleared now */
> +	spin_lock(&ubq->cancel_lock);
> +	for (j = 0; j < ubq->q_depth; j++)
> +		ubq->ios[j].flags &= ~UBLK_IO_FLAG_CANCELED;
> +	spin_unlock(&ubq->cancel_lock);
> +	ubq->fail_io = false;
> +	ubq->canceling = false;
                         ^^^^

The comment on ublk_set_canceling() says "Do not set these flags directly"
and requires cancel_mutex to be held. Is it intentional to set
ubq->canceling directly here without the mutex? The old code used
ublk_set_canceling() with proper locking.

[ ... ]

> @@ -3411,7 +3432,14 @@ static int ublk_batch_unprep_io(struct ublk_queue *ubq,
>  {
>  	struct ublk_io *io = &ubq->ios[elem->tag];
>
> -	data->ub->nr_io_ready--;
> +	/*
> +	 * If queue was ready before this decrement, it won't be anymore,
> +	 * so we need to decrement the queue ready count too.
> +	 */
> +	if (ublk_queue_ready(ubq))
> +		data->ub->nr_queue_ready--;
> +	ubq->nr_io_ready--;
> +
>  	ublk_io_lock(io);
>  	io->flags = 0;
>  	ublk_io_unlock(io);

When ublk_batch_unprep_io() is used for rollback after a batch prep
failure, the counters are correctly decremented, but if the queue
became ready during prep (triggering ublk_queue_reset_io_flags() which
set ubq->canceling = false), the canceling flag is not restored.

Consider this scenario during recovery:
1. Batch prep command with tags 0,1,2,3 (valid) and tag 5 (invalid)
2. Tags 0-3 prep succeeds, queue becomes ready, canceling set to false
3. Tag 5 validation fails (tag >= q_depth)
4. Rollback via ublk_batch_revert_prep_cmd() calls ublk_batch_unprep_io()

After rollback: nr_io_ready=0, nr_queue_ready=0, but canceling=false.

With canceling incorrectly false, new requests in __ublk_queue_rq_common()
would not be aborted and could reach ublk_queue_cmd() which accesses
ubq->ios[tag].cmd - but the IO slots were cleared by rollback.

Should ublk_batch_unprep_io() restore ubq->canceling = true when the
queue transitions from ready to not-ready?

-- 
Jens Axboe