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

[Philipp Stanner <phasta@mailbox.org>]

On Fri, 2025-06-13 at 10:23 +0200, Christian König wrote:
> On 6/13/25 01:48, Danilo Krummrich wrote:
> > On Thu, Jun 12, 2025 at 09:00:34AM +0200, Christian König wrote:
> > > On 6/11/25 17:11, Danilo Krummrich wrote:
> > > > > > > Mhm, reiterating our internal discussion on the mailing
> > > > > > > list.
> > > > > > > 
> > > > > > > I think it would be nicer if we could use negative values
> > > > > > > for the kernel submissions and positive for userspace.
> > > > > > > But as discussed internally we would need to adjust the
> > > > > > > scheduler trace points for that once more.
> > > > > > > 
> > > > > > > @Philip and @Danilo any opinion on that?
> > > > > > 
> > > > > > Both, the U64_MAX and the positive-negative approach, are a
> > > > > > bit hacky. I wonder
> > > > > > why we need client_id to be a u64, wouldn't a u32 not be
> > > > > > enough?
> > > > > 
> > > > > That can trivially overflow on long running boxes.
> > > > 
> > > > I don't know if "trivially" is the word of choice given that
> > > > the number is
> > > > 4,294,967,295.
> > > > 
> > > > But I did indeed miss that this is a for ever increasing
> > > > atomic. Why is it an
> > > > atomic? Why is it not an IDA?
> > > 
> > > Well IDA has some extra overhead compared to an ever increasing
> > > atomic, additional to that it might not be the best choice to re-
> > > use numbers for clients in a trace log.
> > 
> > I think the overhead is not relevant at all, this is called from
> > drm_file_alloc(). The only path I can see where this is called is
> > drm_client_init(), which isn't high frequent stuff at all, is it?
> 
> I don't think so. But we should really use ida_alloc_cyclic to make
> sure that numbers are not re-used so quickly.

Shouldn't the xarray be used nowadays for ID allocation? I think
idr_alloc_cyclic() (ida_alloc_cyclic() doesn't exist) is just a wrapper
around the xarray anyways.

P.


> 
> > 
> > It seems to me that we should probably use IDA here.
> > 
> > > On the other hand using smaller numbers is usually nicer for
> > > manual inspection.
> > 
> > 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.
> 
> Regards,
> Christian.