drm/i915/guc: Proper xarray usage for contexts_lookup

Lock the xarray and take ref to the context if needed.

v2:
 (Checkpatch)
  - Add new line after declaration
 (Daniel Vetter)
  - Correct put / get accounting in xa_for_loops
v3:
 (Checkpatch)
  - Extra new line

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210909164744.31249-19-matthew.brost@intel.com

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 383dc10170045bdd9bf16e0e4096e41f134eba6d..4814acf1c2aceac1e00924080c7869bb0ea3267e 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -608,8 +608,18 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
        unsigned long index, flags;
        bool pending_disable, pending_enable, deregister, destroyed, banned;
 
+       xa_lock_irqsave(&guc->context_lookup, flags);
        xa_for_each(&guc->context_lookup, index, ce) {
-               spin_lock_irqsave(&ce->guc_state.lock, flags);
+               /*
+                * Corner case where the ref count on the object is zero but and
+                * deregister G2H was lost. In this case we don't touch the ref
+                * count and finish the destroy of the context.
+                */
+               bool do_put = kref_get_unless_zero(&ce->ref);
+
+               xa_unlock(&guc->context_lookup);
+
+               spin_lock(&ce->guc_state.lock);
 
                /*
                 * Once we are at this point submission_disabled() is guaranteed
@@ -625,7 +635,9 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
                banned = context_banned(ce);
                init_sched_state(ce);
 
-               spin_unlock_irqrestore(&ce->guc_state.lock, flags);
+               spin_unlock(&ce->guc_state.lock);
+
+               GEM_BUG_ON(!do_put && !destroyed);
 
                if (pending_enable || destroyed || deregister) {
                        decr_outstanding_submission_g2h(guc);
@@ -648,13 +660,19 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
                        }
                        intel_context_sched_disable_unpin(ce);
                        decr_outstanding_submission_g2h(guc);
-                       spin_lock_irqsave(&ce->guc_state.lock, flags);
+
+                       spin_lock(&ce->guc_state.lock);
                        guc_blocked_fence_complete(ce);
-                       spin_unlock_irqrestore(&ce->guc_state.lock, flags);
+                       spin_unlock(&ce->guc_state.lock);
 
                        intel_context_put(ce);
                }
+
+               if (do_put)
+                       intel_context_put(ce);
+               xa_lock(&guc->context_lookup);
        }
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
 }
 
 static inline bool
@@ -890,16 +908,29 @@ void intel_guc_submission_reset(struct intel_guc *guc, bool stalled)
 {
        struct intel_context *ce;
        unsigned long index;
+       unsigned long flags;
 
        if (unlikely(!guc_submission_initialized(guc))) {
                /* Reset called during driver load? GuC not yet initialised! */
                return;
        }
 
-       xa_for_each(&guc->context_lookup, index, ce)
+       xa_lock_irqsave(&guc->context_lookup, flags);
+       xa_for_each(&guc->context_lookup, index, ce) {
+               if (!kref_get_unless_zero(&ce->ref))
+                       continue;
+
+               xa_unlock(&guc->context_lookup);
+
                if (intel_context_is_pinned(ce))
                        __guc_reset_context(ce, stalled);
 
+               intel_context_put(ce);
+
+               xa_lock(&guc->context_lookup);
+       }
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
+
        /* GuC is blown away, drop all references to contexts */
        xa_destroy(&guc->context_lookup);
 }
@@ -974,11 +1005,24 @@ void intel_guc_submission_cancel_requests(struct intel_guc *guc)
 {
        struct intel_context *ce;
        unsigned long index;
+       unsigned long flags;
+
+       xa_lock_irqsave(&guc->context_lookup, flags);
+       xa_for_each(&guc->context_lookup, index, ce) {
+               if (!kref_get_unless_zero(&ce->ref))
+                       continue;
+
+               xa_unlock(&guc->context_lookup);
 
-       xa_for_each(&guc->context_lookup, index, ce)
                if (intel_context_is_pinned(ce))
                        guc_cancel_context_requests(ce);
 
+               intel_context_put(ce);
+
+               xa_lock(&guc->context_lookup);
+       }
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
+
        guc_cancel_sched_engine_requests(guc->sched_engine);
 
        /* GuC is blown away, drop all references to contexts */
@@ -2866,21 +2910,28 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine)
        struct intel_context *ce;
        struct i915_request *rq;
        unsigned long index;
+       unsigned long flags;
 
        /* Reset called during driver load? GuC not yet initialised! */
        if (unlikely(!guc_submission_initialized(guc)))
                return;
 
+       xa_lock_irqsave(&guc->context_lookup, flags);
        xa_for_each(&guc->context_lookup, index, ce) {
-               if (!intel_context_is_pinned(ce))
+               if (!kref_get_unless_zero(&ce->ref))
                        continue;
 
+               xa_unlock(&guc->context_lookup);
+
+               if (!intel_context_is_pinned(ce))
+                       goto next;
+
                if (intel_engine_is_virtual(ce->engine)) {
                        if (!(ce->engine->mask & engine->mask))
-                               continue;
+                               goto next;
                } else {
                        if (ce->engine != engine)
-                               continue;
+                               goto next;
                }
 
                list_for_each_entry(rq, &ce->guc_active.requests, sched.link) {
@@ -2890,9 +2941,16 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine)
                        intel_engine_set_hung_context(engine, ce);
 
                        /* Can only cope with one hang at a time... */
-                       return;
+                       intel_context_put(ce);
+                       xa_lock(&guc->context_lookup);
+                       goto done;
                }
+next:
+               intel_context_put(ce);
+               xa_lock(&guc->context_lookup);
        }
+done:
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
 }
 
 void intel_guc_dump_active_requests(struct intel_engine_cs *engine,
@@ -2908,23 +2966,34 @@ void intel_guc_dump_active_requests(struct intel_engine_cs *engine,
        if (unlikely(!guc_submission_initialized(guc)))
                return;
 
+       xa_lock_irqsave(&guc->context_lookup, flags);
        xa_for_each(&guc->context_lookup, index, ce) {
-               if (!intel_context_is_pinned(ce))
+               if (!kref_get_unless_zero(&ce->ref))
                        continue;
 
+               xa_unlock(&guc->context_lookup);
+
+               if (!intel_context_is_pinned(ce))
+                       goto next;
+
                if (intel_engine_is_virtual(ce->engine)) {
                        if (!(ce->engine->mask & engine->mask))
-                               continue;
+                               goto next;
                } else {
                        if (ce->engine != engine)
-                               continue;
+                               goto next;
                }
 
-               spin_lock_irqsave(&ce->guc_active.lock, flags);
+               spin_lock(&ce->guc_active.lock);
                intel_engine_dump_active_requests(&ce->guc_active.requests,
                                                  hung_rq, m);
-               spin_unlock_irqrestore(&ce->guc_active.lock, flags);
+               spin_unlock(&ce->guc_active.lock);
+
+next:
+               intel_context_put(ce);
+               xa_lock(&guc->context_lookup);
        }
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
 }
 
 void intel_guc_submission_print_info(struct intel_guc *guc,
@@ -2978,7 +3047,9 @@ void intel_guc_submission_print_context_info(struct intel_guc *guc,
 {
        struct intel_context *ce;
        unsigned long index;
+       unsigned long flags;
 
+       xa_lock_irqsave(&guc->context_lookup, flags);
        xa_for_each(&guc->context_lookup, index, ce) {
                drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id);
                drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca);
@@ -2997,6 +3068,7 @@ void intel_guc_submission_print_context_info(struct intel_guc *guc,
 
                guc_log_context_priority(p, ce);
        }
+       xa_unlock_irqrestore(&guc->context_lookup, flags);
 }
 
 static struct intel_context *