Re: [PATCH 1/1] io_uring/poll: fix POLLERR handling

[Pavel Begunkov <asml.silence@gmail.com>]

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