Re: [PATCH 01/10] drm/amdgpu: introduce amdgpu_lookup_queue_by_doorbell

["Zhu, Lingshan" <lingshan.zhu@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]


On 9/22/2026 4:45 PM, Christian König wrote:
> On 9/22/26 10:39, Zhu, Lingshan wrote:
>> On 9/22/2026 4:04 PM, Christian König wrote:
>>
>>> On 9/22/26 09:55, Huang Rui wrote:
>>> ...
>>>>> As far as I can see we have solved the problems at hand and the rules how to handle the user queues should be pretty clear by now.
>>>>>
>>>> Assuming every kref_get() has a matching kref_put(), and the queue object
>>>> itself does not participate in a circular ownership chain, I don't
>>>> immediately see how taking a temporary queue reference would cause
>>>> correctness issues.
>>> The reference on the user queue object is meant to keep it alive for the duration of IOCTLs, e.g. prevent concurrent destruction from the close IOCTL.
>> why only IOCTL???? Since the kref has been embedded in the struct user queue, it should keep the queue alive during ANY OPERATIONS, Not only IOCTL.
>> Or it will race with the destruction callback in kref_put, and causing UAF problems.
>>
>> Do you see any kernel documents says that kref should only be used in ioctl contexts? 
>> We just cycle over the same questions multiple times now. To limit the reference count to the IOCTL is a design decision I made.
> We just cycle over the same questions multiple times now. To limit the reference count to the IOCTL is a design decision I made.
>
> I tried to explain the rational behind that design decision multiple times now but it obviously doesn't seem to be understood.
>
> So to stop wasting my time I'm stopping to reply to this any further.
>
> This patch set is rejected.

This design is wrong, see kernel documents core-api/kref.rst:

krefs allow you to add reference counters to your objects.  If you
have objects that are used in multiple places and passed around, and
you don't have refcounts, your code is almost certainly broken.  If
you want refcounts, krefs are the way to go.


>
> Regards,
> Christian.
>
>>> It CAN'T be used to grab references from either atomic, interrupt or worker context since then you can't drop the reference any more without risking destruction from non IOCTL context which in turn causes tons of lifetime problems.
>>>
>>> This is just standard refcounting rules to keep objects alive during an IOCTL.
>> Where do you see such standard? It is just a atomic number, not a lock. 
>>>> Could you please point us to the historical issue or the corresponding
>>>> commits that led to removing this approach previously?
>>> See Sunils original approach, he made exactly the same mistake and I already had to fix that back then.
>>>
>>> I don't remember if it was actually committed or if we only discussed that on the mailing list.
>>>
>>> Regards,
>>> Christian.
>>>
>>>> Thanks,
>>>> Ray

[-- Attachment #2: Type: text/html, Size: 4486 bytes --]