| Message ID | 20180508115312.12628-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chris Wilson <chris@chris-wilson.co.uk> writes: > As we flush each test and wait for idle before the next, also switch > back to the kernel context. This helps limit the amount of collateral > damage a test may cause by resetting to the default state each time (and > also helps clean up temporaries used by the test). > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > index abff2f04ea84..7f35bddc2e95 100644 > --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c > +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) > > cond_resched(); > > + if (i915_gem_switch_to_kernel_context(i915)) { > + pr_err("Failed to switch back to kernel context; declaring wedged\n"); > + i915_gem_set_wedged(i915); You don't want to give the error code? It comes from request_alloc. Also if the test already wedged itself we would wedge again. -Mika > + } > + > wedge_on_timeout(&w, i915, HZ) > i915_gem_wait_for_idle(i915, flags); > > -- > 2.17.0
Quoting Mika Kuoppala (2018-05-08 13:34:49) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > As we flush each test and wait for idle before the next, also switch > > back to the kernel context. This helps limit the amount of collateral > > damage a test may cause by resetting to the default state each time (and > > also helps clean up temporaries used by the test). > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > index abff2f04ea84..7f35bddc2e95 100644 > > --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) > > > > cond_resched(); > > > > + if (i915_gem_switch_to_kernel_context(i915)) { > > + pr_err("Failed to switch back to kernel context; declaring wedged\n"); > > + i915_gem_set_wedged(i915); > > You don't want to give the error code? It comes from request_alloc. We are setting wedged, so we end up with returning -EIO. > Also if the test already wedged itself we would wedge again. Trickier. But at that moment, doubling up on the error messages isn't the worst thing after hitting a terminal error. -Chris
Quoting Chris Wilson (2018-05-08 13:38:09) > Quoting Mika Kuoppala (2018-05-08 13:34:49) > > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > > > As we flush each test and wait for idle before the next, also switch > > > back to the kernel context. This helps limit the amount of collateral > > > damage a test may cause by resetting to the default state each time (and > > > also helps clean up temporaries used by the test). > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > > index abff2f04ea84..7f35bddc2e95 100644 > > > --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > > +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > > > @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) > > > > > > cond_resched(); > > > > > > + if (i915_gem_switch_to_kernel_context(i915)) { > > > + pr_err("Failed to switch back to kernel context; declaring wedged\n"); > > > + i915_gem_set_wedged(i915); > > > > You don't want to give the error code? It comes from request_alloc. > > We are setting wedged, so we end up with returning -EIO. > > > Also if the test already wedged itself we would wedge again. > > Trickier. But at that moment, doubling up on the error messages isn't > the worst thing after hitting a terminal error. It's the kind of distraction one tackles after hitting the terminal error so that you can ignore the underlying problem for a bit while you think ;) -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Chris Wilson (2018-05-08 13:38:09) >> Quoting Mika Kuoppala (2018-05-08 13:34:49) >> > Chris Wilson <chris@chris-wilson.co.uk> writes: >> > >> > > As we flush each test and wait for idle before the next, also switch >> > > back to the kernel context. This helps limit the amount of collateral >> > > damage a test may cause by resetting to the default state each time (and >> > > also helps clean up temporaries used by the test). >> > > >> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> > > --- >> > > drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ >> > > 1 file changed, 5 insertions(+) >> > > >> > > diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c >> > > index abff2f04ea84..7f35bddc2e95 100644 >> > > --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c >> > > +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c >> > > @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) >> > > >> > > cond_resched(); >> > > >> > > + if (i915_gem_switch_to_kernel_context(i915)) { >> > > + pr_err("Failed to switch back to kernel context; declaring wedged\n"); >> > > + i915_gem_set_wedged(i915); >> > >> > You don't want to give the error code? It comes from request_alloc. >> >> We are setting wedged, so we end up with returning -EIO. >> >> > Also if the test already wedged itself we would wedge again. >> >> Trickier. But at that moment, doubling up on the error messages isn't >> the worst thing after hitting a terminal error. > > It's the kind of distraction one tackles after hitting the terminal > error so that you can ignore the underlying problem for a bit while you > think ;) Yeah it the test manages to wedge we would get EIO so nothing to see there. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Quoting Mika Kuoppala (2018-05-08 13:57:23) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Chris Wilson (2018-05-08 13:38:09) > >> Quoting Mika Kuoppala (2018-05-08 13:34:49) > >> > Chris Wilson <chris@chris-wilson.co.uk> writes: > >> > > >> > > As we flush each test and wait for idle before the next, also switch > >> > > back to the kernel context. This helps limit the amount of collateral > >> > > damage a test may cause by resetting to the default state each time (and > >> > > also helps clean up temporaries used by the test). > >> > > > >> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > >> > > --- > >> > > drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ > >> > > 1 file changed, 5 insertions(+) > >> > > > >> > > diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > >> > > index abff2f04ea84..7f35bddc2e95 100644 > >> > > --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c > >> > > +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c > >> > > @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) > >> > > > >> > > cond_resched(); > >> > > > >> > > + if (i915_gem_switch_to_kernel_context(i915)) { > >> > > + pr_err("Failed to switch back to kernel context; declaring wedged\n"); > >> > > + i915_gem_set_wedged(i915); > >> > > >> > You don't want to give the error code? It comes from request_alloc. > >> > >> We are setting wedged, so we end up with returning -EIO. > >> > >> > Also if the test already wedged itself we would wedge again. > >> > >> Trickier. But at that moment, doubling up on the error messages isn't > >> the worst thing after hitting a terminal error. > > > > It's the kind of distraction one tackles after hitting the terminal > > error so that you can ignore the underlying problem for a bit while you > > think ;) > > Yeah it the test manages to wedge we would get EIO so nothing > to see there. > > Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Applied, thanks for the review. If we ever feel upset about the double message in practice, then is the time to fix it ;) -Chris
diff --git a/drivers/gpu/drm/i915/selftests/igt_flush_test.c b/drivers/gpu/drm/i915/selftests/igt_flush_test.c index abff2f04ea84..7f35bddc2e95 100644 --- a/drivers/gpu/drm/i915/selftests/igt_flush_test.c +++ b/drivers/gpu/drm/i915/selftests/igt_flush_test.c @@ -57,6 +57,11 @@ int igt_flush_test(struct drm_i915_private *i915, unsigned int flags) cond_resched(); + if (i915_gem_switch_to_kernel_context(i915)) { + pr_err("Failed to switch back to kernel context; declaring wedged\n"); + i915_gem_set_wedged(i915); + } + wedge_on_timeout(&w, i915, HZ) i915_gem_wait_for_idle(i915, flags);
As we flush each test and wait for idle before the next, also switch back to the kernel context. This helps limit the amount of collateral damage a test may cause by resetting to the default state each time (and also helps clean up temporaries used by the test). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/selftests/igt_flush_test.c | 5 +++++ 1 file changed, 5 insertions(+)