block: ublk: improve handling device deletion

Inside ublk_ctrl_del_dev(), when the device is removed, we wait
until the device number is freed with holding global lock of
ublk_ctl_mutex, this way isn't friendly from user viewpoint:

1) if device is in-use, the current delete command hangs in
ublk_ctrl_del_dev(), and user can't break from the handling
because wait_event() is used

2) global lock is held, so any new device can't be added and
other old devices can't be removed.

Improve the deleting handling by the following way, suggested by
Nadav:

1) wait without holding the global lock

2) replace wait_event() with wait_event_interruptible()

Reported-by: Nadav Amit <nadav.amit@gmail.com>
Suggested-by: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Link: https://lore.kernel.org/r/20230207150700.545530-1-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 d83fe2c2b3bab2a1ec10d761b2edb0f6092ffff1..e6eceee44366ca830c63bd5fb5eea57c06a90a40 100644 (file)
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -150,6 +150,7 @@ struct ublk_device {
 
 #define UB_STATE_OPEN          0
 #define UB_STATE_USED          1
+#define UB_STATE_DELETED       2
        unsigned long           state;
        int                     ub_number;
 
@@ -1804,20 +1805,33 @@ static int ublk_ctrl_del_dev(struct ublk_device **p_ub)
        if (ret)
                return ret;
 
-       ublk_remove(ub);
+       if (!test_bit(UB_STATE_DELETED, &ub->state)) {
+               ublk_remove(ub);
+               set_bit(UB_STATE_DELETED, &ub->state);
+       }
 
        /* Mark the reference as consumed */
        *p_ub = NULL;
        ublk_put_device(ub);
+       mutex_unlock(&ublk_ctl_mutex);
 
        /*
         * Wait until the idr is removed, then it can be reused after
         * DEL_DEV command is returned.
+        *
+        * If we returns because of user interrupt, future delete command
+        * may come:
+        *
+        * - the device number isn't freed, this device won't or needn't
+        *   be deleted again, since UB_STATE_DELETED is set, and device
+        *   will be released after the last reference is dropped
+        *
+        * - the device number is freed already, we will not find this
+        *   device via ublk_get_device_from_id()
         */
-       wait_event(ublk_idr_wq, ublk_idr_freed(idx));
-       mutex_unlock(&ublk_ctl_mutex);
+       wait_event_interruptible(ublk_idr_wq, ublk_idr_freed(idx));
 
-       return ret;
+       return 0;
 }
 
 static inline void ublk_ctrl_cmd_dump(struct io_uring_cmd *cmd)