[PATCH v1 0/2] io_uring uapi updates

[PATCH v1 0/2] io_uring uapi updates

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

io_uring uapi updates:

1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
   synced 1:1 into liburing's io_uring.h. liburing has a configure
   check to detect the need for linux/time_types.h (Stefan).

2) Do not use a zero-size array because it doesn't allow the user to
   compile an app that uses liburing with the `-pedantic-errors` flag:

       io_uring.h:611:28: error: zero size arrays are an extension [-Werror,-Wzero-length-array]

   Replace the array size from 0 to 1 (me).

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (1):
  io_uring: uapi: Don't use a zero-size array

Stefan Metzmacher (1):
  io_uring: uapi: Don't force linux/time_types.h for userspace

 include/uapi/linux/io_uring.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)


base-commit: 5576035f15dfcc6cb1cec236db40c2c0733b0ba4
-- 
Ammar Faizi

[PATCH v1 1/2] io_uring: uapi: Don't force linux/time_types.h for userspace

[Ammar Faizi]

From: Stefan Metzmacher <metze@samba.org>

include/uapi/linux/io_uring.h is synced 1:1 into
liburing:src/include/liburing/io_uring.h.

liburing has a configure check to detect the need for
linux/time_types.h.

Fixes: 78a861b9495920f8609dee5b670dacbff09d359f ("io_uring: add sync cancelation API through io_uring_register()")
Link: https://github.com/axboe/liburing/issues/708
Link: https://github.com/axboe/liburing/pull/709
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 include/uapi/linux/io_uring.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 2df3225b562f..77027cbaf786 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -1,25 +1,34 @@
 /* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR MIT */
 /*
  * Header file for the io_uring interface.
  *
  * Copyright (C) 2019 Jens Axboe
  * Copyright (C) 2019 Christoph Hellwig
  */
 #ifndef LINUX_IO_URING_H
 #define LINUX_IO_URING_H
 
 #include <linux/fs.h>
 #include <linux/types.h>
+/*
+ * This file is shared with liburing and it has to autodetect
+ * if linux/time_types.h is available
+ */
+#ifdef __KERNEL__
+#define HAVE_LINUX_TIME_TYPES_H 1
+#endif
+#ifdef HAVE_LINUX_TIME_TYPES_H
 #include <linux/time_types.h>
+#endif
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /*
  * IO submission data structure (Submission Queue Entry)
  */
 struct io_uring_sqe {
 	__u8	opcode;		/* type of operation for this sqe */
 	__u8	flags;		/* IOSQE_ flags */
 	__u16	ioprio;		/* ioprio for the request */
-- 
Ammar Faizi

[PATCH v1 2/2] io_uring: uapi: Don't use a zero-size array

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a zero-size array because it doesn't allow the user to
compile an app that uses liburing with the `-pedantic-errors` flag:

  io_uring.h:611:28: error: zero size arrays are an extension [-Werror,-Wzero-length-array]

Replace the array size from 0 to 1.

  - No functional change is intended.
  - No struct/union size change.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 include/uapi/linux/io_uring.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 77027cbaf786..0890784fcc9e 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -85,25 +85,25 @@ struct io_uring_sqe {
 			__u16	__pad3[1];
 		};
 	};
 	union {
 		struct {
 			__u64	addr3;
 			__u64	__pad2[1];
 		};
 		/*
 		 * If the ring is initialized with IORING_SETUP_SQE128, then
 		 * this field is used for 80 bytes of arbitrary command data
 		 */
-		__u8	cmd[0];
+		__u8	cmd[1];
 	};
 };
 
 /*
  * If sqe->file_index is set to this for opcodes that instantiate a new
  * direct descriptor (like openat/openat2/accept), then io_uring will allocate
  * an available direct descriptor instead of having the application pass one
  * in. The picked direct descriptor will be returned in cqe->res, or -ENFILE
  * if the space is full.
  */
 #define IORING_FILE_INDEX_ALLOC		(~0U)
 
@@ -599,25 +599,25 @@ struct io_uring_buf {
 struct io_uring_buf_ring {
 	union {
 		/*
 		 * To avoid spilling into more pages than we need to, the
 		 * ring tail is overlaid with the io_uring_buf->resv field.
 		 */
 		struct {
 			__u64	resv1;
 			__u32	resv2;
 			__u16	resv3;
 			__u16	tail;
 		};
-		struct io_uring_buf	bufs[0];
+		struct io_uring_buf	bufs[1];
 	};
 };
 
 /* argument for IORING_(UN)REGISTER_PBUF_RING */
 struct io_uring_buf_reg {
 	__u64	ring_addr;
 	__u32	ring_entries;
 	__u16	bgid;
 	__u16	pad;
 	__u64	resv[3];
 };
 
-- 
Ammar Faizi

Re: [PATCH v1 2/2] io_uring: uapi: Don't use a zero-size array

[Jens Axboe]

On 11/15/22 2:29 PM, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Don't use a zero-size array because it doesn't allow the user to
> compile an app that uses liburing with the `-pedantic-errors` flag:
> 
>   io_uring.h:611:28: error: zero size arrays are an extension [-Werror,-Wzero-length-array]
> 
> Replace the array size from 0 to 1.
> 
>   - No functional change is intended.
>   - No struct/union size change.

The only reason why they don't grow the struct, is because it's in
a union. I don't like this patch, as the zero sized array is a clear
sign that this struct has data past it. If it's a single entry, that's
very different.

Yes that apparently makes pendantic errors unhappy, but I care more
about the readability of it.

-- 
Jens Axboe

Re: [PATCH v1 2/2] io_uring: uapi: Don't use a zero-size array

[Pavel Begunkov]

On 11/15/22 21:29, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Don't use a zero-size array because it doesn't allow the user to
> compile an app that uses liburing with the `-pedantic-errors` flag:

Ammar, I'd strongly encourage you to at least compile your
patches or even better actually test them. There is an explicit
BUILD_BUG_ON() violated by this change.

-- 
Pavel Begunkov

Re: [PATCH v1 2/2] io_uring: uapi: Don't use a zero-size array

[Ammar Faizi]

On 11/16/22 5:14 PM, Pavel Begunkov wrote:
> On 11/15/22 21:29, Ammar Faizi wrote:
>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>
>> Don't use a zero-size array because it doesn't allow the user to
>> compile an app that uses liburing with the `-pedantic-errors` flag:
> 
> Ammar, I'd strongly encourage you to at least compile your
> patches or even better actually test them. There is an explicit
> BUILD_BUG_ON() violated by this change.

Oh yeah, I didn't realize that. This patch breaks this assertion:

   BUILD_BUG_ON failed: sizeof_field(struct io_uring_sqe, cmd) != 0

This assertion wants the size of cmd[] to be zero. Which is obviously
violated in this patch.

I only tested a liburing app that uses this header and validated
that the struct size is the same, but not its field. That's my
mistake.

I'm *not* going to send a v2 per Jens' comment.

-- 
Ammar Faizi

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Jens Axboe]

On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Hi Jens,
> 
> io_uring uapi updates:
> 
> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>    synced 1:1 into liburing's io_uring.h. liburing has a configure
>    check to detect the need for linux/time_types.h (Stefan).
> 
> [...]

Applied, thanks!

[1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
      commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c

Best regards,
-- 
Jens Axboe

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Ammar Faizi]

On 11/16/22 6:14 AM, Jens Axboe wrote:
> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>
>> Hi Jens,
>>
>> io_uring uapi updates:
>>
>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>     synced 1:1 into liburing's io_uring.h. liburing has a configure
>>     check to detect the need for linux/time_types.h (Stefan).
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>        commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c

Jens, please drop this commit. It breaks the build:

All errors (new ones prefixed by >>):

    In file included from <command-line>:
>> ./usr/include/linux/io_uring.h:654:41: error: field 'timeout' has incomplete type
      654 |         struct __kernel_timespec        timeout;
          |                                         ^~~~~~~

-- 
Ammar Faizi

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Ammar Faizi]

On 11/16/22 1:34 PM, Ammar Faizi wrote:
> On 11/16/22 6:14 AM, Jens Axboe wrote:
>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>
>>> Hi Jens,
>>>
>>> io_uring uapi updates:
>>>
>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>     synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>     check to detect the need for linux/time_types.h (Stefan).
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>        commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
> 
> Jens, please drop this commit. It breaks the build:
> 
> All errors (new ones prefixed by >>):
> 
>     In file included from <command-line>:
>>> ./usr/include/linux/io_uring.h:654:41: error: field 'timeout' has incomplete type
>       654 |         struct __kernel_timespec        timeout;
>           |                                         ^~~~~~~

https://lore.kernel.org/r/202211161421.AfP10hq6-lkp@intel.com

-- 
Ammar Faizi

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Jens Axboe]

On 11/15/22 11:34 PM, Ammar Faizi wrote:
> On 11/16/22 6:14 AM, Jens Axboe wrote:
>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>
>>> Hi Jens,
>>>
>>> io_uring uapi updates:
>>>
>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>     synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>     check to detect the need for linux/time_types.h (Stefan).
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>        commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
> 
> Jens, please drop this commit. It breaks the build:

Dropped - please actually build your patches, or make it clear that
they were not built at all. None of these 2 patches were any good.

-- 
Jens Axboe

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Stefan Metzmacher]

Am 16.11.22 um 14:50 schrieb Jens Axboe:
> On 11/15/22 11:34 PM, Ammar Faizi wrote:
>> On 11/16/22 6:14 AM, Jens Axboe wrote:
>>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>>
>>>> Hi Jens,
>>>>
>>>> io_uring uapi updates:
>>>>
>>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>>      synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>>      check to detect the need for linux/time_types.h (Stefan).
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>>         commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
>>
>> Jens, please drop this commit. It breaks the build:
> 
> Dropped - please actually build your patches, or make it clear that
> they were not built at all. None of these 2 patches were any good.

Is it tools/testing/selftests/net/io_uring_zerocopy_tx.c that doesn't build?
and needs a '#define HAVE_LINUX_TIME_TYPES_H 1'

BTW, the original commit I posted was here:
https://lore.kernel.org/io-uring/c7782923deeb4016f2ac2334bc558921e8d91a67.1666605446.git.metze@samba.org/

What's the magic to compile tools/testing/selftests/net/io_uring_zerocopy_tx.c ?

My naive tries both fail (even without my patch):
All other files including any io_uring.h build and the patch was also included
in a branch where I build a complete working kernel with 'make -j33 bindeb-pkg'

metze@SERNOX19:~/devel/kernel/linux-4.4$ LANG=C make $(find io_uring/*.c fs/file_table.c fs/exec.c kernel/exit.c kernel/fork.c net/socket.c  net/unix/scm.c 
security/selinux/hooks.c security/smack/smack_lsm.c tools/testing/selftests/net/io_uring_zerocopy_tx.c | sed -e 's!\.c$!.o!') EXTRA_CFLAGS="-Wfatal-errors"
   UPD     include/config/kernel.release
   UPD     include/generated/utsrelease.h
   CALL    scripts/checksyscalls.sh
   DESCEND objtool
   DESCEND bpf/resolve_btfids
   CC      fs/file_table.o
   CC      fs/exec.o
   CC      io_uring/advise.o
   CC      io_uring/cancel.o
   CC      io_uring/epoll.o
   CC      io_uring/fdinfo.o
   CC      io_uring/filetable.o
   CC      io_uring/fs.o
   CC      io_uring/io_uring.o
   CC      io_uring/io-wq.o
   CC      io_uring/kbuf.o
   CC      io_uring/msg_ring.o
   CC      io_uring/net.o
   CC      io_uring/nop.o
   CC      io_uring/notif.o
   CC      io_uring/opdef.o
   CC      io_uring/openclose.o
   CC      io_uring/poll.o
   CC      io_uring/rsrc.o
   CC      io_uring/rw.o
   CC      io_uring/splice.o
   CC      io_uring/sqpoll.o
   CC      io_uring/statx.o
   CC      io_uring/sync.o
   CC      io_uring/tctx.o
   CC      io_uring/timeout.o
   CC      io_uring/uring_cmd.o
   CC      io_uring/xattr.o
   CC      kernel/exit.o
   CC      kernel/fork.o
   CC      net/socket.o
   CC      net/unix/scm.o
   CC      security/selinux/hooks.o
   CC      security/smack/smack_lsm.o
   CC      tools/testing/selftests/net/io_uring_zerocopy_tx.o
tools/testing/selftests/net/io_uring_zerocopy_tx.c:3:10: fatal error: assert.h: No such file or directory
     3 | #include <assert.h>
       |          ^~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.build:258: tools/testing/selftests/net/io_uring_zerocopy_tx.o] Error 1
make: *** [Makefile:1997: .] Error 2


metze@SERNOX19:~/devel/kernel/linux-4.4/tools/testing/selftests/net$ make io_uring_zerocopy_tx.o
gcc -Wall -Wl,--no-as-needed -O2 -g -I../../../../usr/include/ -isystem /home/metze/devel/kernel/linux-4.4/tools/testing/selftests/../../../usr/include   -c -o 
io_uring_zerocopy_tx.o io_uring_zerocopy_tx.c
io_uring_zerocopy_tx.c: In function ‘io_uring_prep_sendzc’:
io_uring_zerocopy_tx.c:287:30: error: ‘IORING_OP_SEND_ZC’ undeclared (first use in this function); did you mean ‘IORING_OP_SEND’?
   287 |         sqe->opcode = (__u8) IORING_OP_SEND_ZC;
       |                              ^~~~~~~~~~~~~~~~~
       |                              IORING_OP_SEND
io_uring_zerocopy_tx.c:287:30: note: each undeclared identifier is reported only once for each function it appears in
io_uring_zerocopy_tx.c: In function ‘do_tx’:
io_uring_zerocopy_tx.c:407:56: error: ‘IORING_RECVSEND_FIXED_BUF’ undeclared (first use in this function)
   407 |                                         sqe->ioprio |= IORING_RECVSEND_FIXED_BUF;
       |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~
io_uring_zerocopy_tx.c:429:42: error: ‘IORING_CQE_F_NOTIF’ undeclared (first use in this function); did you mean ‘IORING_CQE_F_MORE’?
   429 |                         if (cqe->flags & IORING_CQE_F_NOTIF) {
       |                                          ^~~~~~~~~~~~~~~~~~
       |                                          IORING_CQE_F_MORE
make: *** [<eingebaut>: io_uring_zerocopy_tx.o] Fehler 1

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Jens Axboe]

On 11/16/22 7:22 AM, Stefan Metzmacher wrote:
> Am 16.11.22 um 14:50 schrieb Jens Axboe:
>> On 11/15/22 11:34 PM, Ammar Faizi wrote:
>>> On 11/16/22 6:14 AM, Jens Axboe wrote:
>>>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>>>
>>>>> Hi Jens,
>>>>>
>>>>> io_uring uapi updates:
>>>>>
>>>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>>> ???? synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>>> ???? check to detect the need for linux/time_types.h (Stefan).
>>>>>
>>>>> [...]
>>>>
>>>> Applied, thanks!
>>>>
>>>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>>> ??????? commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
>>>
>>> Jens, please drop this commit. It breaks the build:
>>
>> Dropped - please actually build your patches, or make it clear that
>> they were not built at all. None of these 2 patches were any good.
> 
> Is it tools/testing/selftests/net/io_uring_zerocopy_tx.c that doesn't build?

Honestly not sure, but saw a few reports come in. Here's the one from
linux-next:

https://lore.kernel.org/all/20221116123556.79a7bbd8@canb.auug.org.au/

> and needs a '#define HAVE_LINUX_TIME_TYPES_H 1'
> 
> BTW, the original commit I posted was here:
> https://lore.kernel.org/io-uring/c7782923deeb4016f2ac2334bc558921e8d91a67.1666605446.git.metze@samba.org/
> 
> What's the magic to compile tools/testing/selftests/net/io_uring_zerocopy_tx.c ?

Some variant of make kselftests-foo?

> My naive tries both fail (even without my patch):

Mine does too, in various other tests. Stephen?

-- 
Jens Axboe

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Stefan Metzmacher]

Am 16.11.22 um 20:46 schrieb Jens Axboe:
> On 11/16/22 7:22 AM, Stefan Metzmacher wrote:
>> Am 16.11.22 um 14:50 schrieb Jens Axboe:
>>> On 11/15/22 11:34 PM, Ammar Faizi wrote:
>>>> On 11/16/22 6:14 AM, Jens Axboe wrote:
>>>>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>>>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>>>>
>>>>>> Hi Jens,
>>>>>>
>>>>>> io_uring uapi updates:
>>>>>>
>>>>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>>>> ???? synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>>>> ???? check to detect the need for linux/time_types.h (Stefan).
>>>>>>
>>>>>> [...]
>>>>>
>>>>> Applied, thanks!
>>>>>
>>>>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>>>> ??????? commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
>>>>
>>>> Jens, please drop this commit. It breaks the build:
>>>
>>> Dropped - please actually build your patches, or make it clear that
>>> they were not built at all. None of these 2 patches were any good.
>>
>> Is it tools/testing/selftests/net/io_uring_zerocopy_tx.c that doesn't build?
> 
> Honestly not sure, but saw a few reports come in. Here's the one from
> linux-next:
> 
> https://lore.kernel.org/all/20221116123556.79a7bbd8@canb.auug.org.au/

Yes, but the output is pretty useless as it doesn't show what
.c file and what command is failing.

>> and needs a '#define HAVE_LINUX_TIME_TYPES_H 1'

Just guessing, but adding this into the commit has a chance to work...

--- a/tools/testing/selftests/net/io_uring_zerocopy_tx.c
+++ b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
@@ -15,6 +15,7 @@
  #include <arpa/inet.h>
  #include <linux/errqueue.h>
  #include <linux/if_packet.h>
+#define HAVE_LINUX_TIME_TYPES_H 1
  #include <linux/io_uring.h>
  #include <linux/ipv6.h>
  #include <linux/socket.h>

>> BTW, the original commit I posted was here:
>> https://lore.kernel.org/io-uring/c7782923deeb4016f2ac2334bc558921e8d91a67.1666605446.git.metze@samba.org/
>>
>> What's the magic to compile tools/testing/selftests/net/io_uring_zerocopy_tx.c ?
> 
> Some variant of make kselftests-foo?
> 
>> My naive tries both fail (even without my patch):
> 
> Mine does too, in various other tests. Stephen?

Pavel, as you created that file, do you remember how you build it?

metze

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Pavel Begunkov]

On 11/16/22 20:03, Stefan Metzmacher wrote:
> Am 16.11.22 um 20:46 schrieb Jens Axboe:
>> On 11/16/22 7:22 AM, Stefan Metzmacher wrote:
>>> Am 16.11.22 um 14:50 schrieb Jens Axboe:
>>>> On 11/15/22 11:34 PM, Ammar Faizi wrote:
>>>>> On 11/16/22 6:14 AM, Jens Axboe wrote:
>>>>>> On Wed, 16 Nov 2022 04:29:51 +0700, Ammar Faizi wrote:
>>>>>>> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>>>>>>>
>>>>>>> Hi Jens,
>>>>>>>
>>>>>>> io_uring uapi updates:
>>>>>>>
>>>>>>> 1) Don't force linux/time_types.h for userspace. Linux's io_uring.h is
>>>>>>> ???? synced 1:1 into liburing's io_uring.h. liburing has a configure
>>>>>>> ???? check to detect the need for linux/time_types.h (Stefan).
>>>>>>>
>>>>>>> [...]
>>>>>>
>>>>>> Applied, thanks!
>>>>>>
>>>>>> [1/2] io_uring: uapi: Don't force linux/time_types.h for userspace
>>>>>> ??????? commit: 958bfdd734b6074ba88ee3abc69d0053e26b7b9c
>>>>>
>>>>> Jens, please drop this commit. It breaks the build:
>>>>
>>>> Dropped - please actually build your patches, or make it clear that
>>>> they were not built at all. None of these 2 patches were any good.
>>>
>>> Is it tools/testing/selftests/net/io_uring_zerocopy_tx.c that doesn't build?
>>
>> Honestly not sure, but saw a few reports come in. Here's the one from
>> linux-next:
>>
>> https://lore.kernel.org/all/20221116123556.79a7bbd8@canb.auug.org.au/
> 
> Yes, but the output is pretty useless as it doesn't show what
> .c file and what command is failing.
> 
>>> and needs a '#define HAVE_LINUX_TIME_TYPES_H 1'
> 
> Just guessing, but adding this into the commit has a chance to work...
> 
> --- a/tools/testing/selftests/net/io_uring_zerocopy_tx.c
> +++ b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
> @@ -15,6 +15,7 @@
>   #include <arpa/inet.h>
>   #include <linux/errqueue.h>
>   #include <linux/if_packet.h>
> +#define HAVE_LINUX_TIME_TYPES_H 1
>   #include <linux/io_uring.h>
>   #include <linux/ipv6.h>
>   #include <linux/socket.h>
> 
>>> BTW, the original commit I posted was here:
>>> https://lore.kernel.org/io-uring/c7782923deeb4016f2ac2334bc558921e8d91a67.1666605446.git.metze@samba.org/
>>>
>>> What's the magic to compile tools/testing/selftests/net/io_uring_zerocopy_tx.c ?
>>
>> Some variant of make kselftests-foo?
>>
>>> My naive tries both fail (even without my patch):
>>
>> Mine does too, in various other tests. Stephen?
> 
> Pavel, as you created that file, do you remember how you build it?

make headers_install
make -C tools/testing/selftests/net/


IIRC, it uses system uapi headers and apparently yours don't have
IORING_CQE_F_NOTIF, etc. And I don't think it uses the right Makefile,
so -C executes it from the selftest/net folder.

-- 
Pavel Begunkov

Re: (subset) [PATCH v1 0/2] io_uring uapi updates

[Stefan Metzmacher]

Hi Jens,

>> and needs a '#define HAVE_LINUX_TIME_TYPES_H 1'
>>
>> BTW, the original commit I posted was here:
>> https://lore.kernel.org/io-uring/c7782923deeb4016f2ac2334bc558921e8d91a67.1666605446.git.metze@samba.org/

I'll push a better version soon, it inverts the ifdef logic like this:

--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -10,7 +10,15 @@

  #include <linux/fs.h>
  #include <linux/types.h>
+/*
+ * this file is shared with liburing and that has to autodetect
+ * if linux/time_types.h is available or not, it can
+ * define UAPI_LINUX_IO_URING_H_SKIP_LINUX_TIME_TYPES_H
+ * if linux/time_types.h is not available
+ */
+#ifndef UAPI_LINUX_IO_URING_H_SKIP_LINUX_TIME_TYPES_H
  #include <linux/time_types.h>
+#endif

It also means that projects without liburing usage are not affected.
And developers only have to care if they want to build on legacy systems
without time_types.h

Once that's accepted into the kernel I'll adjust the logic in liburing

Does that sound like a plan?

metze