Re: [PATCH i-g-t 3/4 v4] tests/drv_module_reload: Convert sh script to C version.

[Marius Vlad <marius.c.vlad@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2224 bytes --]

On Fri, Oct 28, 2016 at 10:48:33AM +0100, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:31:28PM +0300, Marius Vlad wrote:
> > +	igt_subtest_group {
> > +		int fd;
> > +
> > +		igt_fixture
> > +			fd = __drm_open_driver(DRIVER_INTEL);
> > +
> > +		igt_subtest_group {
> > +			igt_subtest("basic-gem-info")
> > +				if ((err = gem_info(fd)))
> > +					igt_fail(err);
> > +			igt_subtest_group
> > +				if ((err = gem_exec_store(fd)))
> > +					igt_fail(err);
> 
> This is a misinterpretation of the existing test. These are not
> subtests, but a sanity check that reload() works. A call to
> gem_exec_store() can just whither away, it doesn't reveal much about the
> general sanity of the driver, but a quick call to an ioctl such as
> gem_info() is enough to be sure the driver is there (i.e. the module load
> worked as adverticed) and not hung upon load.
Indeed, that's exactly what they are: sanity check that reloading of the
driver works, just that grouping them as subtests/subgroups made
embedding them easier and I thought that worked better given igt is
centered around tests/subtests (gem_exec_store has a igt_skip_on_f()
thus the subgroup)

I guess that I can call them directly in "basic-reload" subtest. If
either gem_info and gem_exec_store failed than reloading failed. Is that a
reasonable approach and it is OK with you?

> 
> One more thing about gem_info(), to be used in process it needs to be
> something like:
> 
> /* Check the driver load completed by seeing if it responds to an ioctl
>  * correctly. We pick an ioctl that calls i915_mutex_interruptible()
>  * first (before failing on the invalid arg) so that we can inspect its
>  * driver error detection (-EIO if already wedged, or -EINTR if stuck).
>  */
> void i915_gem_sanitycheck(void)
> {
> 	struct set_cacheing arg = {};
> 	int fd, err;
> 
> 	fd = __drm_driver_open(DRIVER_INTEL);
> 	igt_set_timeout(1, "module reload failed");
> 
> 	err = 0;
> 	if (ioctl(fd, SET_CACHEING, &arg))
> 		err = -errno;
> 
> 	igt_set_timeout(0, NULL);
> 	close(fd);
> 
> 	igt_assert_eq(err, -ENOENT);
> 
> }
Alright will do.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx