From 0abe39dec065133e3f92a52219c3728fe7d7617f Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Tue, 7 Feb 2023 23:07:00 +0800 Subject: 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 Suggested-by: Nadav Amit Signed-off-by: Ming Lei Link: https://lore.kernel.org/r/20230207150700.545530-1-ming.lei@redhat.com Signed-off-by: Jens Axboe --- drivers/block/ublk_drv.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index d83fe2c2b3ba..e6eceee44366 100644 --- 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) -- cgit v1.2.3