Re: [PATCH v1] drm/amdgpu: give each kernel job a unique id

[Danilo Krummrich <dakr@kernel.org>]

On Fri, Jun 13, 2025 at 11:27:08AM +0200, Pierre-Eric Pelloux-Prayer wrote:
> Le 13/06/2025 à 10:35, Danilo Krummrich a écrit :
> > On Fri, Jun 13, 2025 at 10:23:15AM +0200, Christian König wrote:
> > > > Another option is to just add an interface to get a kernel client_id from the
> > > > same atomic / IDA.
> > > 
> > > That won't give us fixed numbers for in kernel clients.
> > 
> > That's fine, then let's come up with an API that reserves fixed numbers.
> > 
> > My main concern is that drivers silently rely on DRM API internals, i.e. that
> > client_id is an u64 atomic, etc.
> 
> Let me express the need then: an id that is printed in gpu_scheduler_trace
> events and this id needs to be mappable by a userspace tool to a semantic.
> 
> The current solution implements this need using:
> * fixed ids which are u64 numbers because drm client id are u64.
> * hard-coded mapping in the tool between these ids and their meaning
> ("u64_max - 1" interpreted as "vm_update" and so on).
> 
> It doesn't depend on how drm internally manage these ids.
> 
> Adding an API to reserve fixed numbers would work but:
> * if the fixed numbers are chosen by the driver ("drm_reserve_id(u64_max
> -1)"), I don't see the benefit over the current patch
> * if the fixed numbers are allocated by drm (drm_reserve_id("vm_update") ->
> u64), it would then require a way to expose them to userspace (through
> /sys/kernel/debug/dri/x/clients?)

Yeah, both is possible, I'm fine with either.

The benefit is that this way it becomes an official API, which can (and must)
be considered should there ever be a change on drm_file::client_id.

If someone would patch drm_file::client_id to start allocating IDs from high to
low, your corresponding driver code would silently break, because it relies on
an implementation detail of something that is not an official API.

Yes, I know that this exact case won't happen, but I guess you get the idea. :-)