[igt-dev] [PATCH i-g-t] i915: Exercise preemption timeout controls in sysfs

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-10-18 13:23:53)
> 
> On 17/10/2019 15:30, Chris Wilson wrote:
> > Dynamic subtests!
> 
> Ouch! :)
> 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> > +static void test_timeout(int i915, int engine)
> > +{
> > +     int delays[] = { 1, 50, 100, 500 };
> > +     unsigned int saved, delay;
> > +
> > +     igt_assert(igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &saved) == 1);
> > +     igt_debug("Initial preempt_timeout_ms:%u\n", saved);
> > +
> > +     gem_quiescent_gpu(i915);
> > +     igt_require(enable_hangcheck(i915, false));
> > +
> > +     for (int i = 0; i < ARRAY_SIZE(delays); i++) {
> > +             uint64_t elapsed;
> > +
> > +             elapsed = __test_timeout(i915, engine, delays[i]);
> > +             igt_info("preempt_timeout_ms:%d, elapsed=%.3fms\n",
> > +                      delays[i], elapsed * 1e-6);
> 
> No checking that measured time relates to configured timeout?

Have now. Just needed some soaking to decide on thresholds. I've 50ms
but that may change as CI tends to have more scheduling intolerance than
local machines.

> > +     }
> > +
> > +     igt_assert(enable_hangcheck(i915, true));
> > +     gem_quiescent_gpu(i915);
> > +
> > +     igt_sysfs_printf(engine, "preempt_timeout_ms", "%u", saved);
> > +     igt_sysfs_scanf(engine, "preempt_timeout_ms", "%u", &delay);
> > +     igt_assert_eq(delay, saved);
> > +}
> > +
> > +igt_main
> > +{
> > +     int i915, sys = -1;
> > +     struct dirent *de;
> > +     int engines;
> > +     DIR *dir;
> > +
> > +     igt_fixture {
> > +             i915 = drm_open_driver(DRIVER_INTEL);
> > +             igt_require_gem(i915);
> > +
> > +             sys = igt_sysfs_open(i915);
> > +             igt_require(sys != -1);
> 
> igt_assert_fd?

Do we guarantee that the sysadmin has mounted sysfs? We don't automount
it unlike debugfs.

> > +             igt_subtest_group {
> > +                     igt_fixture {
> > +                             igt_require(fstatat(engine,
> > +                                                 "preempt_timeout_ms",
> > +                                                 &st, 0) == 0);
> > +                     }
> > +
> > +                     igt_subtest_f("%s-idempotent", name)
> > +                             test_idempotent(i915, engine);
> > +                     igt_subtest_f("%s-invalid", name)
> > +                             test_invalid(i915, engine);
> > +                     igt_subtest_f("%s-timeout", name)
> > +                             test_timeout(i915, engine);
> > +             }
> > +
> > +             free(name);
> > +             close(engine);
> > +     }
> 
> You probably should use __for_each_static_engine and then open sysfs 
> nodes based on that. Gets around the dynamic subtests no-no at least.

Defeatist!
-Chris