On Thu, Jan 13, 2022 at 03:18:14PM +0100, Michal Wajdeczko wrote:
>
>
> 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
>
Ah, wasn't aware of this. If this becomes an issue (running of multi-lrc
ids) for customers I suppose this is the first thing we can do to try to
address this. For now, I think we leave it as is.
> ..
>
>
> >>> @@ -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?
>
Good catch. Yes, it techincally possible two multi-lrc contexts to try
to allocate at the same time. I guess I should just do this at driver
load time + allocate the maximum number of multi-lrc ids and possibly
waste a bit of memory on a PF or VF.
Matt
> -Michal
