Chris Wilson <[email protected]> writes:

> If we have multiple contexts of equal priority pending execution,
> activate a timer to demote the currently executing context in favour of
> the next in the queue when that timeslice expires. This enforces
> fairness between contexts (so long as they allow preemption -- forced
> preemption, in the future, will kick those who do not obey) and allows
> us to avoid userspace blocking forward progress with e.g. unbounded
> MI_SEMAPHORE_WAIT.
>
> For the starting point here, we use the jiffie as our timeslice so that
> we should be reasonably efficient wrt frequent CPU wakeups.
>
> Testcase: igt/gem_exec_scheduler/semaphore-resolve
> Signed-off-by: Chris Wilson <[email protected]>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_types.h |   6 +
>  drivers/gpu/drm/i915/gt/intel_lrc.c          | 111 +++++++++
>  drivers/gpu/drm/i915/gt/selftest_lrc.c       | 223 +++++++++++++++++++
>  drivers/gpu/drm/i915/i915_scheduler.c        |   1 +
>  drivers/gpu/drm/i915/i915_scheduler_types.h  |   1 +
>  5 files changed, 342 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h 
> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 411b7a807b99..6591d6bd2692 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -12,6 +12,7 @@
>  #include <linux/kref.h>
>  #include <linux/list.h>
>  #include <linux/llist.h>
> +#include <linux/timer.h>
>  #include <linux/types.h>
>  
>  #include "i915_gem.h"
> @@ -149,6 +150,11 @@ struct intel_engine_execlists {
>        */
>       struct tasklet_struct tasklet;
>  
> +     /**
> +      * @timer: kick the current context if its timeslice expires
> +      */
> +     struct timer_list timer;
> +
>       /**
>        * @default_priolist: priority list for I915_PRIORITY_NORMAL
>        */
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index dc077b536fa5..fca79adb4aa3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -255,6 +255,7 @@ static int effective_prio(const struct i915_request *rq)
>               prio |= I915_PRIORITY_NOSEMAPHORE;
>  
>       /* Restrict mere WAIT boosts from triggering preemption */
> +     BUILD_BUG_ON(__NO_PREEMPTION & ~I915_PRIORITY_MASK); /* only internal */
>       return prio | __NO_PREEMPTION;
>  }
>  
> @@ -813,6 +814,81 @@ last_active(const struct intel_engine_execlists 
> *execlists)
>       return *last;
>  }
>  
> +static void
> +defer_request(struct i915_request * const rq, struct list_head * const pl)
> +{
> +     struct i915_dependency *p;
> +
> +     /*
> +      * We want to move the interrupted request to the back of
> +      * the round-robin list (i.e. its priority level), but
> +      * in doing so, we must then move all requests that were in
> +      * flight and were waiting for the interrupted request to
> +      * be run after it again.
> +      */
> +     list_move_tail(&rq->sched.link, pl);
> +
> +     list_for_each_entry(p, &rq->sched.waiters_list, wait_link) {
> +             struct i915_request *w =
> +                     container_of(p->waiter, typeof(*w), sched);
> +
> +             /* Leave semaphores spinning on the other engines */
> +             if (w->engine != rq->engine)
> +                     continue;
> +
> +             /* No waiter should start before the active request completed */
> +             GEM_BUG_ON(i915_request_started(w));
> +
> +             GEM_BUG_ON(rq_prio(w) > rq_prio(rq));
> +             if (rq_prio(w) < rq_prio(rq))
> +                     continue;
> +
> +             if (list_empty(&w->sched.link))
> +                     continue; /* Not yet submitted; unready */
> +
> +             /*
> +              * This should be very shallow as it is limited by the
> +              * number of requests that can fit in a ring (<64) and

s/and/or ?

> +              * the number of contexts that can be in flight on this
> +              * engine.
> +              */
> +             defer_request(w, pl);

So the stack frame will be 64*(3*8 + preample/return) at worst case?
can be over 2k

> +     }
> +}
> +
> +static void defer_active(struct intel_engine_cs *engine)
> +{
> +     struct i915_request *rq;
> +
> +     rq = __unwind_incomplete_requests(engine);
> +     if (!rq)
> +             return;
> +
> +     defer_request(rq, i915_sched_lookup_priolist(engine, rq_prio(rq)));
> +}
> +
> +static bool
> +need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
> +{
> +     int hint;
> +
> +     if (list_is_last(&rq->sched.link, &engine->active.requests))
> +             return false;
> +
> +     hint = max(rq_prio(list_next_entry(rq, sched.link)),
> +                engine->execlists.queue_priority_hint);
> +
> +     return hint >= rq_prio(rq);
> +}
> +
> +static bool
> +enable_timeslice(struct intel_engine_cs *engine)
> +{
> +     struct i915_request *last = last_active(&engine->execlists);
> +
> +     return last && need_timeslice(engine, last);
> +}
> +
>  static void execlists_dequeue(struct intel_engine_cs *engine)
>  {
>       struct intel_engine_execlists * const execlists = &engine->execlists;
> @@ -906,6 +982,27 @@ static void execlists_dequeue(struct intel_engine_cs 
> *engine)
>                        */
>                       last->hw_context->lrc_desc |= CTX_DESC_FORCE_RESTORE;
>                       last = NULL;
> +             } else if (need_timeslice(engine, last) &&
> +                        !timer_pending(&engine->execlists.timer)) {
> +                     GEM_TRACE("%s: expired last=%llx:%lld, prio=%d, 
> hint=%d\n",
> +                               engine->name,
> +                               last->fence.context,
> +                               last->fence.seqno,
> +                               last->sched.attr.priority,
> +                               execlists->queue_priority_hint);
> +
> +                     ring_pause(engine) = 1;
> +                     defer_active(engine);
> +
> +                     /*
> +                      * Unlike for preemption, if we rewind and continue
> +                      * executing the same context as previously active,
> +                      * the order of execution will remain the same and
> +                      * the tail will only advance. We do not need to
> +                      * force a full context restore, as a lite-restore
> +                      * is sufficient to resample the monotonic TAIL.
> +                      */

I would have asked about the force preemption without this fine comment.

But this is a similar as the other kind of preemption. So what happens
when the contexts are not the same?

-Mika

> +                     last = NULL;
>               } else {
>                       /*
>                        * Otherwise if we already have a request pending
> @@ -1229,6 +1326,9 @@ static void process_csb(struct intel_engine_cs *engine)
>                                      sizeof(*execlists->pending));
>                       execlists->pending[0] = NULL;
>  
> +                     if (enable_timeslice(engine))
> +                             mod_timer(&execlists->timer, jiffies + 1);
> +
>                       if (!inject_preempt_hang(execlists))
>                               ring_pause(engine) = 0;
>               } else if (status & GEN8_CTX_STATUS_PREEMPTED) {
> @@ -1299,6 +1399,15 @@ static void execlists_submission_tasklet(unsigned long 
> data)
>       spin_unlock_irqrestore(&engine->active.lock, flags);
>  }
>  
> +static void execlists_submission_timer(struct timer_list *timer)
> +{
> +     struct intel_engine_cs *engine =
> +             from_timer(engine, timer, execlists.timer);
> +
> +     /* Kick the tasklet for some interrupt coalescing and reset handling */
> +     tasklet_hi_schedule(&engine->execlists.tasklet);
> +}
> +
>  static void queue_request(struct intel_engine_cs *engine,
>                         struct i915_sched_node *node,
>                         int prio)
> @@ -2524,6 +2633,7 @@ static int gen8_init_rcs_context(struct i915_request 
> *rq)
>  
>  static void execlists_park(struct intel_engine_cs *engine)
>  {
> +     del_timer_sync(&engine->execlists.timer);
>       intel_engine_park(engine);
>  }
>  
> @@ -2621,6 +2731,7 @@ int intel_execlists_submission_setup(struct 
> intel_engine_cs *engine)
>  
>       tasklet_init(&engine->execlists.tasklet,
>                    execlists_submission_tasklet, (unsigned long)engine);
> +     timer_setup(&engine->execlists.timer, execlists_submission_timer, 0);
>  
>       logical_ring_default_vfuncs(engine);
>       logical_ring_default_irqs(engine);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c 
> b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 401e8b539297..0c97f953e908 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -79,6 +79,225 @@ static int live_sanitycheck(void *arg)
>       return err;
>  }
>  
> +static int
> +emit_semaphore_chain(struct i915_request *rq, struct i915_vma *vma, int idx)
> +{
> +     u32 *cs;
> +
> +     cs = intel_ring_begin(rq, 10);
> +     if (IS_ERR(cs))
> +             return PTR_ERR(cs);
> +
> +     *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
> +
> +     *cs++ = MI_SEMAPHORE_WAIT |
> +             MI_SEMAPHORE_GLOBAL_GTT |
> +             MI_SEMAPHORE_POLL |
> +             MI_SEMAPHORE_SAD_NEQ_SDD;
> +     *cs++ = 0;
> +     *cs++ = i915_ggtt_offset(vma) + 4 * idx;
> +     *cs++ = 0;
> +
> +     if (idx > 0) {
> +             *cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> +             *cs++ = i915_ggtt_offset(vma) + 4 * (idx - 1);
> +             *cs++ = 0;
> +             *cs++ = 1;
> +     } else {
> +             *cs++ = MI_NOOP;
> +             *cs++ = MI_NOOP;
> +             *cs++ = MI_NOOP;
> +             *cs++ = MI_NOOP;
> +     }
> +
> +     *cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
> +
> +     intel_ring_advance(rq, cs);
> +     return 0;
> +}
> +
> +static struct i915_request *
> +semaphore_queue(struct intel_engine_cs *engine, struct i915_vma *vma, int 
> idx)
> +{
> +     struct i915_gem_context *ctx;
> +     struct i915_request *rq;
> +     int err;
> +
> +     ctx = kernel_context(engine->i915);
> +     if (!ctx)
> +             return ERR_PTR(-ENOMEM);
> +
> +     rq = igt_request_alloc(ctx, engine);
> +     if (IS_ERR(rq))
> +             goto out_ctx;
> +
> +     err = emit_semaphore_chain(rq, vma, idx);
> +     i915_request_add(rq);
> +     if (err)
> +             rq = ERR_PTR(err);
> +
> +out_ctx:
> +     kernel_context_close(ctx);
> +     return rq;
> +}
> +
> +static int
> +release_queue(struct intel_engine_cs *engine,
> +           struct i915_vma *vma,
> +           int idx)
> +{
> +     struct i915_sched_attr attr = {
> +             .priority = I915_USER_PRIORITY(I915_PRIORITY_MAX),
> +     };
> +     struct i915_request *rq;
> +     u32 *cs;
> +
> +     rq = i915_request_create(engine->kernel_context);
> +     if (IS_ERR(rq))
> +             return PTR_ERR(rq);
> +
> +     cs = intel_ring_begin(rq, 4);
> +     if (IS_ERR(cs)) {
> +             i915_request_add(rq);
> +             return PTR_ERR(cs);
> +     }
> +
> +     *cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
> +     *cs++ = i915_ggtt_offset(vma) + 4 * (idx - 1);
> +     *cs++ = 0;
> +     *cs++ = 1;
> +
> +     intel_ring_advance(rq, cs);
> +     i915_request_add(rq);
> +
> +     engine->schedule(rq, &attr);
> +
> +     return 0;
> +}
> +
> +static int
> +slice_semaphore_queue(struct intel_engine_cs *outer,
> +                   struct i915_vma *vma,
> +                   int count)
> +{
> +     struct intel_engine_cs *engine;
> +     struct i915_request *head;
> +     enum intel_engine_id id;
> +     int err, i, n = 0;
> +
> +     head = semaphore_queue(outer, vma, n++);
> +     if (IS_ERR(head))
> +             return PTR_ERR(head);
> +
> +     i915_request_get(head);
> +     for_each_engine(engine, outer->i915, id) {
> +             for (i = 0; i < count; i++) {
> +                     struct i915_request *rq;
> +
> +                     rq = semaphore_queue(engine, vma, n++);
> +                     if (IS_ERR(rq)) {
> +                             err = PTR_ERR(rq);
> +                             goto out;
> +                     }
> +             }
> +     }
> +
> +     err = release_queue(outer, vma, n);
> +     if (err)
> +             goto out;
> +
> +     if (i915_request_wait(head,
> +                           I915_WAIT_LOCKED,
> +                           2 * RUNTIME_INFO(outer->i915)->num_engines * 
> (count + 2) * (count + 3)) < 0) {
> +             pr_err("Failed to slice along semaphore chain of length (%d, 
> %d)!\n",
> +                    count, n);
> +             GEM_TRACE_DUMP();
> +             i915_gem_set_wedged(outer->i915);
> +             err = -EIO;
> +     }
> +
> +out:
> +     i915_request_put(head);
> +     return err;
> +}
> +
> +static int live_timeslice_preempt(void *arg)
> +{
> +     struct drm_i915_private *i915 = arg;
> +     struct drm_i915_gem_object *obj;
> +     intel_wakeref_t wakeref;
> +     struct i915_vma *vma;
> +     void *vaddr;
> +     int err = 0;
> +     int count;
> +
> +     /*
> +      * If a request takes too long, we would like to give other users
> +      * a fair go on the GPU. In particular, users may create batches
> +      * that wait upon external input, where that input may even be
> +      * supplied by another GPU job. To avoid blocking forever, we
> +      * need to preempt the current task and replace it with another
> +      * ready task.
> +      */
> +
> +     mutex_lock(&i915->drm.struct_mutex);
> +     wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> +     obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> +     if (IS_ERR(obj)) {
> +             err = PTR_ERR(obj);
> +             goto err_unlock;
> +     }
> +
> +     vma = i915_vma_instance(obj, &i915->ggtt.vm, NULL);
> +     if (IS_ERR(vma)) {
> +             err = PTR_ERR(vma);
> +             goto err_obj;
> +     }
> +
> +     vaddr = i915_gem_object_pin_map(obj, I915_MAP_WC);
> +     if (IS_ERR(vaddr)) {
> +             err = PTR_ERR(vaddr);
> +             goto err_obj;
> +     }
> +
> +     err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
> +     if (err)
> +             goto err_map;
> +
> +     for_each_prime_number_from(count, 1, 16) {
> +             struct intel_engine_cs *engine;
> +             enum intel_engine_id id;
> +
> +             for_each_engine(engine, i915, id) {
> +                     memset(vaddr, 0, PAGE_SIZE);
> +
> +                     err = slice_semaphore_queue(engine, vma, count);
> +                     if (err)
> +                             goto err_pin;
> +
> +                     if (igt_flush_test(i915, I915_WAIT_LOCKED)) {
> +                             err = -EIO;
> +                             goto err_pin;
> +                     }
> +             }
> +     }
> +
> +err_pin:
> +     i915_vma_unpin(vma);
> +err_map:
> +     i915_gem_object_unpin_map(obj);
> +err_obj:
> +     i915_gem_object_put(obj);
> +err_unlock:
> +     if (igt_flush_test(i915, I915_WAIT_LOCKED))
> +             err = -EIO;
> +     intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> +     mutex_unlock(&i915->drm.struct_mutex);
> +
> +     return err;
> +}
> +
>  static int live_busywait_preempt(void *arg)
>  {
>       struct drm_i915_private *i915 = arg;
> @@ -398,6 +617,9 @@ static int live_late_preempt(void *arg)
>       if (!ctx_lo)
>               goto err_ctx_hi;
>  
> +     /* Make sure ctx_lo stays before ctx_hi until we trigger preemption. */
> +     ctx_lo->sched.priority = I915_USER_PRIORITY(1);
> +
>       for_each_engine(engine, i915, id) {
>               struct igt_live_test t;
>               struct i915_request *rq;
> @@ -1812,6 +2034,7 @@ int intel_execlists_live_selftests(struct 
> drm_i915_private *i915)
>  {
>       static const struct i915_subtest tests[] = {
>               SUBTEST(live_sanitycheck),
> +             SUBTEST(live_timeslice_preempt),
>               SUBTEST(live_busywait_preempt),
>               SUBTEST(live_preempt),
>               SUBTEST(live_late_preempt),
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c 
> b/drivers/gpu/drm/i915/i915_scheduler.c
> index b1ba3e65cd52..0bd452e851d8 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -394,6 +394,7 @@ bool __i915_sched_node_add_dependency(struct 
> i915_sched_node *node,
>               list_add(&dep->wait_link, &signal->waiters_list);
>               list_add(&dep->signal_link, &node->signalers_list);
>               dep->signaler = signal;
> +             dep->waiter = node;
>               dep->flags = flags;
>  
>               /* Keep track of whether anyone on this chain has a semaphore */
> diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h 
> b/drivers/gpu/drm/i915/i915_scheduler_types.h
> index 3e309631bd0b..aad81acba9dc 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler_types.h
> +++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
> @@ -62,6 +62,7 @@ struct i915_sched_node {
>  
>  struct i915_dependency {
>       struct i915_sched_node *signaler;
> +     struct i915_sched_node *waiter;
>       struct list_head signal_link;
>       struct list_head wait_link;
>       struct list_head dfs_link;
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to