* [PATCH 1/1] io_uring/poll: fix POLLERR handling
@ 2025-07-14 10:59 Pavel Begunkov
2025-07-14 14:56 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2025-07-14 10:59 UTC (permalink / raw)
To: io-uring; +Cc: asml.silence, dw
8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
is a little dirty hack that
1) wrongfully assumes that POLLERR equals to a failed request, which
breaks all POLLERR users, e.g. all error queue recv interfaces.
2) deviates the connection request behaviour from connect(2), and
3) racy and solved at a wrong level.
Nothing can be done with 2) now, and 3) is beyond the scope of the
patch. At least solve 1) by moving the hack out of generic poll handling
into io_connect().
Cc: stable@vger.kernel.org
Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
io_uring/net.c | 4 +++-
io_uring/poll.c | 2 --
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 43a43522f406..e2213e4d9420 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
int io_connect(struct io_kiocb *req, unsigned int issue_flags)
{
+ struct poll_table_struct pt = { ._key = EPOLLERR };
struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
struct io_async_msghdr *io = req->async_data;
unsigned file_flags;
int ret;
bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
- if (unlikely(req->flags & REQ_F_FAIL)) {
+ ret = vfs_poll(req->file, &pt) & req->apoll_events;
+ if (ret & EPOLLERR) {
ret = -ECONNRESET;
goto out;
}
diff --git a/io_uring/poll.c b/io_uring/poll.c
index 0526062e2f81..20e9b46a4adf 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -273,8 +273,6 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
return IOU_POLL_REISSUE;
}
}
- if (unlikely(req->cqe.res & EPOLLERR))
- req_set_fail(req);
if (req->apoll_events & EPOLLONESHOT)
return IOU_POLL_DONE;
--
2.49.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-14 10:59 [PATCH 1/1] io_uring/poll: fix POLLERR handling Pavel Begunkov
@ 2025-07-14 14:56 ` Jens Axboe
2025-07-14 15:30 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-07-14 14:56 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: dw
On 7/14/25 4:59 AM, Pavel Begunkov wrote:
> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
> is a little dirty hack that
> 1) wrongfully assumes that POLLERR equals to a failed request, which
> breaks all POLLERR users, e.g. all error queue recv interfaces.
> 2) deviates the connection request behaviour from connect(2), and
> 3) racy and solved at a wrong level.
>
> Nothing can be done with 2) now, and 3) is beyond the scope of the
> patch. At least solve 1) by moving the hack out of generic poll handling
> into io_connect().
>
> Cc: stable@vger.kernel.org
> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
> io_uring/net.c | 4 +++-
> io_uring/poll.c | 2 --
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/io_uring/net.c b/io_uring/net.c
> index 43a43522f406..e2213e4d9420 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>
> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
> {
> + struct poll_table_struct pt = { ._key = EPOLLERR };
> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
> struct io_async_msghdr *io = req->async_data;
> unsigned file_flags;
> int ret;
> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>
> - if (unlikely(req->flags & REQ_F_FAIL)) {
> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
> + if (ret & EPOLLERR) {
> ret = -ECONNRESET;
> goto out;
Is this req->apoll_events masking necessary or useful?
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-14 14:56 ` Jens Axboe
@ 2025-07-14 15:30 ` Pavel Begunkov
2025-07-14 17:51 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2025-07-14 15:30 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: dw
On 7/14/25 15:56, Jens Axboe wrote:
> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>> is a little dirty hack that
>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>> 2) deviates the connection request behaviour from connect(2), and
>> 3) racy and solved at a wrong level.
>>
>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>> patch. At least solve 1) by moving the hack out of generic poll handling
>> into io_connect().
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>> io_uring/net.c | 4 +++-
>> io_uring/poll.c | 2 --
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/io_uring/net.c b/io_uring/net.c
>> index 43a43522f406..e2213e4d9420 100644
>> --- a/io_uring/net.c
>> +++ b/io_uring/net.c
>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>
>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>> {
>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>> struct io_async_msghdr *io = req->async_data;
>> unsigned file_flags;
>> int ret;
>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>
>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>> + if (ret & EPOLLERR) {
>> ret = -ECONNRESET;
>> goto out;
>
> Is this req->apoll_events masking necessary or useful?
good point, shouldn't be here
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-14 15:30 ` Pavel Begunkov
@ 2025-07-14 17:51 ` Jens Axboe
2025-07-14 20:45 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-07-14 17:51 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: dw
On 7/14/25 9:30 AM, Pavel Begunkov wrote:
> On 7/14/25 15:56, Jens Axboe wrote:
>> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>> is a little dirty hack that
>>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>>> 2) deviates the connection request behaviour from connect(2), and
>>> 3) racy and solved at a wrong level.
>>>
>>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>>> patch. At least solve 1) by moving the hack out of generic poll handling
>>> into io_connect().
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>> ---
>>> io_uring/net.c | 4 +++-
>>> io_uring/poll.c | 2 --
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>> index 43a43522f406..e2213e4d9420 100644
>>> --- a/io_uring/net.c
>>> +++ b/io_uring/net.c
>>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>>> {
>>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>>> struct io_async_msghdr *io = req->async_data;
>>> unsigned file_flags;
>>> int ret;
>>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>>> + if (ret & EPOLLERR) {
>>> ret = -ECONNRESET;
>>> goto out;
>>
>> Is this req->apoll_events masking necessary or useful?
>
> good point, shouldn't be here
Do you want to send a v2?
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-14 17:51 ` Jens Axboe
@ 2025-07-14 20:45 ` Jens Axboe
2025-07-15 9:06 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-07-14 20:45 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: dw
On 7/14/25 11:51 AM, Jens Axboe wrote:
> On 7/14/25 9:30 AM, Pavel Begunkov wrote:
>> On 7/14/25 15:56, Jens Axboe wrote:
>>> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>>>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>> is a little dirty hack that
>>>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>>>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>>>> 2) deviates the connection request behaviour from connect(2), and
>>>> 3) racy and solved at a wrong level.
>>>>
>>>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>>>> patch. At least solve 1) by moving the hack out of generic poll handling
>>>> into io_connect().
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>> ---
>>>> io_uring/net.c | 4 +++-
>>>> io_uring/poll.c | 2 --
>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>>> index 43a43522f406..e2213e4d9420 100644
>>>> --- a/io_uring/net.c
>>>> +++ b/io_uring/net.c
>>>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>>>> {
>>>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>>>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>>>> struct io_async_msghdr *io = req->async_data;
>>>> unsigned file_flags;
>>>> int ret;
>>>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>>>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>>>> + if (ret & EPOLLERR) {
>>>> ret = -ECONNRESET;
>>>> goto out;
>>>
>>> Is this req->apoll_events masking necessary or useful?
>>
>> good point, shouldn't be here
>
> Do you want to send a v2?
Actually I think it can be improved/fixed further. If POLLIN is set, we
should let it go through. And there should not be a need to call
vfs_poll() unless ->in_progress is already set. Something ala:
if (connect->in_progress) {
struct poll_table_struct pt = { ._key = EPOLLERR };
__poll_t mask;
mask = vfs_poll(req->file, &pt);
if ((mask & (EPOLLERR | EPOLLIN)) == EPOLLERR) {
ret = -ECONNRESET;
goto out;
}
}
avoiding what I'm also guessing will be a sparse complaint on the 'ret'
vs __poll_t use as well. Not that poll.c is free of those already, but
new ones will surely bother me via email.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-14 20:45 ` Jens Axboe
@ 2025-07-15 9:06 ` Pavel Begunkov
2025-07-15 18:13 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2025-07-15 9:06 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: dw
On 7/14/25 21:45, Jens Axboe wrote:
> On 7/14/25 11:51 AM, Jens Axboe wrote:
>> On 7/14/25 9:30 AM, Pavel Begunkov wrote:
>>> On 7/14/25 15:56, Jens Axboe wrote:
>>>> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>>>>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>> is a little dirty hack that
>>>>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>>>>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>>>>> 2) deviates the connection request behaviour from connect(2), and
>>>>> 3) racy and solved at a wrong level.
>>>>>
>>>>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>>>>> patch. At least solve 1) by moving the hack out of generic poll handling
>>>>> into io_connect().
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>> ---
>>>>> io_uring/net.c | 4 +++-
>>>>> io_uring/poll.c | 2 --
>>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>>>> index 43a43522f406..e2213e4d9420 100644
>>>>> --- a/io_uring/net.c
>>>>> +++ b/io_uring/net.c
>>>>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>>>>> {
>>>>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>>>>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>>>>> struct io_async_msghdr *io = req->async_data;
>>>>> unsigned file_flags;
>>>>> int ret;
>>>>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>>>>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>>>>> + if (ret & EPOLLERR) {
>>>>> ret = -ECONNRESET;
>>>>> goto out;
>>>>
>>>> Is this req->apoll_events masking necessary or useful?
>>>
>>> good point, shouldn't be here
>>
>> Do you want to send a v2?
>
> Actually I think it can be improved/fixed further. If POLLIN is set, we
How is it related to POLLIN?
> should let it go through. And there should not be a need to call
> vfs_poll() unless ->in_progress is already set. Something ala:
In any case, v1 doesn't seem to work, so needs to be done differently.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-15 9:06 ` Pavel Begunkov
@ 2025-07-15 18:13 ` Jens Axboe
2025-07-15 19:12 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-07-15 18:13 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: dw
On 7/15/25 3:06 AM, Pavel Begunkov wrote:
> On 7/14/25 21:45, Jens Axboe wrote:
>> On 7/14/25 11:51 AM, Jens Axboe wrote:
>>> On 7/14/25 9:30 AM, Pavel Begunkov wrote:
>>>> On 7/14/25 15:56, Jens Axboe wrote:
>>>>> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>>>>>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>>> is a little dirty hack that
>>>>>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>>>>>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>>>>>> 2) deviates the connection request behaviour from connect(2), and
>>>>>> 3) racy and solved at a wrong level.
>>>>>>
>>>>>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>>>>>> patch. At least solve 1) by moving the hack out of generic poll handling
>>>>>> into io_connect().
>>>>>>
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>>> ---
>>>>>> io_uring/net.c | 4 +++-
>>>>>> io_uring/poll.c | 2 --
>>>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>>>>> index 43a43522f406..e2213e4d9420 100644
>>>>>> --- a/io_uring/net.c
>>>>>> +++ b/io_uring/net.c
>>>>>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>>>>>> {
>>>>>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>>>>>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>>>>>> struct io_async_msghdr *io = req->async_data;
>>>>>> unsigned file_flags;
>>>>>> int ret;
>>>>>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>>>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>>>>>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>>>>>> + if (ret & EPOLLERR) {
>>>>>> ret = -ECONNRESET;
>>>>>> goto out;
>>>>>
>>>>> Is this req->apoll_events masking necessary or useful?
>>>>
>>>> good point, shouldn't be here
>>>
>>> Do you want to send a v2?
>>
>> Actually I think it can be improved/fixed further. If POLLIN is set, we
>
> How is it related to POLLIN?
Gah POLLOUT of course.
>> should let it go through. And there should not be a need to call
>> vfs_poll() unless ->in_progress is already set. Something ala:
>
> In any case, v1 doesn't seem to work, so needs to be done differently.
RIght, that's what I aluded to in the "improved/fixed" further. FWIW, I
did dig out the old test case I wrote for this and added it to liburing
as well. So should have somewhat better coverage now.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-15 18:13 ` Jens Axboe
@ 2025-07-15 19:12 ` Jens Axboe
2025-07-16 16:18 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2025-07-15 19:12 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: dw
On 7/15/25 12:13 PM, Jens Axboe wrote:
> On 7/15/25 3:06 AM, Pavel Begunkov wrote:
>> On 7/14/25 21:45, Jens Axboe wrote:
>>> On 7/14/25 11:51 AM, Jens Axboe wrote:
>>>> On 7/14/25 9:30 AM, Pavel Begunkov wrote:
>>>>> On 7/14/25 15:56, Jens Axboe wrote:
>>>>>> On 7/14/25 4:59 AM, Pavel Begunkov wrote:
>>>>>>> 8c8492ca64e7 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>>>> is a little dirty hack that
>>>>>>> 1) wrongfully assumes that POLLERR equals to a failed request, which
>>>>>>> breaks all POLLERR users, e.g. all error queue recv interfaces.
>>>>>>> 2) deviates the connection request behaviour from connect(2), and
>>>>>>> 3) racy and solved at a wrong level.
>>>>>>>
>>>>>>> Nothing can be done with 2) now, and 3) is beyond the scope of the
>>>>>>> patch. At least solve 1) by moving the hack out of generic poll handling
>>>>>>> into io_connect().
>>>>>>>
>>>>>>> Cc: stable@vger.kernel.org
>>>>>>> Fixes: 8c8492ca64e79 ("io_uring/net: don't retry connect operation on EPOLLERR")
>>>>>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>>>>>> ---
>>>>>>> io_uring/net.c | 4 +++-
>>>>>>> io_uring/poll.c | 2 --
>>>>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>>>>>> index 43a43522f406..e2213e4d9420 100644
>>>>>>> --- a/io_uring/net.c
>>>>>>> +++ b/io_uring/net.c
>>>>>>> @@ -1732,13 +1732,15 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>>>> int io_connect(struct io_kiocb *req, unsigned int issue_flags)
>>>>>>> {
>>>>>>> + struct poll_table_struct pt = { ._key = EPOLLERR };
>>>>>>> struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
>>>>>>> struct io_async_msghdr *io = req->async_data;
>>>>>>> unsigned file_flags;
>>>>>>> int ret;
>>>>>>> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>>>>> - if (unlikely(req->flags & REQ_F_FAIL)) {
>>>>>>> + ret = vfs_poll(req->file, &pt) & req->apoll_events;
>>>>>>> + if (ret & EPOLLERR) {
>>>>>>> ret = -ECONNRESET;
>>>>>>> goto out;
>>>>>>
>>>>>> Is this req->apoll_events masking necessary or useful?
>>>>>
>>>>> good point, shouldn't be here
>>>>
>>>> Do you want to send a v2?
>>>
>>> Actually I think it can be improved/fixed further. If POLLIN is set, we
>>
>> How is it related to POLLIN?
>
> Gah POLLOUT of course.
>
>>> should let it go through. And there should not be a need to call
>>> vfs_poll() unless ->in_progress is already set. Something ala:
>>
>> In any case, v1 doesn't seem to work, so needs to be done differently.
>
> RIght, that's what I aluded to in the "improved/fixed" further. FWIW, I
> did dig out the old test case I wrote for this and added it to liburing
> as well. So should have somewhat better coverage now.
How about:
if (connect->in_progress) {
struct poll_table_struct pt = { ._key = EPOLLERR };
if (vfs_poll(req->file, &pt) & EPOLLERR)
goto get_sock_err;
}
with get_sock_err being where we do sock_error()?
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling
2025-07-15 19:12 ` Jens Axboe
@ 2025-07-16 16:18 ` Pavel Begunkov
0 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-07-16 16:18 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 7/15/25 20:12, Jens Axboe wrote:
...>>> How is it related to POLLIN?
>>
>> Gah POLLOUT of course.
>>
>>>> should let it go through. And there should not be a need to call
>>>> vfs_poll() unless ->in_progress is already set. Something ala:
>>>
>>> In any case, v1 doesn't seem to work, so needs to be done differently.
>>
>> RIght, that's what I aluded to in the "improved/fixed" further. FWIW, I
>> did dig out the old test case I wrote for this and added it to liburing
>> as well. So should have somewhat better coverage now.
>
> How about:
>
> if (connect->in_progress) {
> struct poll_table_struct pt = { ._key = EPOLLERR };
>
> if (vfs_poll(req->file, &pt) & EPOLLERR)
> goto get_sock_err;
> }
>
> with get_sock_err being where we do sock_error()?
Seems to work. sock_error() can also race, but I don't want to
waste any more time on it.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-16 16:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-14 10:59 [PATCH 1/1] io_uring/poll: fix POLLERR handling Pavel Begunkov
2025-07-14 14:56 ` Jens Axboe
2025-07-14 15:30 ` Pavel Begunkov
2025-07-14 17:51 ` Jens Axboe
2025-07-14 20:45 ` Jens Axboe
2025-07-15 9:06 ` Pavel Begunkov
2025-07-15 18:13 ` Jens Axboe
2025-07-15 19:12 ` Jens Axboe
2025-07-16 16:18 ` Pavel Begunkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox