Re: [PATCH 3/4] ublk: cancel commits arriving after the quiesce scan

[shao.mingyin]

> Please add this as a new recovery selftest in v2, so the fix is
> covered.

Thanks for the reproducer. I've prepared a driver revision and a new
recovery selftest using fault_inject's existing delay, with no kernel
hooks. The selftest has three cases; the first is your reproducer. It
waits for an in-flight direct write, quiesces the device, and requires
both the write and QUIESCED before cleanup can issue another control
command. The base stays LIVE; the revised driver passes.

> With zero copy or auto buffer registration the committed request
> may still be in flight when the command is aborted here.

I've made the distinction explicit rather than bypassing the guard at
one call site. After COMMIT's buffer operations, the new command is
marked COMMIT_READY before the old request's references are dropped.
Both non-batch dispatch paths clear that bit before queuing task work.
With cancellation set while dispatch is quiesced, the scan can cancel a
waiting command even if its previous request remains started, while
preserving the guard for a new request already queued to the command.

This also matters if cancellation starts just after COMMIT: an early
check alone misses that case. The third case uses a registered buffer:
it submits COMMIT first and verifies that the write is still
outstanding. The scan must abort all four commands, and releasing the
buffer must then let the committed write complete successfully. My
earlier draft passed the first two cases but failed this one; the
current revision passes all three.

> If ublk_wait_for_idle_io() fails with -EBUSY or -EINTR, ->canceling
> is left set and ublk_cancel_dev() isn't called.

I've documented that the idle-wait error leaves canceling set and skips
the scan. The next late command receives ABORT even though QUIESCE_DEV
returned an error; this change does not roll cancellation back. That
error-path behavior was checked in source, not established by the runtime
tests below.

> The new check fires whenever ->canceling is set, not only for
> QUIESCE_DEV, so the subject and commit log are narrower than the
> change.

Agreed. The new subject is "ublk: cancel late commands while the queue is
canceling", and the log includes STOP_DEV and server exit.

The helper is renamed ublk_complete_canceled_cmd().

> io->cmd and UBLK_IO_FLAG_ACTIVE are stored here before ->canceling
> is read, with no barrier in between ...

I've added explicit paired smp_mb() barriers between command publication
and the cancellation check, and before the scan's ACTIVE load, with a
comment explaining the pairing. Queue quiescing serializes queue_rq, but
the uring command handler can publish independently, so I would rather
make that publication ordering explicit than rely on quiescing alone.

> If ublk_get_data() fails and falls back to requeue ...

The second case holds NEED_GET_DATA across a successful quiesce, then
submits an unmappable address. This reaches the real mapping
failure/requeue path. The base completes only three of four commands
with ABORT; the revised driver completes all four, and the device
reaches QUIESCED after the server's ring/file references are released.
The common post-processing cancellation check covers this fallthrough
path.

All three cases (fault_inject delay, NEED_GET_DATA/requeue, registered
buffer) were run against the same next-20261113 x86_64 base and fixed
guests, using QEMU/KVM with four vCPUs and 2 GiB RAM. The existing
stop_restart and stop_live_restart modes also passed on both kernels,
with eight successful reads per mode. Kernel W=1 and selftest -Werror
builds completed; the driver has the same pre-existing shmem stack-frame
warning on both builds. I have not tested concurrent off-task
registrations, automatic buffer registration, or a weakly ordered
architecture.

Thanks,
Shao