| Message ID | 20190708152321.22187-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftests: Reorder error cleanup for whitelist checking | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Reorder the error paths so that we unwind all the locals from any error > path and so avoid setting off divers alarum in case we find an error in > case we find an error. Divers alarum got past me. But for finding an error in case we find error: It is fitting enough that I am not sure if it is a mistake. The error paths are better, Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=111048 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > .../gpu/drm/i915/gt/selftest_workarounds.c | 35 ++++++++++--------- > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > index b933d831eeb1..fa01ea7855de 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c > @@ -287,7 +287,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine, > const char *name) > { > struct drm_i915_private *i915 = engine->i915; > - struct i915_gem_context *ctx; > + struct i915_gem_context *ctx, *tmp; > struct igt_spinner spin; > intel_wakeref_t wakeref; > int err; > @@ -295,56 +295,59 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine, > pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n", > engine->whitelist.count, name); > > - err = igt_spinner_init(&spin, i915); > - if (err) > - return err; > - > ctx = kernel_context(i915); > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > + err = igt_spinner_init(&spin, i915); > + if (err) > + goto out_ctx; > + > err = check_whitelist(ctx, engine); > if (err) { > pr_err("Invalid whitelist *before* %s reset!\n", name); > - goto out; > + goto out_spin; > } > > err = switch_to_scratch_context(engine, &spin); > if (err) > - goto out; > + goto out_spin; > > with_intel_runtime_pm(&i915->runtime_pm, wakeref) > err = reset(engine); > > igt_spinner_end(&spin); > - igt_spinner_fini(&spin); > > if (err) { > pr_err("%s reset failed\n", name); > - goto out; > + goto out_spin; > } > > err = check_whitelist(ctx, engine); > if (err) { > pr_err("Whitelist not preserved in context across %s reset!\n", > name); > - goto out; > + goto out_spin; > } > > + tmp = kernel_context(i915); > + if (IS_ERR(tmp)) { > + err = PTR_ERR(tmp); > + goto out_spin; > + } > kernel_context_close(ctx); > - > - ctx = kernel_context(i915); > - if (IS_ERR(ctx)) > - return PTR_ERR(ctx); > + ctx = tmp; > > err = check_whitelist(ctx, engine); > if (err) { > pr_err("Invalid whitelist *after* %s reset in fresh context!\n", > name); > - goto out; > + goto out_spin; > } > > -out: > +out_spin: > + igt_spinner_fini(&spin); > +out_ctx: > kernel_context_close(ctx); > return err; > } > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index b933d831eeb1..fa01ea7855de 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -287,7 +287,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine, const char *name) { struct drm_i915_private *i915 = engine->i915; - struct i915_gem_context *ctx; + struct i915_gem_context *ctx, *tmp; struct igt_spinner spin; intel_wakeref_t wakeref; int err; @@ -295,56 +295,59 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine, pr_info("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n", engine->whitelist.count, name); - err = igt_spinner_init(&spin, i915); - if (err) - return err; - ctx = kernel_context(i915); if (IS_ERR(ctx)) return PTR_ERR(ctx); + err = igt_spinner_init(&spin, i915); + if (err) + goto out_ctx; + err = check_whitelist(ctx, engine); if (err) { pr_err("Invalid whitelist *before* %s reset!\n", name); - goto out; + goto out_spin; } err = switch_to_scratch_context(engine, &spin); if (err) - goto out; + goto out_spin; with_intel_runtime_pm(&i915->runtime_pm, wakeref) err = reset(engine); igt_spinner_end(&spin); - igt_spinner_fini(&spin); if (err) { pr_err("%s reset failed\n", name); - goto out; + goto out_spin; } err = check_whitelist(ctx, engine); if (err) { pr_err("Whitelist not preserved in context across %s reset!\n", name); - goto out; + goto out_spin; } + tmp = kernel_context(i915); + if (IS_ERR(tmp)) { + err = PTR_ERR(tmp); + goto out_spin; + } kernel_context_close(ctx); - - ctx = kernel_context(i915); - if (IS_ERR(ctx)) - return PTR_ERR(ctx); + ctx = tmp; err = check_whitelist(ctx, engine); if (err) { pr_err("Invalid whitelist *after* %s reset in fresh context!\n", name); - goto out; + goto out_spin; } -out: +out_spin: + igt_spinner_fini(&spin); +out_ctx: kernel_context_close(ctx); return err; }
Reorder the error paths so that we unwind all the locals from any error path and so avoid setting off divers alarum in case we find an error in case we find an error. References: https://bugs.freedesktop.org/show_bug.cgi?id=111048 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- .../gpu/drm/i915/gt/selftest_workarounds.c | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-)