From: Josef Bacik <josef@toxicpanda.com>
To: Ming Lei <tom.leiming@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>,
Caleb Sander Mateos <csander@purestorage.com>,
linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [PATCH 0/9] ublk: fix dispatch to canceled io commands
Date: Wed, 30 Sep 2026 14:17:31 +0000 [thread overview]
Message-ID: <e15d4e3c39e8ed1860b762cfd8e43c92.josef@toxicpanda.com> (raw)
In-Reply-To: <arvOwDHOgvpVW86j@fedora-laptop>
On Tue, Sep 29, 2026 at 09:44:16AM -0500, Ming Lei wrote:
> It looks two races: STOP_DEV vs. START_DEV, STOP_DEV vs. FETCH.
>
> Looks fast io path shouldn't be touched for fixing the races.
>
> > 2. A partial FETCH round whose task exits, once another task
> > completes the round.
> > 3. During recovery, the task of a queue which is ready already
> > exiting before the last queue is ready.
>
> 2 and 3 could be solved in single simpler patch by making use of the
> ub->canceling flag, and it is easier for backport.
Agreed, yours is much simpler, and keeping the flag set for the whole
FETCH round is the right model. I ran it on top of for-next (d70609a2f68c)
with KASAN and lockdep through my reproducers and the ublk selftests. The
oopses for 2 and 3 are gone, and recover_01-04, batch_01-03, generic_17,
stress_01/02/05 and 60 batch QUIESCE_DEV/recover cycles pass.
What's left for 2 and 3 is that the device still comes up. For 2,
START_DEV returns 0 and the new disk fails every request. For 3,
END_USER_RECOVERY returns 0, the device is LIVE, and every read on the
queue whose task exited sits requeued forever, since the queue stays
canceling and nothing kicks the requeue list. With ub->canceling
covering the whole round that's a small check: return -ENODEV from
START_DEV and END_USER_RECOVERY when ub->canceling is set, checked under
cancel_mutex against publishing ub->ub_disk. The server can't fetch
those commands again anyway. Patch 9 of my series did that on the old
model, I'll redo it on top of yours.
For 1, your patch alone still oopses in ublk_queue_rq() from the
partition scan when START_DEV follows STOP_DEV, same as before. I'll
respin my series as just that, on top of your patch and without touching
the commit path: STOP_DEV marks the queues canceling and takes the
fetched commands under ub->mutex, and FETCH marks its command cancelable
before it publishes it, so a cancel from the control path never
completes a command io_uring doesn't have on its cancelable list yet.
For your patch:
Tested-by: Josef Bacik <josef@toxicpanda.com>
Thanks,
Josef
next prev parent reply other threads:[~2026-09-30 14:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-28 16:00 Josef Bacik
2026-09-28 16:00 ` [PATCH 1/9] ublk: keep queue canceling over canceled commands Josef Bacik
2026-09-28 16:46 ` Caleb Sander Mateos
2026-09-28 18:34 ` Josef Bacik
2026-09-28 16:00 ` [PATCH 2/9] ublk: clear ub->canceling with the queue's own flag Josef Bacik
2026-09-28 16:00 ` [PATCH 3/9] ublk: publish io->cmd under io->lock in the commit paths Josef Bacik
2026-09-28 17:53 ` Caleb Sander Mateos
2026-09-29 13:06 ` Josef Bacik
2026-09-28 16:00 ` [PATCH 4/9] ublk: read the io under io->lock in ublk_cancel_cmd() Josef Bacik
2026-09-28 16:00 ` [PATCH 5/9] ublk: complete a command canceled before it was marked from its issuer Josef Bacik
2026-09-28 16:00 ` [PATCH 6/9] ublk: split ublk_claim_cmd() out of ublk_cancel_cmd() Josef Bacik
2026-09-28 16:00 ` [PATCH 7/9] ublk: mark queues and command in one cancel_mutex hold Josef Bacik
2026-09-28 16:00 ` [PATCH 8/9] ublk: claim commands under ub->mutex in ublk_stop_dev() Josef Bacik
2026-09-28 16:00 ` [PATCH 9/9] ublk: refuse to go live over canceled io commands Josef Bacik
2026-09-28 17:35 ` Randy Dunlap
2026-09-29 14:44 ` [PATCH 0/9] ublk: fix dispatch to " Ming Lei
2026-09-30 14:17 ` Josef Bacik [this message]
2026-09-30 16:40 ` Ming Lei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e15d4e3c39e8ed1860b762cfd8e43c92.josef@toxicpanda.com \
--to=josef@toxicpanda.com \
--cc=axboe@kernel.dk \
--cc=csander@purestorage.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tom.leiming@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®