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