Quoting Tvrtko Ursulin (2018-09-25 10:39:06)
>
> On 25/09/2018 09:31, Chris Wilson wrote:
> > Very light stress test to bombard the submission backends with a large
> > stream with requests of randomly assigned priorities. Preemption will be
> > occasionally requested, but never succeed!
>
> Why won't it ever succeed? By design or because test is targetting some bug?
There's no batch, and for all but a small window for arbitration between
requests, we disallow preemption in the ring.
> > v2: Include a second pattern with more frequent preemption
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/selftests/intel_lrc.c | 137 +++++++++++++++++++++
> > 1 file changed, 137 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c
> > index 1aea7a8f2224..3a474bb64c05 100644
> > --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c
> > @@ -6,6 +6,7 @@
> >
> > #include "../i915_selftest.h"
> > #include "igt_flush_test.h"
> > +#include "i915_random.h"
> >
> > #include "mock_context.h"
> >
> > @@ -573,6 +574,141 @@ static int live_preempt_hang(void *arg)
> > return err;
> > }
> >
> > +static int random_range(struct rnd_state *rnd, int min, int max)
> > +{
> > + return i915_prandom_u32_max_state(max - min, rnd) + min;
> > +}
> > +
> > +static int random_priority(struct rnd_state *rnd)
> > +{
> > + return random_range(rnd, I915_PRIORITY_MIN, I915_PRIORITY_MAX);
> > +}
> > +
> > +struct preempt_smoke {
> > + struct drm_i915_private *i915;
> > + struct i915_gem_context **contexts;
> > + unsigned int ncontext;
> > + struct rnd_state prng;
> > +};
> > +
> > +static struct i915_gem_context *smoke_context(struct preempt_smoke *smoke)
> > +{
> > + return smoke->contexts[i915_prandom_u32_max_state(smoke->ncontext,
> > + &smoke->prng)];
> > +}
> > +
> > +static int smoke_crescendo(struct preempt_smoke *smoke)
> > +{
> > + struct intel_engine_cs *engine;
> > + enum intel_engine_id id;
> > + unsigned long count;
> > +
> > + count = 0;
> > + for_each_engine(engine, smoke->i915, id) {
> > + IGT_TIMEOUT(end_time);
> > +
> > + do {
> > + struct i915_gem_context *ctx = smoke_context(smoke);
> > + struct i915_request *rq;
> > +
> > + ctx->sched.priority = count % I915_PRIORITY_MAX;
> > +
> > + rq = i915_request_alloc(engine, ctx);
> > + if (IS_ERR(rq))
> > + return PTR_ERR(rq);
> > +
> > + i915_request_add(rq);
> > + count++;
> > + } while (!__igt_timeout(end_time, NULL));
> > + }
> > +
> > + pr_info("Submitted %lu crescendo requests across %d engines and %d contexts\n",
> > + count, INTEL_INFO(smoke->i915)->num_rings, smoke->ncontext);
> > + return 0;
> > +}
> > +
> > +static int smoke_random(struct preempt_smoke *smoke)
> > +{
> > + struct intel_engine_cs *engine;
> > + enum intel_engine_id id;
> > + IGT_TIMEOUT(end_time);
> > + unsigned long count;
> > +
> > + count = 0;
> > + do {
> > + for_each_engine(engine, smoke->i915, id) {
> > + struct i915_gem_context *ctx = smoke_context(smoke);
> > + struct i915_request *rq;
> > +
> > + ctx->sched.priority = random_priority(&smoke->prng);
> > +
> > + rq = i915_request_alloc(engine, ctx);
> > + if (IS_ERR(rq))
> > + return PTR_ERR(rq);
> > +
> > + i915_request_add(rq);
> > + count++;
> > + }
> > + } while (!__igt_timeout(end_time, NULL));
> > +
> > + pr_info("Submitted %lu random requests across %d engines and %d contexts\n",
> > + count, INTEL_INFO(smoke->i915)->num_rings, smoke->ncontext);
> > + return 0;
> > +}
>
> Merge smoke_crescendo and smoke_random into one which takes flags to
> decide on priority assign policy, since that seems like the only difference?
The chaining along engines from the loop construct is a big difference.
> > +static int live_preempt_smoke(void *arg)
> > +{
> > + struct preempt_smoke smoke = {
> > + .i915 = arg,
> > + .prng = I915_RND_STATE_INITIALIZER(i915_selftest.random_seed),
> > + .ncontext = 1024,
> > + };
> > + int err = -ENOMEM;
> > + int n;
> > +
> > + if (!HAS_LOGICAL_RING_PREEMPTION(smoke.i915))
> > + return 0;
> > +
> > + smoke.contexts = kmalloc_array(smoke.ncontext,
> > + sizeof(*smoke.contexts),
> > + GFP_KERNEL);
> > + if (!smoke.contexts)
> > + return -ENOMEM;
> > +
> > + mutex_lock(&smoke.i915->drm.struct_mutex);
> > + intel_runtime_pm_get(smoke.i915);
> > +
> > + for (n = 0; n < smoke.ncontext; n++) {
> > + smoke.contexts[n] = kernel_context(smoke.i915);
> > + if (!smoke.contexts[n])
> > + goto err_ctx;
>
> There isn't any request emission on context creation I think so here
> could jump to a new label which only frees.
Sure. The flush gives peace of mind.
> > + }
> > +
> > + err = smoke_crescendo(&smoke);
> > + if (err)
> > + goto err_ctx;
>
> Sync/idle/flush between subtests?
Doesn't make much difference since it's all about trying to get the
magic smoke to leak.
> > + err = smoke_random(&smoke);
> > + if (err)
> > + goto err_ctx;
> > +
> > +err_ctx:
> > + if (igt_flush_test(smoke.i915, I915_WAIT_LOCKED))
> > + err = -EIO;
> > +
> > + for (n = 0; n < smoke.ncontext; n++) {
> > + if (!smoke.contexts[n])
> > + break;
>
> So GFP_ZERO or a post-clear is needed on the array. Or kcalloc.
Oh no it isn't as we never see the uninitialised entries. NULL
indicates the error and time to abort.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
