On Fri, Aug 20, 2021 at 11:42:38AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/18/2021 11:16 PM, Matthew Brost wrote:
> > When unblocking a context, do not enable scheduling if the context is
> > banned, guc_id invalid, or not registered.
> >
> > Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
> > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > 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 d61f906105ef..e53a4ef7d442 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1586,6 +1586,9 @@ static void guc_context_unblock(struct intel_context *ce)
> > spin_lock_irqsave(&ce->guc_state.lock, flags);
> > if (unlikely(submission_disabled(guc) ||
> > + intel_context_is_banned(ce) ||
> > + context_guc_id_invalid(ce) ||
> > + !lrc_desc_registered(guc, ce->guc_id) ||
> > !intel_context_is_pinned(ce) ||
> > context_pending_disable(ce) ||
> > context_blocked(ce) > 1)) {
>
> This is getting to a lot of conditions. Maybe we can simplify it a bit? E.g
Yea, this some defensive programming to cover all the basis if another
async operation (ban, reset, unpin) happens when this op is in flight.
Probably some of the conditions are not needed but being extra safe
here.
> it should be possible to check context_blocked, context_banned and
> context_pending_disable as a single op:
>
> #define SCHED_STATE_MULTI_BLOCKED_MASK \ /* 2 or more blocks */
> (SCHED_STATE_BLOCKED_MASK & ~SCHED_STATE_BLOCKED)
> #define SCHED_STATE_NO_UNBLOCK \
> SCHED_STATE_MULTI_BLOCKED_MASK | \
> SCHED_STATE_PENDING_DISABLE | \
> SCHED_STATE_BANNED
Good idea, let me move this to helper in the next spin.
Matt
>
> Not a blocker.
>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
>
> Daniele
>
>
