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

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 21.03.25 um 09:20 schrieb Philipp Stanner:
> 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?

NULL doesn't work because xa_for_each() skips NULL entries, but it looks like somebody else stumbled over the same problem we have here as well.

So there is already the solution to use XA_ZERO_ENTRY! That special value can then either be used with xa_alloc() or through xa_reserve().

It's just that the xarray documentation is not explicitly pointing that out, so I had to dig around in the code a bit to figure out how everything works.

Regards,
Christian.

>
> ^_^
>
> P.
>
>
>> Regards,
>>
>> Tvrtko
>>