On Wed, Jan 11, 2017 at 06:17:48PM +0000, Tvrtko Ursulin wrote: > On 20/12/2016 13:07, Chris Wilson wrote: > >@@ -522,6 +534,11 @@ static struct pci_driver i915_pci_driver = { > > static int __init i915_init(void) > > { > > bool use_kms = true; > >+ int err; > >+ > >+ err = i915_mock_selftests(); > >+ if (err) > >+ return err > 0 ? 0 : err; > > Am I again confused by the return codes? :) Module param of -1 will > result in i915_mock_selftests returning 1, which here translates to > 0 so it won't abort the load like it should. I had to give up on that for silent passing and do the remove from userspace on success instead. Returning anything other than 0 causes noise in dmesg. That I can live with after an error during the selftest, since dmesg should also contain more details on the test failure. If i915.mock_selftests=-1 then we run the tests and stop. We just leave the module loaded even though it hasn't bound to any pci devices. :| igt/drv_selftest and kselftests/gpu/i915.sh then unload the module. > >+static void set_default_test_all(struct selftest *st, unsigned long count) > >+{ > >+ unsigned long i; > >+ > >+ for (i = 0; i < count; i++) > >+ if (st[i].enabled) > >+ return; > >+ > >+ for (i = 0; i < count; i++) > >+ st[i].enabled = true; > >+} > > unsigned int should be enough for everyone! :) (i & count) Such shortsightedness! > >+static int run_selftests(const char *name, > >+ struct selftest *st, > >+ unsigned long count, > >+ void *data) > >+{ > >+ int err = 0; > >+ > > If I got it right: > > /* Make sure both live and mock run with the same seed if ran one > after another. */ Yes, choose the seed once, run every selected test with the same seed. > ? just not sure what happens if user sets zero. I wasn't such if 0 was a valid seed, so I wasn't caring too much if the user did i915.st_random_seed=0. They will see the pr_info() and go wtf, and hopefully don't do that again. > >+ while (!i915_selftest.random_seed) > >+ i915_selftest.random_seed = get_random_int(); > >+ > >+ i915_selftest.timeout_jiffies = > >+ i915_selftest.timeout_ms ? > >+ msecs_to_jiffies_timeout(i915_selftest.timeout_ms) : > >+ MAX_SCHEDULE_TIMEOUT; > >+ > >+ set_default_test_all(st, count); > >+ > >+ pr_info("i915: Performing %s selftests with st_random_seed=%x and st_timeout=%u\n", > >+ name, i915_selftest.random_seed, i915_selftest.timeout_ms); > >+ > >+ /* Tests are listed in order in i915_*_selftests.h */ > >+ for (; count--; st++) { > >+ if (!st->enabled) > >+ continue; > >+ > >+ cond_resched(); > >+ if (signal_pending(current)) > >+ return -EINTR; > >+ > >+ pr_debug("i915: Running %s\n", st->name); > >+ if (data) > >+ err = st->live(data); > >+ else > >+ err = st->mock(); > >+ if (err) > >+ break; > >+ } > >+ > >+ if (WARN(err > 0 || err == -ENOTTY, > >+ "%s returned %d, conflicting with selftest's magic values!\n", > >+ st->name, err)) > >+ err = -1; > >+ > >+ rcu_barrier(); > > Why this? Paranoia for the tests aborting without the barrier, as we can't rely on module_unload providing it since we may go on to load the driver as normal. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx