[PATCH] io_uring: undeprecate epoll_ctl support

[PATCH] io_uring: undeprecate epoll_ctl support

[Ben Noordhuis]

Libuv recently started using it so there is at least one consumer now.

Link: https://github.com/libuv/libuv/pull/3979
---
 io_uring/epoll.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/io_uring/epoll.c b/io_uring/epoll.c
index 9aa74d2c80bc..89bff2068a19 100644
--- a/io_uring/epoll.c
+++ b/io_uring/epoll.c
@@ -25,10 +25,6 @@ int io_epoll_ctl_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_epoll *epoll = io_kiocb_to_cmd(req, struct io_epoll);
 
-	pr_warn_once("%s: epoll_ctl support in io_uring is deprecated and will "
-		     "be removed in a future Linux kernel version.\n",
-		     current->comm);
-
 	if (sqe->buf_index || sqe->splice_fd_in)
 		return -EINVAL;
 
-- 
2.37.2

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Pavel Begunkov]

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.


> Link: https://github.com/libuv/libuv/pull/3979
> ---
>   io_uring/epoll.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/io_uring/epoll.c b/io_uring/epoll.c
> index 9aa74d2c80bc..89bff2068a19 100644
> --- a/io_uring/epoll.c
> +++ b/io_uring/epoll.c
> @@ -25,10 +25,6 @@ int io_epoll_ctl_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>   {
>   	struct io_epoll *epoll = io_kiocb_to_cmd(req, struct io_epoll);
>   
> -	pr_warn_once("%s: epoll_ctl support in io_uring is deprecated and will "
> -		     "be removed in a future Linux kernel version.\n",
> -		     current->comm);
> -
>   	if (sqe->buf_index || sqe->splice_fd_in)
>   		return -EINVAL;
>   

-- 
Pavel Begunkov

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Ben Noordhuis]

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.

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe]

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.

-- 
Jens Axboe

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Pavel Begunkov]

On 5/3/23 09:58, 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.

I see. Looks it was introduced ~9 months ago, it's not as new,
but as Jens mentioned, it's really a shame it hasn't been
backported.

> 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

Well, the commit message is wrong, or rather incomplete. There was
a discussion about that, but not having users was obviously a
requirement for deprecation at the time.

> 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.

-- 
Pavel Begunkov

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Pavel Begunkov]

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.

-- 
Pavel Begunkov

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe]

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

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe]

On 5/1/23 12:52 PM, Ben Noordhuis wrote:
> Libuv recently started using it so there is at least one consumer now.
> 
> Link: https://github.com/libuv/libuv/pull/3979

If we're going to apply this, we also need to have your Signed-off-by
line added. And then I think it'd be a good idea to also add:

Cc: stable@vger.kernel.org
Fixes: 61a2732af4b0 ("io_uring: deprecate epoll_ctl support")

so it would wind up in the stable releases that have the original
deprecation patch.

-- 
Jens Axboe

[PATCH] io_uring: undeprecate epoll_ctl support

[Ben Noordhuis]

Libuv recently started using it so there is at least one consumer now.

Cc: stable@vger.kernel.org
Fixes: 61a2732af4b0 ("io_uring: deprecate epoll_ctl support")
Link: https://github.com/libuv/libuv/pull/3979
Signed-off-by: Ben Noordhuis <info@bnoordhuis.nl>
---
 io_uring/epoll.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/io_uring/epoll.c b/io_uring/epoll.c
index 9aa74d2c80bc..89bff2068a19 100644
--- a/io_uring/epoll.c
+++ b/io_uring/epoll.c
@@ -25,10 +25,6 @@ int io_epoll_ctl_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_epoll *epoll = io_kiocb_to_cmd(req, struct io_epoll);
 
-	pr_warn_once("%s: epoll_ctl support in io_uring is deprecated and will "
-		     "be removed in a future Linux kernel version.\n",
-		     current->comm);
-
 	if (sqe->buf_index || sqe->splice_fd_in)
 		return -EINVAL;
 
-- 
2.39.2

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

Hi,

New libuv is indeed in the wild now and CMake uses it, so I end up
getting
a tonne of:
```
[ 1763.697364] isc-net-0000: epoll_ctl support in io_uring is deprecated
and will be removed in a future Linux kernel version.
```

Could you confirm if this patch looks likely to land so we know if it's
OK
to backport it downstream?

Thank you!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe]

On 5/26/23 7:48 PM, Sam James wrote:
> Hi,
> 
> New libuv is indeed in the wild now and CMake uses it, so I end up
> getting
> a tonne of:
> ```
> [ 1763.697364] isc-net-0000: epoll_ctl support in io_uring is deprecated
> and will be removed in a future Linux kernel version.
> ```
> 
> Could you confirm if this patch looks likely to land so we know if it's
> OK
> to backport it downstream?

Yeah, I think this is the best plan. I was out of town for a bit
around when it got reposted and hence missed it, I'll queue it
up for this release.

-- 
Jens Axboe

Re: [PATCH] io_uring: undeprecate epoll_ctl support

[Jens Axboe]

On Sat, 06 May 2023 11:55:02 +0200, Ben Noordhuis wrote:
> Libuv recently started using it so there is at least one consumer now.
> 
> 

Applied, thanks!

[1/1] io_uring: undeprecate epoll_ctl support
      commit: 4ea0bf4b98d66a7a790abb285539f395596bae92

Best regards,
-- 
Jens Axboe