On 13.01.2022 00:26, Matthew Brost wrote:
> On Thu, Jan 13, 2022 at 12:21:17AM +0100, Michal Wajdeczko wrote:
>> On 11.01.2022 17:30, Matthew Brost wrote:
...
>>> @@ -1863,6 +1861,33 @@ static void guc_submit_request(struct i915_request *rq)
>>> spin_unlock_irqrestore(&sched_engine->lock, flags);
>>> }
>>>
>>> +static int new_mlrc_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>> +{
>>> + int ret;
>>> +
>>> + GEM_BUG_ON(!intel_context_is_parent(ce));
>>> + GEM_BUG_ON(!guc->submission_state.guc_ids_bitmap);
>>> +
>>> + ret = bitmap_find_free_region(guc->submission_state.guc_ids_bitmap,
>>> + NUMBER_MULTI_LRC_GUC_ID(guc),
>>> + order_base_2(ce->parallel.number_children
>>> + + 1));
>>
>> btw, is there any requirement (GuC ABI ?) that allocated ids need
>> to be allocated with power of 2 alignment ? I don't think that we
>> must optimize that hard and in some cases waste extra ids (as we might
>> be limited on some configs)
>>
>
> No pow2 requirement in GuC ABI, bitmaps only work on pow2 alignment and
> didn't optmize this.
>
there is a slower variant of "find" function:
bitmap_find_next_zero_area - find a contiguous aligned zero area
that does not have this limitation
..
>>> @@ -1989,6 +2008,14 @@ static int pin_guc_id(struct intel_guc *guc, struct intel_context *ce)
>>>
>>> GEM_BUG_ON(atomic_read(&ce->guc_id.ref));
>>>
>>> + if (unlikely(intel_context_is_parent(ce) &&
>>> + !guc->submission_state.guc_ids_bitmap)) {
>>> + guc->submission_state.guc_ids_bitmap =
>>> + bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL);
>>> + if (!guc->submission_state.guc_ids_bitmap)
>>> + return -ENOMEM;
>>> + }
>>
>> maybe move this chunk to new_mlrc_guc_id() ?
>> or we can't due to the spin_lock below ?
>> but then how do you protect guc_ids_bitmap pointer itself ?
>>
>
> Can't use GFP_KERNEL inside a spin lock...
>
ok, but what if there will be two or more parallel calls to pin_guc_id()
with all being first parent context? each will see NULL guc_ids_bitmap..
or there is another layer of synchronization?
-Michal
