On 5/3/23 13:49, Jens Axboe wrote:
On 5/3/23 2:58?AM, Ben Noordhuis wrote:
On Tue, May 2, 2023 at 2:51?PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
On 5/1/23 19:52, Ben Noordhuis wrote:
Libuv recently started using it so there is at least one consumer now.
It was rather deprecated because io_uring controlling epoll is a bad
idea and should never be used. One reason is that it means libuv still
uses epoll but not io_uring, and so the use of io_uring wouldn't seem
to make much sense. You're welcome to prove me wrong on that, why libuv
decided to use a deprecated API in the first place?
Sorry, but the warning is going to stay and libuv should revert the use
of epol_ctl requests.
Why use a deprecated API? Because it was only recently deprecated.
Distro kernels don't warn about it yet. I only found out because of
kernel source code spelunking.
Why combine io_uring and epoll? Libuv uses level-triggered I/O for
reasons (I can go into detail but they're not material) so it's very
profitable to batch epoll_ctl syscalls; it's the epoll_ctlv() syscall
people have been asking for since practically forever.
Why not switch to io_uring wholesale? Libuv can't drop support for
epoll because of old kernels, and io_uring isn't always clearly faster
than epoll in the first place.
As to the warning: according to the commit that introduced it, it was
added because no one was using IORING_OP_EPOLL_CTL. Well, now someone
is using it. Saying it's a bad API feels like post-hoc
rationalization. I kindly ask you merge this patch. I'd be happy to
keep an eye on io_uring/epoll.c if you're worried about maintenance
burden.
This is obviously mostly our fault, as the deprecation patch should've
obviously been backported to stable. Just adding it to the current
kernel defeated the purpose, as it added a long period where older
kernels quite happily accepted epoll use cases.
So I do agree, the only sane course of action here is to un-deprecate
it.
nack, keeping piling rubbish is not a great course of action at all.
Has libuv already released it? Because it seems the patches were
just merged.
--
Pavel Begunkov
