Re: [bug report] io_uring/poll: get rid of unlocked cancel hash

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Oct 04, 2024 at 07:54:32AM -0600, Jens Axboe wrote:
> On 10/4/24 7:50 AM, Jens Axboe wrote:
> > On 10/4/24 3:00 AM, Dan Carpenter wrote:
> >> Hello Jens Axboe,
> >>
> >> Commit 313314db5bcb ("io_uring/poll: get rid of unlocked cancel
> >> hash") from Sep 30, 2024 (linux-next), leads to the following Smatch
> >> static checker warning:
> >>
> >> 	io_uring/poll.c:932 io_poll_remove()
> >> 	warn: duplicate check 'ret2' (previous on line 930)
> >>
> >> io_uring/poll.c
> >>     919 int io_poll_remove(struct io_kiocb *req, unsigned int issue_flags)
> >>     920 {
> >>     921         struct io_poll_update *poll_update = io_kiocb_to_cmd(req, struct io_poll_update);
> >>     922         struct io_ring_ctx *ctx = req->ctx;
> >>     923         struct io_cancel_data cd = { .ctx = ctx, .data = poll_update->old_user_data, };
> >>     924         struct io_kiocb *preq;
> >>     925         int ret2, ret = 0;
> >>     926 
> >>     927         io_ring_submit_lock(ctx, issue_flags);
> >>     928         preq = io_poll_find(ctx, true, &cd);
> >>     929         ret2 = io_poll_disarm(preq);
> >>     930         if (!ret2)
> >>     931                 goto found;
> >> --> 932         if (ret2) {
> >>     933                 ret = ret2;
> >>     934                 goto out;
> >>     935         }
> >>
> >> A lot of the function is dead code now.  ;)
> > 
> > Thanks, will revisit and fold in a fix!
> 
> Should just need this incremental. There's no dead code as far as I can
> see, just a needless found label and jump.
> 
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index 69382da48c00..217d667e0622 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -940,13 +940,10 @@ int io_poll_remove(struct io_kiocb *req, unsigned int issue_flags)
>  	ret2 = io_poll_disarm(preq);
>  	if (bucket)
>  		spin_unlock(&bucket->lock);
> -	if (!ret2)
> -		goto found;

Oh.  I thought this was a goto out.  That explains how the code was passing
tests.  That was an easy fix.

regards,
dan carpenter

>  	if (ret2) {
>  		ret = ret2;
>  		goto out;
>  	}
> -found:
>  	if (WARN_ON_ONCE(preq->opcode != IORING_OP_POLL_ADD)) {
>  		ret = -EFAULT;
>  		goto out;
> 
> -- 
> Jens Axboe