Re: [PATCH] io_uring: fix incorrect io_kiocb reference in io_link_skb

[David Kahurani <k.kahurani@gmail.com>]

On Fri, Sep 19, 2025 at 5:14 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 9/19/25 12:25, David Kahurani wrote:
> ...>>>> Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
> >>>>
> >>>> diff --git a/io_uring/notif.c b/io_uring/notif.c
> >>>> index 9a6f6e92d742..ea9c0116cec2 100644
> >>>> --- a/io_uring/notif.c
> >>>> +++ b/io_uring/notif.c
> >>>> @@ -85,7 +85,7 @@ static int io_link_skb(struct sk_buff *skb, struct ubuf_info *uarg)
> >>>>                return -EEXIST;
> >>>>
> >>>>        prev_nd = container_of(prev_uarg, struct io_notif_data, uarg);
> >>>> -     prev_notif = cmd_to_io_kiocb(nd);
> >>>> +     prev_notif = cmd_to_io_kiocb(prev_nd);
> >>>>
> >>>>        /* make sure all noifications can be finished in the same task_work */
> >>>>        if (unlikely(notif->ctx != prev_notif->ctx ||
> >>>
> >>> --
> >>> Pavel Begunkov
> >>>
> >>>
> >
> > This is something unrelated but just bringing it up because it is in
> > the same locality.
> >
> > It doesn't seem like the references(uarg->refcnt) are well accounted
> > for io_notif_data. Any node that gets passed to 'io_tx_ubuf_complete'
> > will gets it's refcnt decremented but assuming there's a list of
> > nodes, some of the nodes in the list will not get their reference
> > count decremented and
>
> And not supposed to. Children reference the head, and the head dies
> last.

I am not sure about the mechanics of this. This is only based on
analysing the code but it seems, if a child node gets completed, it
will pull all the other nodes in that link by jumping to the head
node. But, I trust that you know better :-)

What do you mean it's not supposed to? All the nodes eventually go
through 'io_notif_tw_complete' to be queued back into request queues,
if any nodes whose reference was not handled(all nodes get a reference
of 1 at allocation) goes through the method, then the warning will
trigger.

>
> > that will trigger the lockdep_assert in
> > 'io_notif_tw_complete'
>
> Did you see it trigger? If so, please attach the warning splat.
>
> > It doesn't look that this will have any consequences beyond triggering
> > the lockderp_assert, though.
>
> --
> Pavel Begunkov
>