On 10/4/24 8:54 AM, Dan Carpenter wrote:
> 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.
Yeah, that check was just dead code, but the rest was fine. Thanks for
letting me know!
--
Jens Axboe
