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

Fedor Pchelkin
2026-05-01T21:55:55Z

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:

--
Thanks,
Fedor