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

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

Changes since v4:
 - Selftest added by Ming
 - Move kublk.c change to new selftest commit

Changes since v5:
 - Return -ENXIO when device is going down instead of -EBUSY
 - Update the patch 2 commit msg

Ming Lei (1):
  selftests: ublk: add stop command with --safe option

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/Makefile         |  1 +
 tools/testing/selftests/ublk/kublk.c          | 53 +++++++++++++++++
 tools/testing/selftests/ublk/kublk.h          |  1 +
 .../testing/selftests/ublk/test_generic_16.sh | 57 +++++++++++++++++++
 6 files changed, 165 insertions(+), 6 deletions(-)
 create mode 100755 tools/testing/selftests/ublk/test_generic_16.sh

-- 
2.39.5 (Apple Git-154)


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

* [PATCH v6 1/3] ublk: make ublk_ctrl_stop_dev return void
  2026-01-12 13:36 [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
@ 2026-01-12 13:36 ` Yoav Cohen
  2026-01-12 13:36 ` [PATCH v6 2/3] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-12 13:36 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander, alex
  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 v6 2/3] ublk: add UBLK_CMD_TRY_STOP_DEV command
  2026-01-12 13:36 [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
  2026-01-12 13:36 ` [PATCH v6 1/3] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
@ 2026-01-12 13:36 ` Yoav Cohen
  2026-01-12 13:36 ` [PATCH v6 3/3] selftests: ublk: add stop command with --safe option Yoav Cohen
  2026-01-12 16:22 ` [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Jens Axboe
  3 siblings, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-12 13:36 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander, alex
  Cc: jholzman, omril, Yoav Cohen, Yoav Cohen

Add a best-effort stop command, UBLK_CMD_TRY_STOP_DEV, which only stops a
ublk device when it has no active openers.

Unlike UBLK_CMD_STOP_DEV, this command does not disrupt existing users.
New opens are blocked only after disk_openers has reached zero; if the
device is busy, the command returns -EBUSY and leaves it running.

The ub->block_open flag is used only to close a race with an in-progress
open and does not otherwise change open behavior.

Advertise support via the UBLK_F_SAFE_STOP_DEV feature flag.

Signed-off-by: Yoav Cohen <yoav@nvidia.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c      | 44 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/ublk_cmd.h |  9 ++++++-
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2d5602ef05cc..f91e70aa402e 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 -ENXIO;
+
 	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
-- 
2.39.5 (Apple Git-154)


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

* [PATCH v6 3/3] selftests: ublk: add stop command with --safe option
  2026-01-12 13:36 [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
  2026-01-12 13:36 ` [PATCH v6 1/3] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
  2026-01-12 13:36 ` [PATCH v6 2/3] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
@ 2026-01-12 13:36 ` Yoav Cohen
  2026-01-12 16:22 ` [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Jens Axboe
  3 siblings, 0 replies; 8+ messages in thread
From: Yoav Cohen @ 2026-01-12 13:36 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, linux-block, csander, alex
  Cc: jholzman, omril, Yoav Cohen

From: Ming Lei <ming.lei@redhat.com>

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          | 53 +++++++++++++++++
 tools/testing/selftests/ublk/kublk.h          |  1 +
 .../testing/selftests/ublk/test_generic_16.sh | 57 +++++++++++++++++++
 4 files changed, 112 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 185ba553686a..c217745b0523 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();
@@ -1454,6 +1499,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;
@@ -1581,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);
@@ -1612,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;
@@ -1696,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 '?':
 			/*
@@ -1763,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.39.5 (Apple Git-154)


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

* Re: [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV
  2026-01-12 13:36 [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
                   ` (2 preceding siblings ...)
  2026-01-12 13:36 ` [PATCH v6 3/3] selftests: ublk: add stop command with --safe option Yoav Cohen
@ 2026-01-12 16:22 ` Jens Axboe
  2026-01-12 16:28   ` Caleb Sander Mateos
  3 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2026-01-12 16:22 UTC (permalink / raw)
  To: Yoav Cohen, Ming Lei, linux-block, csander, alex
  Cc: jholzman, omril, Yoav Cohen

Please rebase this on the current for-7.0/block tree. It doesn't
apply at all, and hunks like:

@@ -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

is a clear sign that your way off base at this point. Why else would
STOP_DEV be 1 << 17, with the previous one at 1 << 14?

-- 
Jens Axboe

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

* Re: [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV
  2026-01-12 16:22 ` [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Jens Axboe
@ 2026-01-12 16:28   ` Caleb Sander Mateos
  2026-01-12 16:33     ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: Caleb Sander Mateos @ 2026-01-12 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Yoav Cohen, Ming Lei, linux-block, alex, jholzman, omril,
	Yoav Cohen

On Mon, Jan 12, 2026 at 8:22 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> Please rebase this on the current for-7.0/block tree. It doesn't
> apply at all, and hunks like:
>
> @@ -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
>
> is a clear sign that your way off base at this point. Why else would
> STOP_DEV be 1 << 17, with the previous one at 1 << 14?

This is due to being developed in parallel with other ublk patch sets
which are using feature flags 15 (UBLK_F_BATCH_IO) and 16
(UBLK_F_INTEGRITY). UBLK_F_BATCH_IO is still in development and
UBLK_F_INTEGRITY was just applied. So I don't think it's fair to blame
Yoav for not having rebased on commits that didn't exist yet when this
version of the patch series was sent out :) Should be a trivial
rebase, though, since the value of 17 was already chosen to avoid
conflicting with the other patch sets.

Best,
Caleb

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

* Re: [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV
  2026-01-12 16:28   ` Caleb Sander Mateos
@ 2026-01-12 16:33     ` Jens Axboe
  2026-01-12 17:07       ` Caleb Sander Mateos
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2026-01-12 16:33 UTC (permalink / raw)
  To: Caleb Sander Mateos
  Cc: Yoav Cohen, Ming Lei, linux-block, alex, jholzman, omril,
	Yoav Cohen

On 1/12/26 9:28 AM, Caleb Sander Mateos wrote:
> On Mon, Jan 12, 2026 at 8:22?AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Please rebase this on the current for-7.0/block tree. It doesn't
>> apply at all, and hunks like:
>>
>> @@ -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
>>
>> is a clear sign that your way off base at this point. Why else would
>> STOP_DEV be 1 << 17, with the previous one at 1 << 14?
> 
> This is due to being developed in parallel with other ublk patch sets
> which are using feature flags 15 (UBLK_F_BATCH_IO) and 16
> (UBLK_F_INTEGRITY). UBLK_F_BATCH_IO is still in development and
> UBLK_F_INTEGRITY was just applied. So I don't think it's fair to blame
> Yoav for not having rebased on commits that didn't exist yet when this
> version of the patch series was sent out :) Should be a trivial
> rebase, though, since the value of 17 was already chosen to avoid
> conflicting with the other patch sets.

At the time v6 was posted, it did not apply to the current branches. So
regardless of how it was developed, that's not a very useful approach as
it means I have to hand apply it. Which isn't too difficult for patches
1-2. But now we have the integrity bits in, which means the selftest
bits take more work. At that point I just threw my hands up.

Hence please repost against the current tree.

-- 
Jens Axboe

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

* Re: [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV
  2026-01-12 16:33     ` Jens Axboe
@ 2026-01-12 17:07       ` Caleb Sander Mateos
  0 siblings, 0 replies; 8+ messages in thread
From: Caleb Sander Mateos @ 2026-01-12 17:07 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Yoav Cohen, Ming Lei, linux-block, alex, jholzman, omril,
	Yoav Cohen

On Mon, Jan 12, 2026 at 8:33 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 1/12/26 9:28 AM, Caleb Sander Mateos wrote:
> > On Mon, Jan 12, 2026 at 8:22?AM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> Please rebase this on the current for-7.0/block tree. It doesn't
> >> apply at all, and hunks like:
> >>
> >> @@ -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
> >>
> >> is a clear sign that your way off base at this point. Why else would
> >> STOP_DEV be 1 << 17, with the previous one at 1 << 14?
> >
> > This is due to being developed in parallel with other ublk patch sets
> > which are using feature flags 15 (UBLK_F_BATCH_IO) and 16
> > (UBLK_F_INTEGRITY). UBLK_F_BATCH_IO is still in development and
> > UBLK_F_INTEGRITY was just applied. So I don't think it's fair to blame
> > Yoav for not having rebased on commits that didn't exist yet when this
> > version of the patch series was sent out :) Should be a trivial
> > rebase, though, since the value of 17 was already chosen to avoid
> > conflicting with the other patch sets.
>
> At the time v6 was posted, it did not apply to the current branches. So

I just tried it, and v6 applies just fine at commit 5df832ba5f9d
("Merge branch 'block-6.19' into for-7.0/block") prior to the "ublk:
add support for integrity data" patch set. And v6 was sent at 13:36
UTC, while the integrity patch set was applied at 16:22 UTC. So It
looks like v6 in fact did apply to for-7.0/block when posted.

> regardless of how it was developed, that's not a very useful approach as
> it means I have to hand apply it. Which isn't too difficult for patches
> 1-2. But now we have the integrity bits in, which means the selftest
> bits take more work. At that point I just threw my hands up.
>
> Hence please repost against the current tree.

Yes, completely agree it makes sense to resend the patch set rebased
on top of for-7.0/block with the integrity changes.

Best,
Caleb

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

end of thread, other threads:[~2026-01-12 17:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-12 13:36 [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Yoav Cohen
2026-01-12 13:36 ` [PATCH v6 1/3] ublk: make ublk_ctrl_stop_dev return void Yoav Cohen
2026-01-12 13:36 ` [PATCH v6 2/3] ublk: add UBLK_CMD_TRY_STOP_DEV command Yoav Cohen
2026-01-12 13:36 ` [PATCH v6 3/3] selftests: ublk: add stop command with --safe option Yoav Cohen
2026-01-12 16:22 ` [PATCH v6 0/3] ublk: introduce UBLK_CMD_TRY_STOP_DEV Jens Axboe
2026-01-12 16:28   ` Caleb Sander Mateos
2026-01-12 16:33     ` Jens Axboe
2026-01-12 17:07       ` Caleb Sander Mateos

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