public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] ublk: introduce UBLK_CMD_TRY_STOP_DEV
@ 2026-01-06 20:33 Yoav Cohen
  2026-01-06 20:33 ` [PATCH v4 1/2] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
  2026-01-06 20:33 ` [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
  0 siblings, 2 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-06 20:33 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander
  Cc: jholzman, omril, Yoav Cohen, Yoav Cohen

Hello,

This patch series introduces a new command for ublk device management.

The first patch changes `ublk_ctrl_stop_dev()` to return void, since it
always returned 0. This simplifies the API.

The second patch introduces `UBLK_CMD_TRY_STOP_DEV`, which stops the
device only if there are no active openers. Unlike the existing stop
command (`UBLK_CMD_STOP_DEV`), this command avoids disrupting active
users by returning -EBUSY if the device is busy.

These patches only introduce the new command and API simplification
without altering existing behavior for active users

Changes since v2:
 - Address Ming Lei’s comments in patch 2:
   - Add a feature flag and some minor comments.
 - Patch 1 unchanged, keeps Reviewed-by

Changes since v3:
 - Address Caleb's comments in patch 2
   - add to kublk.c
   - add to auto features
   - set feature flag to 17

Yoav Cohen (2):
  ublk: make ublk_ctrl_stop_dev return void
  ublk: add UBLK_CMD_TRY_STOP_DEV command

 drivers/block/ublk_drv.c             | 50 +++++++++++++++++++++++++---
 include/uapi/linux/ublk_cmd.h        |  9 ++++-
 tools/testing/selftests/ublk/kublk.c |  1 +
 3 files changed, 54 insertions(+), 6 deletions(-)

-- 
2.39.5 (Apple Git-154)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] ublk: make ublk_ctrl_stop_dev return void
  2026-01-06 20:33 [PATCH v4 0/2] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
@ 2026-01-06 20:33 ` Yoav Cohen
  2026-01-06 20:33 ` [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
  1 sibling, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-06 20:33 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander
  Cc: jholzman, omril, Yoav Cohen, Yoav Cohen

This function always returns 0, so there is no need to return a value.

Signed-off-by: Yoav Cohen <yoav@nvidia.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 837fedb02e0d..2d5602ef05cc 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -3304,10 +3304,9 @@ static inline void ublk_ctrl_cmd_dump(struct io_uring_cmd *cmd)
 			header->data[0], header->addr, header->len);
 }
 
-static int ublk_ctrl_stop_dev(struct ublk_device *ub)
+static void ublk_ctrl_stop_dev(struct ublk_device *ub)
 {
 	ublk_stop_dev(ub);
-	return 0;
 }
 
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
@@ -3780,7 +3779,8 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
 		ret = ublk_ctrl_start_dev(ub, header);
 		break;
 	case UBLK_CMD_STOP_DEV:
-		ret = ublk_ctrl_stop_dev(ub);
+		ublk_ctrl_stop_dev(ub);
+		ret = 0;
 		break;
 	case UBLK_CMD_GET_DEV_INFO:
 	case UBLK_CMD_GET_DEV_INFO2:
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-06 20:33 [PATCH v4 0/2] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
  2026-01-06 20:33 ` [PATCH v4 1/2] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
@ 2026-01-06 20:33 ` Yoav Cohen
  2026-01-07 13:20   ` Ming Lei
       [not found]   ` <922b4c45-14ba-497b-8a00-eac2903a2854@zkern.zazolabs.com>
  1 sibling, 2 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-06 20:33 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander
  Cc: jholzman, omril, Yoav Cohen, Yoav Cohen

This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
device if there are no active openers for the ublk block device.
If the device is busy, the command returns -EBUSY instead of
disrupting active clients. This allows safe, non-destructive stopping.

Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
feature flag.

Signed-off-by: Yoav Cohen <yoav@nvidia.com>
---
 drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
 include/uapi/linux/ublk_cmd.h        |  9 +++++-
 tools/testing/selftests/ublk/kublk.c |  1 +
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2d5602ef05cc..fc8b87902f8f 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -54,6 +54,7 @@
 #define UBLK_CMD_DEL_DEV_ASYNC	_IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
 #define UBLK_CMD_UPDATE_SIZE	_IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
 #define UBLK_CMD_QUIESCE_DEV	_IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
+#define UBLK_CMD_TRY_STOP_DEV	_IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
 
 #define UBLK_IO_REGISTER_IO_BUF		_IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
 #define UBLK_IO_UNREGISTER_IO_BUF	_IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
@@ -73,7 +74,8 @@
 		| UBLK_F_AUTO_BUF_REG \
 		| UBLK_F_QUIESCE \
 		| UBLK_F_PER_IO_DAEMON \
-		| UBLK_F_BUF_REG_OFF_DAEMON)
+		| UBLK_F_BUF_REG_OFF_DAEMON \
+		| UBLK_F_SAFE_STOP_DEV)
 
 #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
 		| UBLK_F_USER_RECOVERY_REISSUE \
@@ -239,6 +241,8 @@ struct ublk_device {
 	struct delayed_work	exit_work;
 	struct work_struct	partition_scan_work;
 
+	bool			block_open; /* protected by open_mutex */
+
 	struct ublk_queue       *queues[];
 };
 
@@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
 			return -EPERM;
 	}
 
+	if (ub->block_open)
+		return -EBUSY;
+
 	return 0;
 }
 
@@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
 	ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
 		UBLK_F_URING_CMD_COMP_IN_TASK |
 		UBLK_F_PER_IO_DAEMON |
-		UBLK_F_BUF_REG_OFF_DAEMON;
+		UBLK_F_BUF_REG_OFF_DAEMON |
+		UBLK_F_SAFE_STOP_DEV;
 
 	/* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
 	if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
@@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
 	ublk_stop_dev(ub);
 }
 
+static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
+{
+	struct gendisk *disk;
+	int ret = 0;
+
+	disk = ublk_get_disk(ub);
+	if (!disk)
+		return -ENODEV;
+
+	mutex_lock(&disk->open_mutex);
+	if (disk_openers(disk) > 0) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+	ub->block_open = true;
+	/* release open_mutex as del_gendisk() will reacquire it */
+	mutex_unlock(&disk->open_mutex);
+
+	ublk_ctrl_stop_dev(ub);
+	goto out;
+
+unlock:
+	mutex_unlock(&disk->open_mutex);
+out:
+	ublk_put_disk(disk);
+	return ret;
+}
+
 static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
 		const struct ublksrv_ctrl_cmd *header)
 {
@@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
 	case UBLK_CMD_END_USER_RECOVERY:
 	case UBLK_CMD_UPDATE_SIZE:
 	case UBLK_CMD_QUIESCE_DEV:
+	case UBLK_CMD_TRY_STOP_DEV:
 		mask = MAY_READ | MAY_WRITE;
 		break;
 	default:
@@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
 	case UBLK_CMD_QUIESCE_DEV:
 		ret = ublk_ctrl_quiesce_dev(ub, header);
 		break;
+	case UBLK_CMD_TRY_STOP_DEV:
+		ret = ublk_ctrl_try_stop_dev(ub);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index ec77dabba45b..9daa8ab372f0 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -55,7 +55,8 @@
 	_IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
 #define UBLK_U_CMD_QUIESCE_DEV		\
 	_IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
-
+#define UBLK_U_CMD_TRY_STOP_DEV		\
+	_IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
 /*
  * 64bits are enough now, and it should be easy to extend in case of
  * running out of feature flags
@@ -311,6 +312,12 @@
  */
 #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
 
+/*
+ * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
+ * allows stopping the device only if there are no openers.
+ */
+#define UBLK_F_SAFE_STOP_DEV	(1ULL << 17)
+
 /* device state */
 #define UBLK_S_DEV_DEAD	0
 #define UBLK_S_DEV_LIVE	1
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 185ba553686a..6739e28c4059 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
 		FEAT_NAME(UBLK_F_QUIESCE),
 		FEAT_NAME(UBLK_F_PER_IO_DAEMON),
 		FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
+		FEAT_NAME(UBLK_F_SAFE_STOP_DEV),
 	};
 	struct ublk_dev *dev;
 	__u64 features = 0;
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-06 20:33 ` [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
@ 2026-01-07 13:20   ` Ming Lei
  2026-01-07 13:34     ` Yoav Cohen
       [not found]   ` <922b4c45-14ba-497b-8a00-eac2903a2854@zkern.zazolabs.com>
  1 sibling, 1 reply; 8+ messages in thread
From: Ming Lei @ 2026-01-07 13:20 UTC (permalink / raw)
  To: Yoav Cohen; +Cc: Jens Axboe, linux-block, csander, jholzman, omril

[-- Attachment #1: Type: text/plain, Size: 5559 bytes --]

On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
> device if there are no active openers for the ublk block device.
> If the device is busy, the command returns -EBUSY instead of
> disrupting active clients. This allows safe, non-destructive stopping.
> 
> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
> feature flag.
> 
> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> ---
>  drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>  include/uapi/linux/ublk_cmd.h        |  9 +++++-
>  tools/testing/selftests/ublk/kublk.c |  1 +
>  3 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 2d5602ef05cc..fc8b87902f8f 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -54,6 +54,7 @@
>  #define UBLK_CMD_DEL_DEV_ASYNC	_IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
>  #define UBLK_CMD_UPDATE_SIZE	_IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
>  #define UBLK_CMD_QUIESCE_DEV	_IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
> +#define UBLK_CMD_TRY_STOP_DEV	_IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
>  
>  #define UBLK_IO_REGISTER_IO_BUF		_IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
>  #define UBLK_IO_UNREGISTER_IO_BUF	_IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
> @@ -73,7 +74,8 @@
>  		| UBLK_F_AUTO_BUF_REG \
>  		| UBLK_F_QUIESCE \
>  		| UBLK_F_PER_IO_DAEMON \
> -		| UBLK_F_BUF_REG_OFF_DAEMON)
> +		| UBLK_F_BUF_REG_OFF_DAEMON \
> +		| UBLK_F_SAFE_STOP_DEV)
>  
>  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>  		| UBLK_F_USER_RECOVERY_REISSUE \
> @@ -239,6 +241,8 @@ struct ublk_device {
>  	struct delayed_work	exit_work;
>  	struct work_struct	partition_scan_work;
>  
> +	bool			block_open; /* protected by open_mutex */
> +
>  	struct ublk_queue       *queues[];
>  };
>  
> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
>  			return -EPERM;
>  	}
>  
> +	if (ub->block_open)
> +		return -EBUSY;
> +
>  	return 0;
>  }
>  
> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>  	ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
>  		UBLK_F_URING_CMD_COMP_IN_TASK |
>  		UBLK_F_PER_IO_DAEMON |
> -		UBLK_F_BUF_REG_OFF_DAEMON;
> +		UBLK_F_BUF_REG_OFF_DAEMON |
> +		UBLK_F_SAFE_STOP_DEV;
>  
>  	/* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
>  	if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
>  	ublk_stop_dev(ub);
>  }
>  
> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
> +{
> +	struct gendisk *disk;
> +	int ret = 0;
> +
> +	disk = ublk_get_disk(ub);
> +	if (!disk)
> +		return -ENODEV;
> +
> +	mutex_lock(&disk->open_mutex);
> +	if (disk_openers(disk) > 0) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +	ub->block_open = true;
> +	/* release open_mutex as del_gendisk() will reacquire it */
> +	mutex_unlock(&disk->open_mutex);
> +
> +	ublk_ctrl_stop_dev(ub);
> +	goto out;
> +
> +unlock:
> +	mutex_unlock(&disk->open_mutex);
> +out:
> +	ublk_put_disk(disk);
> +	return ret;
> +}
> +
>  static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
>  		const struct ublksrv_ctrl_cmd *header)
>  {
> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
>  	case UBLK_CMD_END_USER_RECOVERY:
>  	case UBLK_CMD_UPDATE_SIZE:
>  	case UBLK_CMD_QUIESCE_DEV:
> +	case UBLK_CMD_TRY_STOP_DEV:
>  		mask = MAY_READ | MAY_WRITE;
>  		break;
>  	default:
> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
>  	case UBLK_CMD_QUIESCE_DEV:
>  		ret = ublk_ctrl_quiesce_dev(ub, header);
>  		break;
> +	case UBLK_CMD_TRY_STOP_DEV:
> +		ret = ublk_ctrl_try_stop_dev(ub);
> +		break;
>  	default:
>  		ret = -EOPNOTSUPP;
>  		break;
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index ec77dabba45b..9daa8ab372f0 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -55,7 +55,8 @@
>  	_IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
>  #define UBLK_U_CMD_QUIESCE_DEV		\
>  	_IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
> -
> +#define UBLK_U_CMD_TRY_STOP_DEV		\
> +	_IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
>  /*
>   * 64bits are enough now, and it should be easy to extend in case of
>   * running out of feature flags
> @@ -311,6 +312,12 @@
>   */
>  #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
>  
> +/*
> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
> + * allows stopping the device only if there are no openers.
> + */
> +#define UBLK_F_SAFE_STOP_DEV	(1ULL << 17)
> +
>  /* device state */
>  #define UBLK_S_DEV_DEAD	0
>  #define UBLK_S_DEV_LIVE	1
> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
> index 185ba553686a..6739e28c4059 100644
> --- a/tools/testing/selftests/ublk/kublk.c
> +++ b/tools/testing/selftests/ublk/kublk.c
> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
>  		FEAT_NAME(UBLK_F_QUIESCE),
>  		FEAT_NAME(UBLK_F_PER_IO_DAEMON),
>  		FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
> +		FEAT_NAME(UBLK_F_SAFE_STOP_DEV),

We usually split driver and test code into different patches, and the patch
looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

But we usually separate test code from driver change, also new feature needs
selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
the attached selftest patch and post all 3 in v5?



Thanks,
Ming

[-- Attachment #2: 0001-selftests-ublk-add-stop-command-with-safe-option.patch --]
[-- Type: text/plain, Size: 6230 bytes --]

From 3574f73d3098dc56f63b7c2b1097c0f090c4ad3c Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Wed, 7 Jan 2026 13:01:39 +0000
Subject: [PATCH] selftests: ublk: add stop command with --safe option

Add 'stop' subcommand to kublk utility that uses the new
UBLK_CMD_TRY_STOP_DEV command when --safe option is specified.
This allows stopping a device only if it has no active openers,
returning -EBUSY otherwise.

Also add test_generic_16.sh to test the new functionality.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 tools/testing/selftests/ublk/Makefile         |  1 +
 tools/testing/selftests/ublk/kublk.c          | 52 +++++++++++++++++
 tools/testing/selftests/ublk/kublk.h          |  1 +
 .../testing/selftests/ublk/test_generic_16.sh | 57 +++++++++++++++++++
 4 files changed, 111 insertions(+)
 create mode 100755 tools/testing/selftests/ublk/test_generic_16.sh

diff --git a/tools/testing/selftests/ublk/Makefile b/tools/testing/selftests/ublk/Makefile
index 06ba6fde098d..e9614f77b809 100644
--- a/tools/testing/selftests/ublk/Makefile
+++ b/tools/testing/selftests/ublk/Makefile
@@ -23,6 +23,7 @@ TEST_PROGS += test_generic_12.sh
 TEST_PROGS += test_generic_13.sh
 TEST_PROGS += test_generic_14.sh
 TEST_PROGS += test_generic_15.sh
+TEST_PROGS += test_generic_16.sh
 
 TEST_PROGS += test_null_01.sh
 TEST_PROGS += test_null_02.sh
diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 6739e28c4059..1a563705caa0 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -107,6 +107,15 @@ static int ublk_ctrl_stop_dev(struct ublk_dev *dev)
 	return __ublk_ctrl_cmd(dev, &data);
 }
 
+static int ublk_ctrl_try_stop_dev(struct ublk_dev *dev)
+{
+	struct ublk_ctrl_cmd_data data = {
+		.cmd_op	= UBLK_U_CMD_TRY_STOP_DEV,
+	};
+
+	return __ublk_ctrl_cmd(dev, &data);
+}
+
 static int ublk_ctrl_start_dev(struct ublk_dev *dev,
 		int daemon_pid)
 {
@@ -1392,6 +1401,42 @@ static int cmd_dev_del(struct dev_ctx *ctx)
 	return 0;
 }
 
+static int cmd_dev_stop(struct dev_ctx *ctx)
+{
+	int number = ctx->dev_id;
+	struct ublk_dev *dev;
+	int ret;
+
+	if (number < 0) {
+		ublk_err("%s: device id is required\n", __func__);
+		return -EINVAL;
+	}
+
+	dev = ublk_ctrl_init();
+	dev->dev_info.dev_id = number;
+
+	ret = ublk_ctrl_get_info(dev);
+	if (ret < 0)
+		goto fail;
+
+	if (ctx->safe_stop) {
+		ret = ublk_ctrl_try_stop_dev(dev);
+		if (ret < 0)
+			ublk_err("%s: try_stop dev %d failed ret %d\n",
+					__func__, number, ret);
+	} else {
+		ret = ublk_ctrl_stop_dev(dev);
+		if (ret < 0)
+			ublk_err("%s: stop dev %d failed ret %d\n",
+					__func__, number, ret);
+	}
+
+fail:
+	ublk_ctrl_deinit(dev);
+
+	return ret;
+}
+
 static int __cmd_dev_list(struct dev_ctx *ctx)
 {
 	struct ublk_dev *dev = ublk_ctrl_init();
@@ -1582,6 +1627,8 @@ static int cmd_dev_help(char *exe)
 
 	printf("%s del [-n dev_id] -a \n", exe);
 	printf("\t -a delete all devices -n delete specified device\n\n");
+	printf("%s stop -n dev_id [--safe]\n", exe);
+	printf("\t --safe only stop if device has no active openers\n\n");
 	printf("%s list [-n dev_id] -a \n", exe);
 	printf("\t -a list all devices, -n list specified device, default -a \n\n");
 	printf("%s features\n", exe);
@@ -1613,6 +1660,7 @@ int main(int argc, char *argv[])
 		{ "nthreads",		1,	NULL,  0 },
 		{ "per_io_tasks",	0,	NULL,  0 },
 		{ "no_ublk_fixed_fd",	0,	NULL,  0 },
+		{ "safe",		0,	NULL,  0 },
 		{ 0, 0, 0, 0 }
 	};
 	const struct ublk_tgt_ops *ops = NULL;
@@ -1697,6 +1745,8 @@ int main(int argc, char *argv[])
 				ctx.per_io_tasks = 1;
 			if (!strcmp(longopts[option_idx].name, "no_ublk_fixed_fd"))
 				ctx.no_ublk_fixed_fd = 1;
+			if (!strcmp(longopts[option_idx].name, "safe"))
+				ctx.safe_stop = 1;
 			break;
 		case '?':
 			/*
@@ -1764,6 +1814,8 @@ int main(int argc, char *argv[])
 		}
 	} else if (!strcmp(cmd, "del"))
 		ret = cmd_dev_del(&ctx);
+	else if (!strcmp(cmd, "stop"))
+		ret = cmd_dev_stop(&ctx);
 	else if (!strcmp(cmd, "list")) {
 		ctx.all = 1;
 		ret = cmd_dev_list(&ctx);
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 8a83b90ec603..4b6b4992f78e 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -78,6 +78,7 @@ struct dev_ctx {
 	unsigned int	auto_zc_fallback:1;
 	unsigned int	per_io_tasks:1;
 	unsigned int	no_ublk_fixed_fd:1;
+	unsigned int	safe_stop:1;
 
 	int _evtfd;
 	int _shmid;
diff --git a/tools/testing/selftests/ublk/test_generic_16.sh b/tools/testing/selftests/ublk/test_generic_16.sh
new file mode 100755
index 000000000000..e08af7b685c9
--- /dev/null
+++ b/tools/testing/selftests/ublk/test_generic_16.sh
@@ -0,0 +1,57 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+. "$(cd "$(dirname "$0")" && pwd)"/test_common.sh
+
+TID="generic_16"
+ERR_CODE=0
+
+_prep_test "null" "stop --safe command"
+
+# Check if SAFE_STOP_DEV feature is supported
+if ! _have_feature "SAFE_STOP_DEV"; then
+	_cleanup_test "null"
+	exit "$UBLK_SKIP_CODE"
+fi
+
+# Test 1: stop --safe on idle device should succeed
+dev_id=$(_add_ublk_dev -t null -q 2 -d 32)
+_check_add_dev $TID $?
+
+# Device is idle (no openers), stop --safe should succeed
+if ! ${UBLK_PROG} stop -n "${dev_id}" --safe; then
+	echo "stop --safe on idle device failed unexpectedly!"
+	ERR_CODE=255
+fi
+
+# Clean up device
+${UBLK_PROG} del -n "${dev_id}" > /dev/null 2>&1
+udevadm settle
+
+# Test 2: stop --safe on device with active opener should fail
+dev_id=$(_add_ublk_dev -t null -q 2 -d 32)
+_check_add_dev $TID $?
+
+# Open device in background (dd reads indefinitely)
+dd if=/dev/ublkb${dev_id} of=/dev/null bs=4k iflag=direct > /dev/null 2>&1 &
+dd_pid=$!
+
+# Give dd time to start
+sleep 0.2
+
+# Device has active opener, stop --safe should fail with -EBUSY
+if ${UBLK_PROG} stop -n "${dev_id}" --safe 2>/dev/null; then
+	echo "stop --safe on busy device succeeded unexpectedly!"
+	ERR_CODE=255
+fi
+
+# Kill dd and clean up
+kill $dd_pid 2>/dev/null
+wait $dd_pid 2>/dev/null
+
+# Now device should be idle, regular delete should work
+${UBLK_PROG} del -n "${dev_id}"
+udevadm settle
+
+_cleanup_test "null"
+_show_result $TID $ERR_CODE
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-07 13:20   ` Ming Lei
@ 2026-01-07 13:34     ` Yoav Cohen
  2026-01-07 14:34       ` Ming Lei
  0 siblings, 1 reply; 8+ messages in thread
From: Yoav Cohen @ 2026-01-07 13:34 UTC (permalink / raw)
  To: Ming Lei
  Cc: Jens Axboe, linux-block@vger.kernel.org, csander@purestorage.com,
	Jared Holzman, Omri Levi

On 07/01/2026 15:20, Ming Lei wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
>> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
>> device if there are no active openers for the ublk block device.
>> If the device is busy, the command returns -EBUSY instead of
>> disrupting active clients. This allows safe, non-destructive stopping.
>>
>> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
>> feature flag.
>>
>> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
>> ---
>>   drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>>   include/uapi/linux/ublk_cmd.h        |  9 +++++-
>>   tools/testing/selftests/ublk/kublk.c |  1 +
>>   3 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 2d5602ef05cc..fc8b87902f8f 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -54,6 +54,7 @@
>>   #define UBLK_CMD_DEL_DEV_ASYNC       _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
>>   #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
>>   #define UBLK_CMD_QUIESCE_DEV _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
>> +#define UBLK_CMD_TRY_STOP_DEV        _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
>>
>>   #define UBLK_IO_REGISTER_IO_BUF              _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
>>   #define UBLK_IO_UNREGISTER_IO_BUF    _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
>> @@ -73,7 +74,8 @@
>>                | UBLK_F_AUTO_BUF_REG \
>>                | UBLK_F_QUIESCE \
>>                | UBLK_F_PER_IO_DAEMON \
>> -             | UBLK_F_BUF_REG_OFF_DAEMON)
>> +             | UBLK_F_BUF_REG_OFF_DAEMON \
>> +             | UBLK_F_SAFE_STOP_DEV)
>>
>>   #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>>                | UBLK_F_USER_RECOVERY_REISSUE \
>> @@ -239,6 +241,8 @@ struct ublk_device {
>>        struct delayed_work     exit_work;
>>        struct work_struct      partition_scan_work;
>>
>> +     bool                    block_open; /* protected by open_mutex */
>> +
>>        struct ublk_queue       *queues[];
>>   };
>>
>> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
>>                        return -EPERM;
>>        }
>>
>> +     if (ub->block_open)
>> +             return -EBUSY;
>> +
>>        return 0;
>>   }
>>
>> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>>        ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
>>                UBLK_F_URING_CMD_COMP_IN_TASK |
>>                UBLK_F_PER_IO_DAEMON |
>> -             UBLK_F_BUF_REG_OFF_DAEMON;
>> +             UBLK_F_BUF_REG_OFF_DAEMON |
>> +             UBLK_F_SAFE_STOP_DEV;
>>
>>        /* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
>>        if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
>> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
>>        ublk_stop_dev(ub);
>>   }
>>
>> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
>> +{
>> +     struct gendisk *disk;
>> +     int ret = 0;
>> +
>> +     disk = ublk_get_disk(ub);
>> +     if (!disk)
>> +             return -ENODEV;
>> +
>> +     mutex_lock(&disk->open_mutex);
>> +     if (disk_openers(disk) > 0) {
>> +             ret = -EBUSY;
>> +             goto unlock;
>> +     }
>> +     ub->block_open = true;
>> +     /* release open_mutex as del_gendisk() will reacquire it */
>> +     mutex_unlock(&disk->open_mutex);
>> +
>> +     ublk_ctrl_stop_dev(ub);
>> +     goto out;
>> +
>> +unlock:
>> +     mutex_unlock(&disk->open_mutex);
>> +out:
>> +     ublk_put_disk(disk);
>> +     return ret;
>> +}
>> +
>>   static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
>>                const struct ublksrv_ctrl_cmd *header)
>>   {
>> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
>>        case UBLK_CMD_END_USER_RECOVERY:
>>        case UBLK_CMD_UPDATE_SIZE:
>>        case UBLK_CMD_QUIESCE_DEV:
>> +     case UBLK_CMD_TRY_STOP_DEV:
>>                mask = MAY_READ | MAY_WRITE;
>>                break;
>>        default:
>> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
>>        case UBLK_CMD_QUIESCE_DEV:
>>                ret = ublk_ctrl_quiesce_dev(ub, header);
>>                break;
>> +     case UBLK_CMD_TRY_STOP_DEV:
>> +             ret = ublk_ctrl_try_stop_dev(ub);
>> +             break;
>>        default:
>>                ret = -EOPNOTSUPP;
>>                break;
>> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
>> index ec77dabba45b..9daa8ab372f0 100644
>> --- a/include/uapi/linux/ublk_cmd.h
>> +++ b/include/uapi/linux/ublk_cmd.h
>> @@ -55,7 +55,8 @@
>>        _IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
>>   #define UBLK_U_CMD_QUIESCE_DEV               \
>>        _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
>> -
>> +#define UBLK_U_CMD_TRY_STOP_DEV              \
>> +     _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
>>   /*
>>    * 64bits are enough now, and it should be easy to extend in case of
>>    * running out of feature flags
>> @@ -311,6 +312,12 @@
>>    */
>>   #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
>>
>> +/*
>> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
>> + * allows stopping the device only if there are no openers.
>> + */
>> +#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>> +
>>   /* device state */
>>   #define UBLK_S_DEV_DEAD      0
>>   #define UBLK_S_DEV_LIVE      1
>> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
>> index 185ba553686a..6739e28c4059 100644
>> --- a/tools/testing/selftests/ublk/kublk.c
>> +++ b/tools/testing/selftests/ublk/kublk.c
>> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
>>                FEAT_NAME(UBLK_F_QUIESCE),
>>                FEAT_NAME(UBLK_F_PER_IO_DAEMON),
>>                FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
>> +             FEAT_NAME(UBLK_F_SAFE_STOP_DEV),
> 
> We usually split driver and test code into different patches, and the patch
> looks fine:
> 
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> 
> But we usually separate test code from driver change, also new feature needs
> selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
> the attached selftest patch and post all 3 in v5?
> 
> 
> 
> Thanks,
> Ming

Hi,

Thank you Ming, just to be sure:
I'll split the kublk change to a 3rd patch - that's ok.
But do you also want me to write a selftest for this feature as part of 
the 3rd patch?

Thank you

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-07 13:34     ` Yoav Cohen
@ 2026-01-07 14:34       ` Ming Lei
  2026-01-07 21:04         ` Yoav Cohen
  0 siblings, 1 reply; 8+ messages in thread
From: Ming Lei @ 2026-01-07 14:34 UTC (permalink / raw)
  To: Yoav Cohen
  Cc: Jens Axboe, linux-block@vger.kernel.org, csander@purestorage.com,
	Jared Holzman, Omri Levi

On Wed, Jan 07, 2026 at 01:34:49PM +0000, Yoav Cohen wrote:
> On 07/01/2026 15:20, Ming Lei wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
> >> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
> >> device if there are no active openers for the ublk block device.
> >> If the device is busy, the command returns -EBUSY instead of
> >> disrupting active clients. This allows safe, non-destructive stopping.
> >>
> >> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
> >> feature flag.
> >>
> >> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
> >> ---
> >>   drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
> >>   include/uapi/linux/ublk_cmd.h        |  9 +++++-
> >>   tools/testing/selftests/ublk/kublk.c |  1 +
> >>   3 files changed, 51 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> >> index 2d5602ef05cc..fc8b87902f8f 100644
> >> --- a/drivers/block/ublk_drv.c
> >> +++ b/drivers/block/ublk_drv.c
> >> @@ -54,6 +54,7 @@
> >>   #define UBLK_CMD_DEL_DEV_ASYNC       _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
> >>   #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> >>   #define UBLK_CMD_QUIESCE_DEV _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
> >> +#define UBLK_CMD_TRY_STOP_DEV        _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
> >>
> >>   #define UBLK_IO_REGISTER_IO_BUF              _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
> >>   #define UBLK_IO_UNREGISTER_IO_BUF    _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
> >> @@ -73,7 +74,8 @@
> >>                | UBLK_F_AUTO_BUF_REG \
> >>                | UBLK_F_QUIESCE \
> >>                | UBLK_F_PER_IO_DAEMON \
> >> -             | UBLK_F_BUF_REG_OFF_DAEMON)
> >> +             | UBLK_F_BUF_REG_OFF_DAEMON \
> >> +             | UBLK_F_SAFE_STOP_DEV)
> >>
> >>   #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> >>                | UBLK_F_USER_RECOVERY_REISSUE \
> >> @@ -239,6 +241,8 @@ struct ublk_device {
> >>        struct delayed_work     exit_work;
> >>        struct work_struct      partition_scan_work;
> >>
> >> +     bool                    block_open; /* protected by open_mutex */
> >> +
> >>        struct ublk_queue       *queues[];
> >>   };
> >>
> >> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
> >>                        return -EPERM;
> >>        }
> >>
> >> +     if (ub->block_open)
> >> +             return -EBUSY;
> >> +
> >>        return 0;
> >>   }
> >>
> >> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
> >>        ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
> >>                UBLK_F_URING_CMD_COMP_IN_TASK |
> >>                UBLK_F_PER_IO_DAEMON |
> >> -             UBLK_F_BUF_REG_OFF_DAEMON;
> >> +             UBLK_F_BUF_REG_OFF_DAEMON |
> >> +             UBLK_F_SAFE_STOP_DEV;
> >>
> >>        /* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
> >>        if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
> >> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
> >>        ublk_stop_dev(ub);
> >>   }
> >>
> >> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
> >> +{
> >> +     struct gendisk *disk;
> >> +     int ret = 0;
> >> +
> >> +     disk = ublk_get_disk(ub);
> >> +     if (!disk)
> >> +             return -ENODEV;
> >> +
> >> +     mutex_lock(&disk->open_mutex);
> >> +     if (disk_openers(disk) > 0) {
> >> +             ret = -EBUSY;
> >> +             goto unlock;
> >> +     }
> >> +     ub->block_open = true;
> >> +     /* release open_mutex as del_gendisk() will reacquire it */
> >> +     mutex_unlock(&disk->open_mutex);
> >> +
> >> +     ublk_ctrl_stop_dev(ub);
> >> +     goto out;
> >> +
> >> +unlock:
> >> +     mutex_unlock(&disk->open_mutex);
> >> +out:
> >> +     ublk_put_disk(disk);
> >> +     return ret;
> >> +}
> >> +
> >>   static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
> >>                const struct ublksrv_ctrl_cmd *header)
> >>   {
> >> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
> >>        case UBLK_CMD_END_USER_RECOVERY:
> >>        case UBLK_CMD_UPDATE_SIZE:
> >>        case UBLK_CMD_QUIESCE_DEV:
> >> +     case UBLK_CMD_TRY_STOP_DEV:
> >>                mask = MAY_READ | MAY_WRITE;
> >>                break;
> >>        default:
> >> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
> >>        case UBLK_CMD_QUIESCE_DEV:
> >>                ret = ublk_ctrl_quiesce_dev(ub, header);
> >>                break;
> >> +     case UBLK_CMD_TRY_STOP_DEV:
> >> +             ret = ublk_ctrl_try_stop_dev(ub);
> >> +             break;
> >>        default:
> >>                ret = -EOPNOTSUPP;
> >>                break;
> >> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> >> index ec77dabba45b..9daa8ab372f0 100644
> >> --- a/include/uapi/linux/ublk_cmd.h
> >> +++ b/include/uapi/linux/ublk_cmd.h
> >> @@ -55,7 +55,8 @@
> >>        _IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
> >>   #define UBLK_U_CMD_QUIESCE_DEV               \
> >>        _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
> >> -
> >> +#define UBLK_U_CMD_TRY_STOP_DEV              \
> >> +     _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
> >>   /*
> >>    * 64bits are enough now, and it should be easy to extend in case of
> >>    * running out of feature flags
> >> @@ -311,6 +312,12 @@
> >>    */
> >>   #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
> >>
> >> +/*
> >> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
> >> + * allows stopping the device only if there are no openers.
> >> + */
> >> +#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
> >> +
> >>   /* device state */
> >>   #define UBLK_S_DEV_DEAD      0
> >>   #define UBLK_S_DEV_LIVE      1
> >> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
> >> index 185ba553686a..6739e28c4059 100644
> >> --- a/tools/testing/selftests/ublk/kublk.c
> >> +++ b/tools/testing/selftests/ublk/kublk.c
> >> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
> >>                FEAT_NAME(UBLK_F_QUIESCE),
> >>                FEAT_NAME(UBLK_F_PER_IO_DAEMON),
> >>                FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
> >> +             FEAT_NAME(UBLK_F_SAFE_STOP_DEV),
> > 
> > We usually split driver and test code into different patches, and the patch
> > looks fine:
> > 
> > Reviewed-by: Ming Lei <ming.lei@redhat.com>
> > 
> > But we usually separate test code from driver change, also new feature needs
> > selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
> > the attached selftest patch and post all 3 in v5?
> > 
> > 
> > 
> > Thanks,
> > Ming
> 
> Hi,
> 
> Thank you Ming, just to be sure:
> I'll split the kublk change to a 3rd patch - that's ok.
> But do you also want me to write a selftest for this feature as part of 
> the 3rd patch?

The 3rd patch has included one complete test case, but feel free to
fix/improve, or even rewrite it if you think it isn't good.


Thanks,
Ming


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-07 14:34       ` Ming Lei
@ 2026-01-07 21:04         ` Yoav Cohen
  0 siblings, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-07 21:04 UTC (permalink / raw)
  To: Ming Lei, linux-block@vger.kernel.org

On 07/01/2026 16:34, Ming Lei wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Wed, Jan 07, 2026 at 01:34:49PM +0000, Yoav Cohen wrote:
>> On 07/01/2026 15:20, Ming Lei wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
>>>> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
>>>> device if there are no active openers for the ublk block device.
>>>> If the device is busy, the command returns -EBUSY instead of
>>>> disrupting active clients. This allows safe, non-destructive stopping.
>>>>
>>>> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
>>>> feature flag.
>>>>
>>>> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
>>>> ---
>>>>    drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>>>>    include/uapi/linux/ublk_cmd.h        |  9 +++++-
>>>>    tools/testing/selftests/ublk/kublk.c |  1 +
>>>>    3 files changed, 51 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>>>> index 2d5602ef05cc..fc8b87902f8f 100644
>>>> --- a/drivers/block/ublk_drv.c
>>>> +++ b/drivers/block/ublk_drv.c
>>>> @@ -54,6 +54,7 @@
>>>>    #define UBLK_CMD_DEL_DEV_ASYNC       _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
>>>>    #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
>>>>    #define UBLK_CMD_QUIESCE_DEV _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
>>>> +#define UBLK_CMD_TRY_STOP_DEV        _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
>>>>
>>>>    #define UBLK_IO_REGISTER_IO_BUF              _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
>>>>    #define UBLK_IO_UNREGISTER_IO_BUF    _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
>>>> @@ -73,7 +74,8 @@
>>>>                 | UBLK_F_AUTO_BUF_REG \
>>>>                 | UBLK_F_QUIESCE \
>>>>                 | UBLK_F_PER_IO_DAEMON \
>>>> -             | UBLK_F_BUF_REG_OFF_DAEMON)
>>>> +             | UBLK_F_BUF_REG_OFF_DAEMON \
>>>> +             | UBLK_F_SAFE_STOP_DEV)
>>>>
>>>>    #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>>>>                 | UBLK_F_USER_RECOVERY_REISSUE \
>>>> @@ -239,6 +241,8 @@ struct ublk_device {
>>>>         struct delayed_work     exit_work;
>>>>         struct work_struct      partition_scan_work;
>>>>
>>>> +     bool                    block_open; /* protected by open_mutex */
>>>> +
>>>>         struct ublk_queue       *queues[];
>>>>    };
>>>>
>>>> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
>>>>                         return -EPERM;
>>>>         }
>>>>
>>>> +     if (ub->block_open)
>>>> +             return -EBUSY;
>>>> +
>>>>         return 0;
>>>>    }
>>>>
>>>> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>>>>         ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
>>>>                 UBLK_F_URING_CMD_COMP_IN_TASK |
>>>>                 UBLK_F_PER_IO_DAEMON |
>>>> -             UBLK_F_BUF_REG_OFF_DAEMON;
>>>> +             UBLK_F_BUF_REG_OFF_DAEMON |
>>>> +             UBLK_F_SAFE_STOP_DEV;
>>>>
>>>>         /* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
>>>>         if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
>>>> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
>>>>         ublk_stop_dev(ub);
>>>>    }
>>>>
>>>> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
>>>> +{
>>>> +     struct gendisk *disk;
>>>> +     int ret = 0;
>>>> +
>>>> +     disk = ublk_get_disk(ub);
>>>> +     if (!disk)
>>>> +             return -ENODEV;
>>>> +
>>>> +     mutex_lock(&disk->open_mutex);
>>>> +     if (disk_openers(disk) > 0) {
>>>> +             ret = -EBUSY;
>>>> +             goto unlock;
>>>> +     }
>>>> +     ub->block_open = true;
>>>> +     /* release open_mutex as del_gendisk() will reacquire it */
>>>> +     mutex_unlock(&disk->open_mutex);
>>>> +
>>>> +     ublk_ctrl_stop_dev(ub);
>>>> +     goto out;
>>>> +
>>>> +unlock:
>>>> +     mutex_unlock(&disk->open_mutex);
>>>> +out:
>>>> +     ublk_put_disk(disk);
>>>> +     return ret;
>>>> +}
>>>> +
>>>>    static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
>>>>                 const struct ublksrv_ctrl_cmd *header)
>>>>    {
>>>> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
>>>>         case UBLK_CMD_END_USER_RECOVERY:
>>>>         case UBLK_CMD_UPDATE_SIZE:
>>>>         case UBLK_CMD_QUIESCE_DEV:
>>>> +     case UBLK_CMD_TRY_STOP_DEV:
>>>>                 mask = MAY_READ | MAY_WRITE;
>>>>                 break;
>>>>         default:
>>>> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
>>>>         case UBLK_CMD_QUIESCE_DEV:
>>>>                 ret = ublk_ctrl_quiesce_dev(ub, header);
>>>>                 break;
>>>> +     case UBLK_CMD_TRY_STOP_DEV:
>>>> +             ret = ublk_ctrl_try_stop_dev(ub);
>>>> +             break;
>>>>         default:
>>>>                 ret = -EOPNOTSUPP;
>>>>                 break;
>>>> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
>>>> index ec77dabba45b..9daa8ab372f0 100644
>>>> --- a/include/uapi/linux/ublk_cmd.h
>>>> +++ b/include/uapi/linux/ublk_cmd.h
>>>> @@ -55,7 +55,8 @@
>>>>         _IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
>>>>    #define UBLK_U_CMD_QUIESCE_DEV               \
>>>>         _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
>>>> -
>>>> +#define UBLK_U_CMD_TRY_STOP_DEV              \
>>>> +     _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
>>>>    /*
>>>>     * 64bits are enough now, and it should be easy to extend in case of
>>>>     * running out of feature flags
>>>> @@ -311,6 +312,12 @@
>>>>     */
>>>>    #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
>>>>
>>>> +/*
>>>> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
>>>> + * allows stopping the device only if there are no openers.
>>>> + */
>>>> +#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>>>> +
>>>>    /* device state */
>>>>    #define UBLK_S_DEV_DEAD      0
>>>>    #define UBLK_S_DEV_LIVE      1
>>>> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
>>>> index 185ba553686a..6739e28c4059 100644
>>>> --- a/tools/testing/selftests/ublk/kublk.c
>>>> +++ b/tools/testing/selftests/ublk/kublk.c
>>>> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
>>>>                 FEAT_NAME(UBLK_F_QUIESCE),
>>>>                 FEAT_NAME(UBLK_F_PER_IO_DAEMON),
>>>>                 FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
>>>> +             FEAT_NAME(UBLK_F_SAFE_STOP_DEV),
>>>
>>> We usually split driver and test code into different patches, and the patch
>>> looks fine:
>>>
>>> Reviewed-by: Ming Lei <ming.lei@redhat.com>
>>>
>>> But we usually separate test code from driver change, also new feature needs
>>> selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
>>> the attached selftest patch and post all 3 in v5?
>>>
>>>
>>>
>>> Thanks,
>>> Ming
>>
>> Hi,
>>
>> Thank you Ming, just to be sure:
>> I'll split the kublk change to a 3rd patch - that's ok.
>> But do you also want me to write a selftest for this feature as part of
>> the 3rd patch?
> 
> The 3rd patch has included one complete test case, but feel free to
> fix/improve, or even rewrite it if you think it isn't good.
> 
> 
> Thanks,
> Ming
> 

No problem.
Will do.

Thank you

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command
       [not found]   ` <922b4c45-14ba-497b-8a00-eac2903a2854@zkern.zazolabs.com>
@ 2026-01-18 14:09     ` Yoav Cohen
  0 siblings, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-18 14:09 UTC (permalink / raw)
  To: Alexander Atanasov, Ming Lei, Jens Axboe,
	linux-block@vger.kernel.org, csander@purestorage.com
  Cc: Jared Holzman, Omri Levi

On 18/01/2026 16:06, Alexander Atanasov wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 6.01.26 22:33, Yoav Cohen wrote:
>> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
>> device if there are no active openers for the ublk block device.
>> If the device is busy, the command returns -EBUSY instead of
>> disrupting active clients. This allows safe, non-destructive stopping.
>>
>> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
>> feature flag.
>>
>> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
>> ---
>>   drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>>   include/uapi/linux/ublk_cmd.h        |  9 +++++-
>>   tools/testing/selftests/ublk/kublk.c |  1 +
>>   3 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 2d5602ef05cc..fc8b87902f8f 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -54,6 +54,7 @@
>>   #define UBLK_CMD_DEL_DEV_ASYNC      _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
> [snip]
> 
> 
>> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, 
>> blk_mode_t mode)
>>                       return -EPERM;
>>       }
>>
>> +     if (ub->block_open)
>> +             return -EBUSY;
>> +
> 
> Why back to EBUSY?
> 
> 
This is an old patch version - v7 was already pushed with your comments 
applied(-ENXIO)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-01-18 14:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-06 20:33 [PATCH v4 0/2] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
2026-01-06 20:33 ` [PATCH v4 1/2] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
2026-01-06 20:33 ` [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
2026-01-07 13:20   ` Ming Lei
2026-01-07 13:34     ` Yoav Cohen
2026-01-07 14:34       ` Ming Lei
2026-01-07 21:04         ` Yoav Cohen
     [not found]   ` <922b4c45-14ba-497b-8a00-eac2903a2854@zkern.zazolabs.com>
2026-01-18 14:09     ` Yoav Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox