| Message ID | cover.1662116617.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | zerocopy send API changes | expand |
On 9/2/22 6:12 PM, Pavel Begunkov wrote: > Fix up helpers and tests to match API changes and also add some more tests. > > Pavel Begunkov (4): > tests: verify that send addr is copied when async > zc: adjust sendzc to the simpler uapi > test: test iowq zc sends > examples: adjust zc bench to the new uapi Hi Pavel, Patch #2 and #3 are broken, but after applying patch #4, everything builds just fine. Please resend and avoid breakage in the middle. Thanks! -------------------------------------------------------------------------------- Patch #2 send-zerocopy.c:152:9: error: call to undeclared function 'io_uring_register_notifications'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] ret = io_uring_register_notifications(&ring, 1, b); ^ send-zerocopy.c:152:9: note: did you mean 'io_uring_register_restrictions'? ../src/include/liburing.h:181:5: note: 'io_uring_register_restrictions' declared here int io_uring_register_restrictions(struct io_uring *ring, ^ send-zerocopy.c:185:16: error: use of undeclared identifier 'IORING_RECVSEND_NOTIF_FLUSH' zc_flags |= IORING_RECVSEND_NOTIF_FLUSH; ^ send-zerocopy.c:194:5: error: call to undeclared function 'io_uring_prep_sendzc_fixed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] io_uring_prep_sendzc_fixed(sqe, fd, payload, ^ send-zerocopy.c:194:5: note: did you mean 'io_uring_prep_read_fixed'? ../src/include/liburing.h:405:20: note: 'io_uring_prep_read_fixed' declared here static inline void io_uring_prep_read_fixed(struct io_uring_sqe *sqe, int fd, ^ send-zerocopy.c:199:5: error: call to undeclared function 'io_uring_prep_sendzc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] io_uring_prep_sendzc(sqe, fd, payload, ^ send-zerocopy.c:199:5: note: did you mean 'io_uring_prep_send_zc'? ../src/include/liburing.h:701:20: note: 'io_uring_prep_send_zc' declared here static inline void io_uring_prep_send_zc(struct io_uring_sqe *sqe, int sockfd, ^ send-zerocopy.c:260:9: error: call to undeclared function 'io_uring_unregister_notifications'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] ret = io_uring_unregister_notifications(&ring); ^ send-zerocopy.c:260:9: note: did you mean 'io_uring_register_restrictions'? ../src/include/liburing.h:181:5: note: 'io_uring_register_restrictions' declared here int io_uring_register_restrictions(struct io_uring *ring, ^ 5 errors generated. make[1]: *** [Makefile:36: send-zerocopy] Error 1 make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory '/home/runner/work/liburing/liburing/examples' make: *** [Makefile:12: all] Error 2 Error: Process completed with exit code 2. -------------------------------------------------------------------------------- Patch #3: send-zerocopy.c: In function ‘do_tx’: send-zerocopy.c:152:23: error: implicit declaration of function ‘io_uring_register_notifications’; did you mean ‘io_uring_register_restrictions’? [-Werror=implicit-function-declaration] 152 | ret = io_uring_register_notifications(&ring, 1, b); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | io_uring_register_restrictions send-zerocopy.c:185:37: error: ‘IORING_RECVSEND_NOTIF_FLUSH’ undeclared (first use in this function); did you mean ‘IORING_RECVSEND_POLL_FIRST’? 185 | zc_flags |= IORING_RECVSEND_NOTIF_FLUSH; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ | IORING_RECVSEND_POLL_FIRST send-zerocopy.c:185:37: note: each undeclared identifier is reported only once for each function it appears in send-zerocopy.c:194:33: error: implicit declaration of function ‘io_uring_prep_sendzc_fixed’; did you mean ‘io_uring_prep_read_fixed’? [-Werror=implicit-function-declaration] 194 | io_uring_prep_sendzc_fixed(sqe, fd, payload, | ^~~~~~~~~~~~~~~~~~~~~~~~~~ | io_uring_prep_read_fixed send-zerocopy.c:199:33: error: implicit declaration of function ‘io_uring_prep_sendzc’; did you mean ‘io_uring_prep_send_zc’? [-Werror=implicit-function-declaration] 199 | io_uring_prep_sendzc(sqe, fd, payload, | ^~~~~~~~~~~~~~~~~~~~ | io_uring_prep_send_zc send-zerocopy.c:260:23: error: implicit declaration of function ‘io_uring_unregister_notifications’; did you mean ‘io_uring_register_restrictions’? [-Werror=implicit-function-declaration] 260 | ret = io_uring_unregister_notifications(&ring); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | io_uring_register_restrictions cc1: all warnings being treated as errors make[1]: *** [Makefile:36: send-zerocopy] Error 1 make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory '/home/runner/work/liburing/liburing/examples' make: *** [Makefile:12: all] Error 2 Error: Process completed with exit code 2.
On Fri, 2 Sep 2022 12:12:35 +0100, Pavel Begunkov wrote: > Fix up helpers and tests to match API changes and also add some more tests. > > Pavel Begunkov (4): > tests: verify that send addr is copied when async > zc: adjust sendzc to the simpler uapi > test: test iowq zc sends > examples: adjust zc bench to the new uapi > > [...] Applied, thanks! [1/4] tests: verify that send addr is copied when async commit: 54b16e6cfa489edd1f4f538ae245d98d65d42db7 [2/4] zc: adjust sendzc to the simpler uapi commit: 880a932c8ae36506b4d5040b9258a91251164589 [3/4] test: test iowq zc sends commit: 713ecf1cf9ad58ceb19893eead2b704b27367a8a [4/4] examples: adjust zc bench to the new uapi commit: 860db521db4c86a1cb5d0b672a8fba83a89f01f0 Best regards,
On 9/2/22 6:56 PM, Ammar Faizi wrote: > On 9/2/22 6:12 PM, Pavel Begunkov wrote: >> Fix up helpers and tests to match API changes and also add some more tests. >> >> Pavel Begunkov (4): >> tests: verify that send addr is copied when async >> zc: adjust sendzc to the simpler uapi >> test: test iowq zc sends >> examples: adjust zc bench to the new uapi > > Hi Pavel, > > Patch #2 and #3 are broken, but after applying patch #4, everything builds > just fine. Please resend and avoid breakage in the middle. > > Thanks! Nevermind. It's already upstream now.
On 9/2/22 6:01 AM, Ammar Faizi wrote: > On 9/2/22 6:56 PM, Ammar Faizi wrote: >> On 9/2/22 6:12 PM, Pavel Begunkov wrote: >>> Fix up helpers and tests to match API changes and also add some more tests. >>> >>> Pavel Begunkov (4): >>> tests: verify that send addr is copied when async >>> zc: adjust sendzc to the simpler uapi >>> test: test iowq zc sends >>> examples: adjust zc bench to the new uapi >> >> Hi Pavel, >> >> Patch #2 and #3 are broken, but after applying patch #4, everything builds >> just fine. Please resend and avoid breakage in the middle. >> >> Thanks! > > Nevermind. It's already upstream now. Ah shoot, how did I miss that... That's annoying.
On 9/2/22 13:03, Jens Axboe wrote: > On 9/2/22 6:01 AM, Ammar Faizi wrote: >> On 9/2/22 6:56 PM, Ammar Faizi wrote: >>> On 9/2/22 6:12 PM, Pavel Begunkov wrote: >>>> Fix up helpers and tests to match API changes and also add some more tests. >>>> >>>> Pavel Begunkov (4): >>>> tests: verify that send addr is copied when async >>>> zc: adjust sendzc to the simpler uapi >>>> test: test iowq zc sends >>>> examples: adjust zc bench to the new uapi >>> >>> Hi Pavel, >>> >>> Patch #2 and #3 are broken, but after applying patch #4, everything builds >>> just fine. Please resend and avoid breakage in the middle. >>> >>> Thanks! >> >> Nevermind. It's already upstream now. > > Ah shoot, how did I miss that... That's annoying. We can squash them into a single commit if we care about it. Don't really want do the disable + fix +e nable dancing here.
On 9/2/22 6:07 AM, Pavel Begunkov wrote: > On 9/2/22 13:03, Jens Axboe wrote: >> On 9/2/22 6:01 AM, Ammar Faizi wrote: >>> On 9/2/22 6:56 PM, Ammar Faizi wrote: >>>> On 9/2/22 6:12 PM, Pavel Begunkov wrote: >>>>> Fix up helpers and tests to match API changes and also add some more tests. >>>>> >>>>> Pavel Begunkov (4): >>>>> ??? tests: verify that send addr is copied when async >>>>> ??? zc: adjust sendzc to the simpler uapi >>>>> ??? test: test iowq zc sends >>>>> ??? examples: adjust zc bench to the new uapi >>>> >>>> Hi Pavel, >>>> >>>> Patch #2 and #3 are broken, but after applying patch #4, everything builds >>>> just fine. Please resend and avoid breakage in the middle. >>>> >>>> Thanks! >>> >>> Nevermind. It's already upstream now. >> >> Ah shoot, how did I miss that... That's annoying. > > We can squash them into a single commit if we care about it. > Don't really want do the disable + fix +e nable dancing here. It's already pushed out, so whatever is there is set in stone... Not a huge deal, but would've been nice to avoid. It's problematic when someone needs to bisect and issue and runs into a non-compiling step. Makes that process a lot more annoying, so yes we definitely do care about not introducing build breakage in a series of patches.