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

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

On 9/22/25 10:52, Pavel Begunkov wrote:
> On 9/19/25 15:28, David Kahurani wrote:
>> 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
>
> It'll put its reference to the head, but nothing is going to
> be destroyed until the head refs hit 0.


I take that to mean there's some code elsewhere that also interacts with 
these references otherwise just based on this code, it seems like notifs 
always have a reference of 1

Because I don't have a stacktrace, I will leave it that.


>
>> node. But, I trust that you know better :-)
>>
>> What do you mean it's not supposed to? All the nodes eventually go
>
> I was saying that the head isn't supposed to put the children's
> references, it goes the other way around. Children have refs to
> head, and everything is destroyed once the head is put down.
>
>> 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.