ublk: move device reset into ublk_ch_release()

ublk_ch_release() is called after ublk char device is closed, when all
uring_cmd are done, so it is perfect fine to move ublk device reset to
ublk_ch_release() from ublk_ctrl_start_recovery().

This way can avoid to grab the exiting daemon task_struct too long.

However, reset of the following ublk IO flags has to be moved until ublk
io_uring queues are ready:

- ubq->canceling

For requeuing IO in case of ublk_nosrv_dev_should_queue_io() before device
is recovered

- ubq->fail_io

For failing IO in case of UBLK_F_USER_RECOVERY_FAIL_IO before device is
recovered

- ublk_io->flags

For preventing using io->cmd

With this way, recovery is simplified a lot.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
Link: https://lore.kernel.org/r/20250416035444.99569-5-ming.lei@redhat.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index a479969fd77e4da4bfd331273d463f3385786725..1fe39cf85b2f4dfebee08cd24f6bd1a6deb0b17e 100644 (file)
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1074,7 +1074,7 @@ static inline struct ublk_uring_cmd_pdu *ublk_get_uring_cmd_pdu(
 
 static inline bool ubq_daemon_is_dying(struct ublk_queue *ubq)
 {
-       return ubq->ubq_daemon->flags & PF_EXITING;
+       return !ubq->ubq_daemon || ubq->ubq_daemon->flags & PF_EXITING;
 }
 
 /* todo: handle partial completion */
@@ -1470,6 +1470,37 @@ static const struct blk_mq_ops ublk_mq_ops = {
        .timeout        = ublk_timeout,
 };
 
+static void ublk_queue_reinit(struct ublk_device *ub, struct ublk_queue *ubq)
+{
+       int i;
+
+       /* All old ioucmds have to be completed */
+       ubq->nr_io_ready = 0;
+
+       /*
+        * old daemon is PF_EXITING, put it now
+        *
+        * It could be NULL in case of closing one quisced device.
+        */
+       if (ubq->ubq_daemon)
+               put_task_struct(ubq->ubq_daemon);
+       /* We have to reset it to NULL, otherwise ub won't accept new FETCH_REQ */
+       ubq->ubq_daemon = NULL;
+       ubq->timeout = false;
+
+       for (i = 0; i < ubq->q_depth; i++) {
+               struct ublk_io *io = &ubq->ios[i];
+
+               /*
+                * UBLK_IO_FLAG_CANCELED is kept for avoiding to touch
+                * io->cmd
+                */
+               io->flags &= UBLK_IO_FLAG_CANCELED;
+               io->cmd = NULL;
+               io->addr = 0;
+       }
+}
+
 static int ublk_ch_open(struct inode *inode, struct file *filp)
 {
        struct ublk_device *ub = container_of(inode->i_cdev,
@@ -1481,10 +1512,26 @@ static int ublk_ch_open(struct inode *inode, struct file *filp)
        return 0;
 }
 
+static void ublk_reset_ch_dev(struct ublk_device *ub)
+{
+       int i;
+
+       for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
+               ublk_queue_reinit(ub, ublk_get_queue(ub, i));
+
+       /* set to NULL, otherwise new ubq_daemon cannot mmap the io_cmd_buf */
+       ub->mm = NULL;
+       ub->nr_queues_ready = 0;
+       ub->nr_privileged_daemon = 0;
+}
+
 static int ublk_ch_release(struct inode *inode, struct file *filp)
 {
        struct ublk_device *ub = filp->private_data;
 
+       /* all uring_cmd has been done now, reset device & ubq */
+       ublk_reset_ch_dev(ub);
+
        clear_bit(UB_STATE_OPEN, &ub->state);
        return 0;
 }
@@ -1831,6 +1878,24 @@ static void ublk_nosrv_work(struct work_struct *work)
        ublk_cancel_dev(ub);
 }
 
+/* reset ublk io_uring queue & io flags */
+static void ublk_reset_io_flags(struct ublk_device *ub)
+{
+       int i, 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->canceling = false;
+               ubq->fail_io = false;
+       }
+}
+
 /* device can only be started after all IOs are ready */
 static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
        __must_hold(&ub->mutex)
@@ -1844,8 +1909,12 @@ static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
                if (capable(CAP_SYS_ADMIN))
                        ub->nr_privileged_daemon++;
        }
-       if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues)
+
+       if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues) {
+               /* now we are ready for handling ublk io request */
+               ublk_reset_io_flags(ub);
                complete_all(&ub->completion);
+       }
 }
 
 static void ublk_handle_need_get_data(struct ublk_device *ub, int q_id,
@@ -2954,41 +3023,14 @@ static int ublk_ctrl_set_params(struct ublk_device *ub,
        return ret;
 }
 
-static void ublk_queue_reinit(struct ublk_device *ub, struct ublk_queue *ubq)
-{
-       int i;
-
-       WARN_ON_ONCE(!(ubq->ubq_daemon && ubq_daemon_is_dying(ubq)));
-
-       /* All old ioucmds have to be completed */
-       ubq->nr_io_ready = 0;
-       /* old daemon is PF_EXITING, put it now */
-       put_task_struct(ubq->ubq_daemon);
-       /* We have to reset it to NULL, otherwise ub won't accept new FETCH_REQ */
-       ubq->ubq_daemon = NULL;
-       ubq->timeout = false;
-
-       for (i = 0; i < ubq->q_depth; i++) {
-               struct ublk_io *io = &ubq->ios[i];
-
-               /* forget everything now and be ready for new FETCH_REQ */
-               io->flags = 0;
-               io->cmd = NULL;
-               io->addr = 0;
-       }
-}
-
 static int ublk_ctrl_start_recovery(struct ublk_device *ub,
                const struct ublksrv_ctrl_cmd *header)
 {
        int ret = -EINVAL;
-       int i;
 
        mutex_lock(&ub->mutex);
        if (ublk_nosrv_should_stop_dev(ub))
                goto out_unlock;
-       if (!ub->nr_queues_ready)
-               goto out_unlock;
        /*
         * START_RECOVERY is only allowd after:
         *
@@ -3012,12 +3054,6 @@ static int ublk_ctrl_start_recovery(struct ublk_device *ub,
                goto out_unlock;
        }
        pr_devel("%s: start recovery for dev id %d.\n", __func__, header->dev_id);
-       for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
-               ublk_queue_reinit(ub, ublk_get_queue(ub, i));
-       /* set to NULL, otherwise new ubq_daemon cannot mmap the io_cmd_buf */
-       ub->mm = NULL;
-       ub->nr_queues_ready = 0;
-       ub->nr_privileged_daemon = 0;
        init_completion(&ub->completion);
        ret = 0;
  out_unlock:
@@ -3030,7 +3066,6 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
 {
        int ublksrv_pid = (int)header->data[0];
        int ret = -EINVAL;
-       int i;
 
        pr_devel("%s: Waiting for new ubq_daemons(nr: %d) are ready, dev id %d...\n",
                        __func__, ub->dev_info.nr_hw_queues, header->dev_id);
@@ -3050,22 +3085,10 @@ static int ublk_ctrl_end_recovery(struct ublk_device *ub,
                goto out_unlock;
        }
        ub->dev_info.ublksrv_pid = ublksrv_pid;
+       ub->dev_info.state = UBLK_S_DEV_LIVE;
        pr_devel("%s: new ublksrv_pid %d, dev id %d\n",
                        __func__, ublksrv_pid, header->dev_id);
-
-       blk_mq_quiesce_queue(ub->ub_disk->queue);
-       ub->dev_info.state = UBLK_S_DEV_LIVE;
-       for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
-               struct ublk_queue *ubq = ublk_get_queue(ub, i);
-
-               ubq->canceling = false;
-               ubq->fail_io = false;
-       }
-       blk_mq_unquiesce_queue(ub->ub_disk->queue);
-       pr_devel("%s: queue unquiesced, dev id %d.\n",
-                       __func__, header->dev_id);
        blk_mq_kick_requeue_list(ub->ub_disk->queue);
-
        ret = 0;
  out_unlock:
        mutex_unlock(&ub->mutex);