diff mbox series

[V3,5/5] ublk_drv: cleanup ublksrv_ctrl_dev_info

Message ID 20220729072954.1070514-6-ming.lei@redhat.com (mailing list archive)
State New, archived
Headers show
Series ublk_drv: add generic mechanism to get/set parameters | expand

Commit Message

Ming Lei July 29, 2022, 7:29 a.m. UTC 
Remove all block device related info from ublksrv_ctrl_dev_info,
meantime reduce its size into 64 bytes because:

1) ublksrv_ctrl_dev_info becomes cleaner without including any
block related info

2) generic set/get parameter command can be used to set block
related setting easily and cleanly

3) generic set/get parameter command can be used for extending
ublk without needing more info in ublksrv_ctrl_dev_info

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c      | 17 +++++++----------
 include/uapi/linux/ublk_cmd.h | 10 +++++-----
 2 files changed, 12 insertions(+), 15 deletions(-)

Comments

Christoph Hellwig July 29, 2022, 2:24 p.m. UTC | #1 
On Fri, Jul 29, 2022 at 03:29:54PM +0800, Ming Lei wrote:
> Remove all block device related info from ublksrv_ctrl_dev_info,
> meantime reduce its size into 64 bytes because:
> 
> 1) ublksrv_ctrl_dev_info becomes cleaner without including any
> block related info
> 
> 2) generic set/get parameter command can be used to set block
> related setting easily and cleanly
> 
> 3) generic set/get parameter command can be used for extending
> ublk without needing more info in ublksrv_ctrl_dev_info

This should condense the structure instead of spreading random
reserveѕ.

One more reason why we should not just merge half-baked UAPIs before
a few good rounds of review :(
Ming Lei July 29, 2022, 3:02 p.m. UTC | #2 
On Fri, Jul 29, 2022 at 04:24:31PM +0200, Christoph Hellwig wrote:
> On Fri, Jul 29, 2022 at 03:29:54PM +0800, Ming Lei wrote:
> > Remove all block device related info from ublksrv_ctrl_dev_info,
> > meantime reduce its size into 64 bytes because:
> > 
> > 1) ublksrv_ctrl_dev_info becomes cleaner without including any
> > block related info
> > 
> > 2) generic set/get parameter command can be used to set block
> > related setting easily and cleanly
> > 
> > 3) generic set/get parameter command can be used for extending
> > ublk without needing more info in ublksrv_ctrl_dev_info
> 
> This should condense the structure instead of spreading random
> reserveѕ.
> 
> One more reason why we should not just merge half-baked UAPIs before
> a few good rounds of review :(

This patch does depend on set/get parameter(s) command.



Thanks,
Ming
diff mbox series

Patch

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2f55c1c07a09..b7dd50099aef 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -119,7 +119,6 @@  struct ublk_device {
 	char	*__queues;
 
 	unsigned short  queue_size;
-	unsigned short  bs_shift;
 	struct ublksrv_ctrl_dev_info	dev_info;
 
 	struct blk_mq_tag_set	tag_set;
@@ -174,7 +173,7 @@  static int ublk_dev_param_basic_validate(const struct ublk_device *ub,
 	if (p->logical_bs_shift > p->physical_bs_shift)
 		return -EINVAL;
 
-	if (p->max_sectors > (ub->dev_info.rq_max_blocks << (ub->bs_shift - 9)))
+	if (p->max_sectors > (ub->dev_info.max_io_buf_bytes >> 9))
 		return -EINVAL;
 
 	return 0;
@@ -1262,13 +1261,13 @@  static void ublk_stop_work_fn(struct work_struct *work)
 	ublk_stop_dev(ub);
 }
 
-/* align maximum I/O size to PAGE_SIZE */
+/* align max io buffer size with PAGE_SIZE */
 static void ublk_align_max_io_size(struct ublk_device *ub)
 {
-	unsigned int max_rq_bytes = ub->dev_info.rq_max_blocks << ub->bs_shift;
+	unsigned int max_io_bytes = ub->dev_info.max_io_buf_bytes;
 
-	ub->dev_info.rq_max_blocks =
-		round_down(max_rq_bytes, PAGE_SIZE) >> ub->bs_shift;
+	ub->dev_info.max_io_buf_bytes =
+		round_down(max_io_bytes, PAGE_SIZE);
 }
 
 static int ublk_add_tag_set(struct ublk_device *ub)
@@ -1430,9 +1429,8 @@  static inline void ublk_dump_dev_info(struct ublksrv_ctrl_dev_info *info)
 {
 	pr_devel("%s: dev id %d flags %llx\n", __func__,
 			info->dev_id, info->flags);
-	pr_devel("\t nr_hw_queues %d queue_depth %d block size %d dev_capacity %lld\n",
-			info->nr_hw_queues, info->queue_depth,
-			info->block_size, info->dev_blocks);
+	pr_devel("\t nr_hw_queues %d queue_depth %d\n",
+			info->nr_hw_queues, info->queue_depth);
 }
 
 static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
@@ -1492,7 +1490,6 @@  static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
 	/* We are not ready to support zero copy */
 	ub->dev_info.flags &= ~UBLK_F_SUPPORT_ZERO_COPY;
 
-	ub->bs_shift = ilog2(ub->dev_info.block_size);
 	ub->dev_info.nr_hw_queues = min_t(unsigned int,
 			ub->dev_info.nr_hw_queues, nr_cpu_ids);
 	ublk_align_max_io_size(ub);
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index 78c2b420b720..c5a01268060c 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -80,22 +80,22 @@  struct ublksrv_ctrl_cmd {
 struct ublksrv_ctrl_dev_info {
 	__u16	nr_hw_queues;
 	__u16	queue_depth;
-	__u16	block_size;
+	__u16	reserved0;
 	__u16	state;
 
-	__u32	rq_max_blocks;
+	__u32	max_io_buf_bytes;
 	__u32	dev_id;
 
-	__u64   dev_blocks;
+	__u64   reserved1;
 
 	__s32	ublksrv_pid;
-	__s32	reserved0;
+	__s32	reserved2;
 	__u64	flags;
 	__u64	flags_reserved;
 
 	/* For ublksrv internal use, invisible to ublk driver */
 	__u64	ublksrv_flags;
-	__u64	reserved1[9];
+	__u64	reserved3;
 };
 
 #define		UBLK_IO_OP_READ		0