[lvc-project] [PATCH 5.10 491/491] io_uring/poll: correctly handle io_poll_add() return value on update

[Jens Axboe]

On 5/1/26 3:55 PM, Fedor Pchelkin wrote:
> On Sat, 02. May 00:50, Fedor Pchelkin wrote:
>> On Fri, 01. May 15:33, Jens Axboe wrote:
>>>>> @@ -6024,16 +6035,17 @@ static int io_poll_update(struct io_kioc
>>>>>  		if (req->poll_update.update_user_data)
>>>>>  			preq->user_data = req->poll_update.new_user_data;
>>>>>  
>>>>> -		ret2 = io_poll_add(preq, issue_flags);
>>>>> +		ret2 = __io_poll_add(preq, issue_flags);
>>>>>  		/* successfully updated, don't complete poll request */
>>>>>  		if (!ret2)
>>>>>  			goto out;
>>>>> +		preq->result = ret2;
>>>>> +
>>>>>  	}
>>>>> -	req_set_fail(preq);
>>>>> -	io_req_complete(preq, -ECANCELED);
>>>>> +	if (preq->result < 0)
>>>>> +		req_set_fail(preq);
>>>>> +	io_req_complete(preq, preq->result);
>>>
>>> This should all be handled in the fixup patch - yes this one ended up
>>> being broken, but that's why there's the followup fix.
>>>
>>> Now this is all pretty broken because some patches ended up in 5.15 and
>>> some in 5.10 and honestly I've almost lost track at this point. Sasha
>>> spotted some that were dropped in some broken commit from Greg. For
>>> 5.15, the two attached are what I recently asked for to be added. 5.10
>>> should ALWAYS get the exact same patches as 5.15, because of the whole
>>> sale backport that was done years ago. I always ask for that explicitly
>>> in the emails. But looks like that wasn't always done...
>>>
>>> 5.10 doesn't look like it ever got what is sha
>>> 349ef5d2e7bfb292e7000e6041a984ab56eccf28 in 5.15-stable, hence the fixup
>>> can be merged with queueing that backport.
>>>
>>> Sigh...
>>
>> Oh, for the fixup patch - it is in the 5.10-queue (or at least was when I
>> wrote up report today).  It was taken into consideration.  The concern
>> for the Fixes tag is resolved now by you, thanks for clarification.
>>
>> But that `if (preq->result) < 0` thing is not covered by the fixup patch.
>> This check is not correct in 5.10/5.15 because preq->result is unsigned.
>>
>> Taken that double complete is OK (I had doubts about that), the following
>> change should do the job now:
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 8b0dfea96ee0..b17a26b1b5e1 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -6006,7 +6006,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
>>  {
>>         struct io_ring_ctx *ctx = req->ctx;
>>         struct io_kiocb *preq;
>> -       int ret2, ret = 0;
>> +       int ret2 = -ECANCELED, ret = 0;
>>  
>>         io_ring_submit_lock(ctx, !(issue_flags & IO_URING_F_NONBLOCK));
>>  
>> @@ -6037,7 +6037,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
>>                 preq->result = ret2;
>>  
>>         }
>> -       if (preq->result < 0)
>> +       if (ret2 < 0)
>>                 req_set_fail(preq);
>>         io_req_complete(preq, preq->result);
>>  out:
>>
>>
>> The fixup patch may be updated with this if the changes look OK.
>>
> 
> Or maybe this one which is less hassle:
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 8b0dfea96ee0..bb01eaa9761f 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -6037,7 +6037,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags)
>                 preq->result = ret2;
>  
>         }
> -       if (preq->result < 0)
> +       if (preq->result)
>                 req_set_fail(preq);
>         io_req_complete(preq, preq->result);
>  out:

That'd be fine. Note that both your patches are white space damaged. I
can send out updated fixup patches, the above does improve them. But at
this point I don't even know what is queued up where. As far as I can
tell neither 5.10-stable or 5.15-stable have the fixup queued up yet,
even though I asked for it 10 days ago. Neither the fixup, nor the
EPOLL_URING_WAKE patch.

I feel like I spend a lot of time both on stable, and defending stable
against people that complain about it. And usually it's all been fine,
but lately it does seem like a bit of a mess :(, and as a maintainer, I
absolutely 100% need to rely on stable doing what they are asked to do,
and if patches fail, then I need to know about it. If patches get yanked
for whatever reason, I need to get asked. And if I ask for patches to
get included and I get an ack, I should not need to scour various stable
branches to check if that actually happened or not.

So let me make it VERY clear - apply the two attached patches to BOTH
5.10-stable and 5.15-stable. Thank you.

-- 
Jens Axboe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-io_uring-poll-fix-EPOLL_URING_WAKE-sometimes-not-bei.patch
Type: text/x-patch
Size: 1516 bytes
Desc: not available
URL: <http://linuxtesting.org/pipermail/lvc-project/attachments/20260501/deee43e5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-io_uring-poll-fix-backport-of-io_poll_add-changes.patch
Type: text/x-patch
Size: 1665 bytes
Desc: not available
URL: <http://linuxtesting.org/pipermail/lvc-project/attachments/20260501/deee43e5/attachment-0001.bin>