Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe <axboe@kernel.dk>]

On 5/3/23 7:21?AM, Pavel Begunkov wrote:
> 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@gmail.com> 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.

This is not a NAK situation. Fact is that the code is out there, and
libuv isn't the first to discover this by accident. We messed up not
getting this to stable, but I think it's a reasonable assumption that
there are likely others there as most folks run distro and/or stable
kernels and are not on the bleeding edge.

The deprecation patch was in 6.0, so anyone running kernels before that
could be using the epoll support and have no idea that it would be going
away. Outside of that, it's also quite easy to miss a single dmesg blurp
on this unless you're actively looking for it or just happen to come
across it.

Unless there are reasons beyond "I'd love to remove this code", then it
will be reinstated. We don't get to make up special rules for io_uring
code that are counter to what the kernel generally guarantees, most
notable that you cannot remove an API that is out there and in use. This
isn't really about libuv in particular, as the io_uring support there is
rather new and they could change course. It's more about other projects
out there that already have it in production.

-- 
Jens Axboe