From: Josef Bacik Since commit "ublk: keep a canceled FETCH round canceling until the server is gone", a device whose FETCH round saw a cancel keeps its queues canceling until the server goes away, but START_DEV and END_USER_RECOVERY still bring it up. Without UBLK_F_USER_RECOVERY, or with UBLK_F_USER_RECOVERY_FAIL_IO, every request of the new disk fails. With UBLK_F_USER_RECOVERY, requests are requeued and never kicked: after START_DEV the partition scan hangs under disk->open_mutex, and after END_USER_RECOVERY every read parks while the command returned 0. The server cannot fetch the canceled commands again, so the device can't serve I/O until it restarts anyway. Return -EBUSY from START_DEV and END_USER_RECOVERY while ub->canceling is set. In ublk_ctrl_start_dev() check it and publish ub->ub_disk in one cancel_mutex section, and have ublk_start_cancel() read the disk in its cancel_mutex section. Today ublk_start_cancel() samples the disk before taking the mutex, so a server dying during its own START_DEV can mark the queues without quiescing a disk START_DEV published in between, with its first I/O past the canceling check. Now either START_DEV sees the cancel, or the cancel sees the disk and quiesces it before marking. The END_USER_RECOVERY check is best effort: the disk exists there, and a cancel after it is the ordinary death of the new server, which ublk_start_cancel() handles by quiescing and marking. Assisted-by: LLM Reviewed-by: Ming Lei Signed-off-by: Josef Bacik Signed-off-by: Ming Lei --- Documentation/block/ublk.rst | 10 ++++++++-- drivers/block/ublk_drv.c | 38 ++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst index ed3285e361ad..f07611b0d489 100644 --- a/Documentation/block/ublk.rst +++ b/Documentation/block/ublk.rst @@ -118,7 +118,11 @@ managing and controlling ublk devices with help of several control commands: After the server prepares userspace resources (such as creating I/O handler threads & io_uring for handling ublk IO), this command is sent to the driver for allocating & exposing ``/dev/ublkb*``. Parameters set via - ``UBLK_CMD_SET_PARAMS`` are applied for creating the device. + ``UBLK_CMD_SET_PARAMS`` are applied for creating the device. The command + fails with ``-EBUSY`` if an I/O command fetched by the current server + was canceled, because its io_uring is gone. The server can't fetch it + again, and the device can be started again once the server has closed + ``/dev/ublkc*``. - ``UBLK_CMD_STOP_DEV`` @@ -202,7 +206,9 @@ managing and controlling ublk devices with help of several control commands: command is accepted after ublk device is quiesced and a new process has opened ``/dev/ublkc*`` and get all ublk queues be ready. When this command returns, ublk device is unquiesced and new I/O requests are passed to the - new process. + new process. It fails with ``-EBUSY`` if an I/O command of the new + process was canceled already. The recovery can be started over once the + new process has closed ``/dev/ublkc*``. - user recovery feature description diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index a0b8b730e4f5..24a7f80e191c 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -2759,9 +2759,11 @@ static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq) static void ublk_start_cancel(struct ublk_device *ub) { - struct gendisk *disk = ublk_get_disk(ub); + struct gendisk *disk; + /* sync with ublk_ctrl_start_dev() publishing the disk */ mutex_lock(&ub->cancel_mutex); + disk = ublk_get_disk(ub); if (ub->canceling) goto out; @@ -4575,6 +4577,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, .dma_alignment = 3, }; struct gendisk *disk; + bool canceled; int ret = -EINVAL; if (ublksrv_pid <= 0) @@ -4665,8 +4668,24 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, disk->fops = &ub_fops; disk->private_data = ub; + /* + * A command of this FETCH round was canceled and can't be fetched + * again, don't bring up a disk over it. Check and publish the disk + * in one cancel_mutex section: either this sees ub->canceling, or + * ublk_start_cancel() sees the disk and quiesces it before marking + * the queues. + */ + mutex_lock(&ub->cancel_mutex); + canceled = ub->canceling; + if (!canceled) + ub->ub_disk = disk; + mutex_unlock(&ub->cancel_mutex); + if (canceled) { + put_disk(disk); + ret = -EBUSY; + goto out_unlock; + } ub->dev_info.ublksrv_pid = ub->ublksrv_tgid; - ub->ub_disk = disk; ublk_apply_params(ub); @@ -5243,6 +5262,7 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub, const struct ublksrv_ctrl_cmd *header) { int ublksrv_pid = (int)header->data[0]; + bool canceled; int ret = -EINVAL; pr_devel("%s: Waiting for all FETCH_REQs, dev id %d...\n", __func__, @@ -5266,6 +5286,20 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub, ret = -EBUSY; goto out_unlock; } + + /* + * As in ublk_ctrl_start_dev(), a canceled command can't be fetched + * again. Best effort: the disk exists here, and a cancel after this + * check is the ordinary death of the new server, which + * ublk_start_cancel() handles by quiescing and marking. + */ + mutex_lock(&ub->cancel_mutex); + canceled = ub->canceling; + mutex_unlock(&ub->cancel_mutex); + if (canceled) { + ret = -EBUSY; + goto out_unlock; + } ub->dev_info.ublksrv_pid = ub->ublksrv_tgid; ub->dev_info.state = UBLK_S_DEV_LIVE; pr_devel("%s: new ublksrv_pid %d, dev id %d\n", -- 2.55.0