[PATCH 0/2] ublk: specify io_cmd_buf pointer type

[PATCH 0/2] ublk: specify io_cmd_buf pointer type

[Caleb Sander Mateos]

io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
char *. Indexing the array requires an explicit multiplication and cast.
The compiler also can't check the pointer types.

Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
indexed directly and the compiler can type-check the code.

Make the same change to the ublk selftests.

Caleb Sander Mateos (2):
  ublk: specify io_cmd_buf pointer type
  selftests: ublk: specify io_cmd_buf pointer type

 drivers/block/ublk_drv.c             | 8 ++++----
 tools/testing/selftests/ublk/kublk.c | 2 +-
 tools/testing/selftests/ublk/kublk.h | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.45.2

[PATCH 1/2] ublk: specify io_cmd_buf pointer type

[Caleb Sander Mateos]

io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
char *. Indexing the array requires an explicit multiplication and cast.
The compiler also can't check the pointer types.

Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
indexed directly and the compiler can type-check the code.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 drivers/block/ublk_drv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 355a59c78539..ed73e2ffdf09 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -153,11 +153,11 @@ struct ublk_queue {
 	int q_id;
 	int q_depth;
 
 	unsigned long flags;
 	struct task_struct	*ubq_daemon;
-	char *io_cmd_buf;
+	struct ublksrv_io_desc *io_cmd_buf;
 
 	bool force_abort;
 	bool timeout;
 	bool canceling;
 	bool fail_io; /* copy of dev->state == UBLK_S_DEV_FAIL_IO */
@@ -701,15 +701,15 @@ static inline bool ublk_rq_has_data(const struct request *rq)
 }
 
 static inline struct ublksrv_io_desc *ublk_get_iod(struct ublk_queue *ubq,
 		int tag)
 {
-	return (struct ublksrv_io_desc *)
-		&(ubq->io_cmd_buf[tag * sizeof(struct ublksrv_io_desc)]);
+	return &ubq->io_cmd_buf[tag];
 }
 
-static inline char *ublk_queue_cmd_buf(struct ublk_device *ub, int q_id)
+static inline struct ublksrv_io_desc *
+ublk_queue_cmd_buf(struct ublk_device *ub, int q_id)
 {
 	return ublk_get_queue(ub, q_id)->io_cmd_buf;
 }
 
 static inline int __ublk_queue_cmd_buf_size(int depth)
-- 
2.45.2

[PATCH 2/2] selftests: ublk: specify io_cmd_buf pointer type

[Caleb Sander Mateos]

Matching the ublk driver, change the type of io_cmd_buf from char * to
struct ublksrv_io_desc *.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
---
 tools/testing/selftests/ublk/kublk.c | 2 +-
 tools/testing/selftests/ublk/kublk.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
index 05147b53c361..83756f97c26e 100644
--- a/tools/testing/selftests/ublk/kublk.c
+++ b/tools/testing/selftests/ublk/kublk.c
@@ -320,11 +320,11 @@ static int ublk_queue_init(struct ublk_queue *q)
 		q->state |= UBLKSRV_ZC;
 	}
 
 	cmd_buf_size = ublk_queue_cmd_buf_sz(q);
 	off = UBLKSRV_CMD_BUF_OFFSET + q->q_id * ublk_queue_max_cmd_buf_sz();
-	q->io_cmd_buf = (char *)mmap(0, cmd_buf_size, PROT_READ,
+	q->io_cmd_buf = mmap(0, cmd_buf_size, PROT_READ,
 			MAP_SHARED | MAP_POPULATE, dev->fds[0], off);
 	if (q->io_cmd_buf == MAP_FAILED) {
 		ublk_err("ublk dev %d queue %d map io_cmd_buf failed %m\n",
 				q->dev->dev_info.dev_id, q->q_id);
 		goto fail;
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index f31a5c4d4143..760ff8ffb810 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -126,11 +126,11 @@ struct ublk_queue {
 	int q_depth;
 	unsigned int cmd_inflight;
 	unsigned int io_inflight;
 	struct ublk_dev *dev;
 	const struct ublk_tgt_ops *tgt_ops;
-	char *io_cmd_buf;
+	struct ublksrv_io_desc *io_cmd_buf;
 	struct io_uring ring;
 	struct ublk_io ios[UBLK_QUEUE_DEPTH];
 #define UBLKSRV_QUEUE_STOPPING	(1U << 0)
 #define UBLKSRV_QUEUE_IDLE	(1U << 1)
 #define UBLKSRV_NO_BUF		(1U << 2)
@@ -300,11 +300,11 @@ static inline void ublk_mark_io_done(struct ublk_io *io, int res)
 	io->result = res;
 }
 
 static inline const struct ublksrv_io_desc *ublk_get_iod(const struct ublk_queue *q, int tag)
 {
-	return (struct ublksrv_io_desc *)&(q->io_cmd_buf[tag * sizeof(struct ublksrv_io_desc)]);
+	return &q->io_cmd_buf[tag];
 }
 
 static inline void ublk_set_sqe_cmd_op(struct io_uring_sqe *sqe, __u32 cmd_op)
 {
 	__u32 *addr = (__u32 *)&sqe->off;
-- 
2.45.2

Re: [PATCH 0/2] ublk: specify io_cmd_buf pointer type

[Jens Axboe]

On 3/28/25 1:42 PM, Caleb Sander Mateos wrote:
> io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
> char *. Indexing the array requires an explicit multiplication and cast.
> The compiler also can't check the pointer types.
> 
> Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
> indexed directly and the compiler can type-check the code.
> 
> Make the same change to the ublk selftests.

Looks good to me, nice cleanup, more readable now.

-- 
Jens Axboe

Re: [PATCH 0/2] ublk: specify io_cmd_buf pointer type

[Shuah Khan]

On 3/28/25 13:42, Caleb Sander Mateos wrote:
> io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
> char *. Indexing the array requires an explicit multiplication and cast.
> The compiler also can't check the pointer types.
> 
> Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
> indexed directly and the compiler can type-check the code.
> 
> Make the same change to the ublk selftests.
> 
> Caleb Sander Mateos (2):
>    ublk: specify io_cmd_buf pointer type
>    selftests: ublk: specify io_cmd_buf pointer type
> 
>   drivers/block/ublk_drv.c             | 8 ++++----
>   tools/testing/selftests/ublk/kublk.c | 2 +-
>   tools/testing/selftests/ublk/kublk.h | 4 ++--
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 

For selftests changes:

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH 0/2] ublk: specify io_cmd_buf pointer type

[Ming Lei]

On Fri, Mar 28, 2025 at 01:42:28PM -0600, Caleb Sander Mateos wrote:
> io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
> char *. Indexing the array requires an explicit multiplication and cast.
> The compiler also can't check the pointer types.
> 
> Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
> indexed directly and the compiler can type-check the code.
> 
> Make the same change to the ublk selftests.
> 
> Caleb Sander Mateos (2):
>   ublk: specify io_cmd_buf pointer type
>   selftests: ublk: specify io_cmd_buf pointer type
> 
>  drivers/block/ublk_drv.c             | 8 ++++----
>  tools/testing/selftests/ublk/kublk.c | 2 +-
>  tools/testing/selftests/ublk/kublk.h | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)

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

Thanks,
Ming

Re: [PATCH 0/2] ublk: specify io_cmd_buf pointer type

[Jens Axboe]

On Fri, 28 Mar 2025 13:42:28 -0600, Caleb Sander Mateos wrote:
> io_cmd_buf points to an array of ublksrv_io_desc structs but its type is
> char *. Indexing the array requires an explicit multiplication and cast.
> The compiler also can't check the pointer types.
> 
> Change io_cmd_buf's type to struct ublksrv_io_desc * so it can be
> indexed directly and the compiler can type-check the code.
> 
> [...]

Applied, thanks!

[1/2] ublk: specify io_cmd_buf pointer type
      commit: 9a45714fc51321ea8f5e5567f70e06753a848f62
[2/2] selftests: ublk: specify io_cmd_buf pointer type
      commit: 25aaa81371e7db34ddb42c69ed4f6c5bc8de2afa

Best regards,
-- 
Jens Axboe