Re: [PATCH 1/2] drm/sched: add drm_sched_prealloc_dependency_slots

[Philipp Stanner <phasta@mailbox.org>]

On Thu, 2025-03-20 at 11:49 +0000, Tvrtko Ursulin wrote:
> 
> On 19/03/2025 11:23, Christian König wrote:
> > > > + *
> > > > + * Return:
> > > > + * 0 on success, or an error on failing to expand the array.
> > > > + */
> > > > +int drm_sched_job_prealloc_dependency_slots(struct
> > > > drm_sched_job
> > > > *job,
> > > > +					    unsigned int
> > > > num_deps)
> > > > +{
> > > > +	struct dma_fence *fence;
> > > > +	u32 id = 0;
> > > > +	int ret;
> > > > +
> > > > +	while (num_deps--) {
> > > > +		fence = dma_fence_get_stub();
> > > > +		ret = xa_alloc(&job->dependencies, &id, fence,
> > > > xa_limit_32b,
> > > > +			       GFP_KERNEL);
> > > So this would fill the xarr with already signaled fences which
> > > then
> > > later will be replaced with unsignaled fences?
> > 
> > Yes, exactly that's the idea.
> > 
> > > Help me out here: would it also work to add NULL instead of that
> > > stub-
> > > fence?
> > 
> > Good question, idk. That's an implementation detail of the xarray.
> > 
> > Tvrtko also correctly pointed out that it is most likely a bad idea
> > to 
> > use dma_fence_is_signaled() in the critical code path.
> > 
> > I will try to dig through the xarray behavior up and update the
> > patch if 
> > possible.
> 
> I think NULL on its own is not possible, but the two low bits are 
> available for pointer tagging, or designating pointers vs integers, 
> which looks like it could work. Something like storing 
> xa_tag_pointer(NULL, 1) to reserved slots and at lookup time they
> would 
> be detected with "xa_pointer_tag(fence) & 1".

Almost!

they would be detected with a super-readable

#define DRM_SCHED_XARR_TAG_RESERVED_ENTRY 1

or maybe …UNUSED_ENTRY?

^_^

P.


> 
> Regards,
> 
> Tvrtko
>