public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe
@ 2020-01-29 13:22 Stefan Metzmacher
  2020-01-29 13:39 ` Stefan Metzmacher
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Metzmacher @ 2020-01-29 13:22 UTC (permalink / raw)
  To: io-uring; +Cc: Stefan Metzmacher

With nesting of anonymous unions and structs it's hard to
review layout changes. It's better to ask the compiler
for these things.

Signed-off-by: Stefan Metzmacher <[email protected]>
---
 fs/io_uring.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9479b5481ad2..791bbc28c0b7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6882,6 +6882,39 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
 
 static int __init io_uring_init(void)
 {
+#define __BUILD_BUG_VERIFY_ELEMENT(stype, eoffset, etype, ename) do { \
+	BUILD_BUG_ON(offsetof(stype, ename) != eoffset); \
+	BUILD_BUG_ON(sizeof(etype) != sizeof_field(stype, ename)); \
+} while (0)
+
+#define BUILD_BUG_SQE_ELEM(eoffset, etype, ename) \
+	__BUILD_BUG_VERIFY_ELEMENT(struct io_uring_sqe, eoffset, etype, ename)
+	BUILD_BUG_ON(sizeof(struct io_uring_sqe) != 64);
+	BUILD_BUG_SQE_ELEM(0,  __u8,   opcode);
+	BUILD_BUG_SQE_ELEM(1,  __u8,   flags);
+	BUILD_BUG_SQE_ELEM(2,  __u16,  ioprio);
+	BUILD_BUG_SQE_ELEM(4,  __s32,  fd);
+	BUILD_BUG_SQE_ELEM(8,  __u64,  off);
+	BUILD_BUG_SQE_ELEM(8,  __u64,  addr2);
+	BUILD_BUG_SQE_ELEM(16, __u64,  addr);
+	BUILD_BUG_SQE_ELEM(24, __u32,  len);
+	BUILD_BUG_SQE_ELEM(28,    __kernel_rwf_t,  rw_flags);
+	BUILD_BUG_SQE_ELEM(28, /* compat */   int, rw_flags);
+	BUILD_BUG_SQE_ELEM(28, /* compat */ __u32, rw_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  fsync_flags);
+	BUILD_BUG_SQE_ELEM(28, __u16,  poll_events);
+	BUILD_BUG_SQE_ELEM(28, __u32,  sync_range_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  msg_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  timeout_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  accept_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  cancel_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  open_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  statx_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  fadvise_advice);
+	BUILD_BUG_SQE_ELEM(32, __u64,  user_data);
+	BUILD_BUG_SQE_ELEM(40, __u16,  buf_index);
+	BUILD_BUG_SQE_ELEM(42, __u16,  personality);
+
 	BUILD_BUG_ON(ARRAY_SIZE(io_op_defs) != IORING_OP_LAST);
 	req_cachep = KMEM_CACHE(io_kiocb, SLAB_HWCACHE_ALIGN | SLAB_PANIC);
 	return 0;
-- 
2.17.1


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

* [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe
  2020-01-29 13:22 [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe Stefan Metzmacher
@ 2020-01-29 13:39 ` Stefan Metzmacher
  2020-01-30 15:51   ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Metzmacher @ 2020-01-29 13:39 UTC (permalink / raw)
  To: io-uring; +Cc: Stefan Metzmacher

With nesting of anonymous unions and structs it's hard to
review layout changes. It's better to ask the compiler
for these things.

Signed-off-by: Stefan Metzmacher <[email protected]>
---
 fs/io_uring.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9479b5481ad2..b31523f4db6e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6882,6 +6882,39 @@ SYSCALL_DEFINE4(io_uring_register, unsigned int, fd, unsigned int, opcode,
 
 static int __init io_uring_init(void)
 {
+#define __BUILD_BUG_VERIFY_ELEMENT(stype, eoffset, etype, ename) do { \
+	BUILD_BUG_ON(offsetof(stype, ename) != eoffset); \
+	BUILD_BUG_ON(sizeof(etype) != sizeof_field(stype, ename)); \
+} while (0)
+
+#define BUILD_BUG_SQE_ELEM(eoffset, etype, ename) \
+	__BUILD_BUG_VERIFY_ELEMENT(struct io_uring_sqe, eoffset, etype, ename)
+	BUILD_BUG_ON(sizeof(struct io_uring_sqe) != 64);
+	BUILD_BUG_SQE_ELEM(0,  __u8,   opcode);
+	BUILD_BUG_SQE_ELEM(1,  __u8,   flags);
+	BUILD_BUG_SQE_ELEM(2,  __u16,  ioprio);
+	BUILD_BUG_SQE_ELEM(4,  __s32,  fd);
+	BUILD_BUG_SQE_ELEM(8,  __u64,  off);
+	BUILD_BUG_SQE_ELEM(8,  __u64,  addr2);
+	BUILD_BUG_SQE_ELEM(16, __u64,  addr);
+	BUILD_BUG_SQE_ELEM(24, __u32,  len);
+	BUILD_BUG_SQE_ELEM(28,     __kernel_rwf_t, rw_flags);
+	BUILD_BUG_SQE_ELEM(28, /* compat */   int, rw_flags);
+	BUILD_BUG_SQE_ELEM(28, /* compat */ __u32, rw_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  fsync_flags);
+	BUILD_BUG_SQE_ELEM(28, __u16,  poll_events);
+	BUILD_BUG_SQE_ELEM(28, __u32,  sync_range_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  msg_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  timeout_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  accept_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  cancel_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  open_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  statx_flags);
+	BUILD_BUG_SQE_ELEM(28, __u32,  fadvise_advice);
+	BUILD_BUG_SQE_ELEM(32, __u64,  user_data);
+	BUILD_BUG_SQE_ELEM(40, __u16,  buf_index);
+	BUILD_BUG_SQE_ELEM(42, __u16,  personality);
+
 	BUILD_BUG_ON(ARRAY_SIZE(io_op_defs) != IORING_OP_LAST);
 	req_cachep = KMEM_CACHE(io_kiocb, SLAB_HWCACHE_ALIGN | SLAB_PANIC);
 	return 0;
-- 
2.17.1


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

* Re: [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe
  2020-01-29 13:39 ` Stefan Metzmacher
@ 2020-01-30 15:51   ` Jens Axboe
  2020-01-30 16:01     ` Stefan Metzmacher
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2020-01-30 15:51 UTC (permalink / raw)
  To: Stefan Metzmacher, io-uring

On 1/29/20 6:39 AM, Stefan Metzmacher wrote:
> With nesting of anonymous unions and structs it's hard to
> review layout changes. It's better to ask the compiler
> for these things.

I don't think I've once messed it up while adding new features,
but I'm fine with adding this as an extra safeguard. Realistically,
the regression tests would fail spectacularly if we ever did mess
this up.

-- 
Jens Axboe


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

* Re: [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe
  2020-01-30 15:51   ` Jens Axboe
@ 2020-01-30 16:01     ` Stefan Metzmacher
  2020-01-30 16:06       ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Metzmacher @ 2020-01-30 16:01 UTC (permalink / raw)
  To: Jens Axboe, io-uring


[-- Attachment #1.1: Type: text/plain, Size: 1609 bytes --]

Am 30.01.20 um 16:51 schrieb Jens Axboe:
> On 1/29/20 6:39 AM, Stefan Metzmacher wrote:
>> With nesting of anonymous unions and structs it's hard to
>> review layout changes. It's better to ask the compiler
>> for these things.
> 
> I don't think I've once messed it up while adding new features,
> but I'm fine with adding this as an extra safeguard. Realistically,
> the regression tests would fail spectacularly if we ever did mess
> this up.

Only if you don't just use :-)

cp linux/include/uapi/linux/io_uring.h \
   liburing/src/include/liburing/io_uring.h

BTW: the current diff is

--- linux/include/uapi/linux/io_uring.h
+++ liburing/src/include/liburing/io_uring.h
@@ -43,7 +43,7 @@ struct io_uring_sqe {
                struct {
                        /* index into fixed buffers, if used */
                        __u16   buf_index;
-                       /* personality to use, if used */
+                       /* personality to use */
                        __u16   personality;
                };
                __u64   __pad2[3];
@@ -112,7 +112,6 @@ enum {
        IORING_OP_SEND,
        IORING_OP_RECV,
        IORING_OP_OPENAT2,
-       IORING_OP_EPOLL_CTL,

        /* this goes last, obviously */
        IORING_OP_LAST,
@@ -203,7 +202,6 @@ struct io_uring_params {
 #define IORING_FEAT_NODROP             (1U << 1)
 #define IORING_FEAT_SUBMIT_STABLE      (1U << 2)
 #define IORING_FEAT_RW_CUR_POS         (1U << 3)
-#define IORING_FEAT_CUR_PERSONALITY    (1U << 4)

 /*
  * io_uring_register(2) opcodes and arguments


metze




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe
  2020-01-30 16:01     ` Stefan Metzmacher
@ 2020-01-30 16:06       ` Jens Axboe
  0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2020-01-30 16:06 UTC (permalink / raw)
  To: Stefan Metzmacher, io-uring

On 1/30/20 9:01 AM, Stefan Metzmacher wrote:
> Am 30.01.20 um 16:51 schrieb Jens Axboe:
>> On 1/29/20 6:39 AM, Stefan Metzmacher wrote:
>>> With nesting of anonymous unions and structs it's hard to
>>> review layout changes. It's better to ask the compiler
>>> for these things.
>>
>> I don't think I've once messed it up while adding new features,
>> but I'm fine with adding this as an extra safeguard. Realistically,
>> the regression tests would fail spectacularly if we ever did mess
>> this up.
> 
> Only if you don't just use :-)
> 
> cp linux/include/uapi/linux/io_uring.h \
>    liburing/src/include/liburing/io_uring.h
> 
> BTW: the current diff is
> 
> --- linux/include/uapi/linux/io_uring.h
> +++ liburing/src/include/liburing/io_uring.h
> @@ -43,7 +43,7 @@ struct io_uring_sqe {
>                 struct {
>                         /* index into fixed buffers, if used */
>                         __u16   buf_index;
> -                       /* personality to use, if used */
> +                       /* personality to use */
>                         __u16   personality;
>                 };
>                 __u64   __pad2[3];
> @@ -112,7 +112,6 @@ enum {
>         IORING_OP_SEND,
>         IORING_OP_RECV,
>         IORING_OP_OPENAT2,
> -       IORING_OP_EPOLL_CTL,
> 
>         /* this goes last, obviously */
>         IORING_OP_LAST,
> @@ -203,7 +202,6 @@ struct io_uring_params {
>  #define IORING_FEAT_NODROP             (1U << 1)
>  #define IORING_FEAT_SUBMIT_STABLE      (1U << 2)
>  #define IORING_FEAT_RW_CUR_POS         (1U << 3)
> -#define IORING_FEAT_CUR_PERSONALITY    (1U << 4)
> 
>  /*
>   * io_uring_register(2) opcodes and arguments

I'll copy it in, I do that every now and then. But now's a good time
since everything has been flushed out to Linus.

-- 
Jens Axboe


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

end of thread, other threads:[~2020-01-30 16:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-29 13:22 [PATCH] io_uring: add BUILD_BUG_ON() to assert the layout of struct io_uring_sqe Stefan Metzmacher
2020-01-29 13:39 ` Stefan Metzmacher
2020-01-30 15:51   ` Jens Axboe
2020-01-30 16:01     ` Stefan Metzmacher
2020-01-30 16:06       ` Jens Axboe

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