linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block: ublk: improve handling device deletion
@ 2023-02-07 15:07 Ming Lei
  2023-02-08  5:57 ` Ziyang Zhang
  2023-02-09 15:12 ` Jens Axboe
  0 siblings, 2 replies; 4+ messages in thread
From: Ming Lei @ 2023-02-07 15:07 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Ming Lei, Nadav Amit

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>
---
 drivers/block/ublk_drv.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

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)
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-09 15:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-07 15:07 [PATCH] block: ublk: improve handling device deletion Ming Lei
2023-02-08  5:57 ` Ziyang Zhang
2023-02-08  7:31   ` Ming Lei
2023-02-09 15:12 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).