* [PATCH 1/2] ublk: improve getting & putting ublk device
2024-02-23 7:55 [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
@ 2024-02-23 7:55 ` Ming Lei
2024-02-23 7:55 ` [PATCH 2/2] ublk: add UBLK_CMD_DEL_DEV_ASYNC Ming Lei
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2024-02-23 7:55 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Ming Lei
Firstly convert get_device() and put_device() into ublk_get_device()
and ublk_put_device().
Secondly annotate ublk_get_device() & ublk_put_device() as noinline
for trace, especially it is often to trigger device deletion hang
when incorrect order is used on ublkc mmap, ublkc close,
io_uring_sqe_unregister_file, ublkb close.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 01afe90a47ac..06d88d2008ba 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -605,14 +605,16 @@ static inline bool ublk_need_get_data(const struct ublk_queue *ubq)
return ubq->flags & UBLK_F_NEED_GET_DATA;
}
-static struct ublk_device *ublk_get_device(struct ublk_device *ub)
+/* Called in slow path only, keep it noinline for trace purpose */
+static noinline struct ublk_device *ublk_get_device(struct ublk_device *ub)
{
if (kobject_get_unless_zero(&ub->cdev_dev.kobj))
return ub;
return NULL;
}
-static void ublk_put_device(struct ublk_device *ub)
+/* Called in slow path only, keep it noinline for trace purpose */
+static noinline void ublk_put_device(struct ublk_device *ub)
{
put_device(&ub->cdev_dev);
}
@@ -671,7 +673,7 @@ static void ublk_free_disk(struct gendisk *disk)
struct ublk_device *ub = disk->private_data;
clear_bit(UB_STATE_USED, &ub->state);
- put_device(&ub->cdev_dev);
+ ublk_put_device(ub);
}
static void ublk_store_owner_uid_gid(unsigned int *owner_uid,
@@ -2142,7 +2144,7 @@ static void ublk_remove(struct ublk_device *ub)
cancel_work_sync(&ub->stop_work);
cancel_work_sync(&ub->quiesce_work);
cdev_device_del(&ub->cdev, &ub->cdev_dev);
- put_device(&ub->cdev_dev);
+ ublk_put_device(ub);
ublks_added--;
}
@@ -2235,7 +2237,7 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd)
if (ub->nr_privileged_daemon != ub->nr_queues_ready)
set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
- get_device(&ub->cdev_dev);
+ ublk_get_device(ub);
ub->dev_info.state = UBLK_S_DEV_LIVE;
if (ublk_dev_is_zoned(ub)) {
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ublk: add UBLK_CMD_DEL_DEV_ASYNC
2024-02-23 7:55 [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
2024-02-23 7:55 ` [PATCH 1/2] ublk: improve getting & putting ublk device Ming Lei
@ 2024-02-23 7:55 ` Ming Lei
2024-02-29 1:27 ` [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
2024-02-29 1:47 ` Jens Axboe
3 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2024-02-23 7:55 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Ming Lei
The current command UBLK_CMD_DEL_DEV won't return until the device is
released, this way looks more reliable, but makes userspace more
difficult to implement, especially about orders: unmap command
buffer(which holds one ublkc reference), ublkc close,
io_uring_file_unregister, ublkb close.
Add UBLK_CMD_DEL_DEV_ASYNC so that device deletion won't wait release,
then userspace needn't worry about the above order. Actually both loop
and nbd is deleted in this async way.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/ublk_drv.c | 9 ++++++---
include/uapi/linux/ublk_cmd.h | 2 ++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 06d88d2008ba..bea3d5cf8a83 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2468,7 +2468,7 @@ static inline bool ublk_idr_freed(int id)
return ptr == NULL;
}
-static int ublk_ctrl_del_dev(struct ublk_device **p_ub)
+static int ublk_ctrl_del_dev(struct ublk_device **p_ub, bool wait)
{
struct ublk_device *ub = *p_ub;
int idx = ub->ub_number;
@@ -2502,7 +2502,7 @@ static int ublk_ctrl_del_dev(struct ublk_device **p_ub)
* - the device number is freed already, we will not find this
* device via ublk_get_device_from_id()
*/
- if (wait_event_interruptible(ublk_idr_wq, ublk_idr_freed(idx)))
+ if (wait && wait_event_interruptible(ublk_idr_wq, ublk_idr_freed(idx)))
return -EINTR;
return 0;
}
@@ -2901,7 +2901,10 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
ret = ublk_ctrl_add_dev(cmd);
break;
case UBLK_CMD_DEL_DEV:
- ret = ublk_ctrl_del_dev(&ub);
+ ret = ublk_ctrl_del_dev(&ub, true);
+ break;
+ case UBLK_U_CMD_DEL_DEV_ASYNC:
+ ret = ublk_ctrl_del_dev(&ub, false);
break;
case UBLK_CMD_GET_QUEUE_AFFINITY:
ret = ublk_ctrl_get_queue_affinity(ub, cmd);
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index b9cfc5c96268..c8dc5f8ea699 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -49,6 +49,8 @@
_IOR('u', UBLK_CMD_GET_DEV_INFO2, struct ublksrv_ctrl_cmd)
#define UBLK_U_CMD_GET_FEATURES \
_IOR('u', 0x13, struct ublksrv_ctrl_cmd)
+#define UBLK_U_CMD_DEL_DEV_ASYNC \
+ _IOR('u', 0x14, struct ublksrv_ctrl_cmd)
/*
* 64bits are enough now, and it should be easy to extend in case of
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] ublk: improve ublk device deletion
2024-02-23 7:55 [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
2024-02-23 7:55 ` [PATCH 1/2] ublk: improve getting & putting ublk device Ming Lei
2024-02-23 7:55 ` [PATCH 2/2] ublk: add UBLK_CMD_DEL_DEV_ASYNC Ming Lei
@ 2024-02-29 1:27 ` Ming Lei
2024-02-29 1:35 ` Jens Axboe
2024-02-29 1:47 ` Jens Axboe
3 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2024-02-29 1:27 UTC (permalink / raw)
To: Jens Axboe, linux-block
On Fri, Feb 23, 2024 at 03:55:37PM +0800, Ming Lei wrote:
> Hello,
>
> The 1st patch cleans up get/put device, and annotate them
> via noline for trace purpose.
>
> The 2nd patch adds UBLK_U_CMD_DEL_DEV_ASYNC so userspace device
> deletion can be implemented easier.
Hello Jens,
All APIs in lib/ are built as notrace, so the 1st patch adds
noline for ublk get/put device(slow path), and it is helpful for
investigating reference issue by existed trace utilities.
The 2nd one makes userspace happy to not consider complicated dependency
issue in removing code path.
Can you queue the two patches for 6.9 if you are fine?
Thanks,
Ming
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] ublk: improve ublk device deletion
2024-02-29 1:27 ` [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
@ 2024-02-29 1:35 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2024-02-29 1:35 UTC (permalink / raw)
To: Ming Lei, linux-block
On 2/28/24 6:27 PM, Ming Lei wrote:
> On Fri, Feb 23, 2024 at 03:55:37PM +0800, Ming Lei wrote:
>> Hello,
>>
>> The 1st patch cleans up get/put device, and annotate them
>> via noline for trace purpose.
>>
>> The 2nd patch adds UBLK_U_CMD_DEL_DEV_ASYNC so userspace device
>> deletion can be implemented easier.
>
> Hello Jens,
>
>
> All APIs in lib/ are built as notrace, so the 1st patch adds
> noline for ublk get/put device(slow path), and it is helpful for
> investigating reference issue by existed trace utilities.
>
> The 2nd one makes userspace happy to not consider complicated dependency
> issue in removing code path.
>
> Can you queue the two patches for 6.9 if you are fine?
Yep they look fine to me, will get them queued up. Thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] ublk: improve ublk device deletion
2024-02-23 7:55 [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
` (2 preceding siblings ...)
2024-02-29 1:27 ` [PATCH 0/2] ublk: improve ublk device deletion Ming Lei
@ 2024-02-29 1:47 ` Jens Axboe
3 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2024-02-29 1:47 UTC (permalink / raw)
To: linux-block, Ming Lei
On Fri, 23 Feb 2024 15:55:37 +0800, Ming Lei wrote:
> The 1st patch cleans up get/put device, and annotate them
> via noline for trace purpose.
>
> The 2nd patch adds UBLK_U_CMD_DEL_DEV_ASYNC so userspace device
> deletion can be implemented easier.
>
> Ming Lei (2):
> ublk: improve getting & putting ublk device
> ublk: add UBLK_CMD_DEL_DEV_ASYNC
>
> [...]
Applied, thanks!
[1/2] ublk: improve getting & putting ublk device
commit: 1221b9e982e181f1c37789c46fe5bfe32d97bec4
[2/2] ublk: add UBLK_CMD_DEL_DEV_ASYNC
commit: 13fe8e6825e44129b6cbeee41d3012554bf8d687
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread