All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
@ 2018-07-27 10:04 Chris Wilson
  2018-07-27 10:14 ` Petri Latvala
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2018-07-27 10:04 UTC (permalink / raw)
  To: igt-dev

The next test will happily load whatever module it requires.

v2: Unload at start, mandate tests cleanup after themselves.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tests/drv_module_reload.c | 48 ++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 34f55eab1..cf4aea80d 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -220,20 +220,6 @@ static void store_all(int fd)
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
 
-static int
-reload(const char *opts_i915)
-{
-	int err = IGT_EXIT_SUCCESS;
-
-	if ((err = igt_i915_driver_unload()))
-		return err;
-
-	if ((err = igt_i915_driver_load(opts_i915)))
-		return err;
-
-	return err;
-}
-
 static int open_parameters(const char *module_name)
 {
 	char path[256];
@@ -340,21 +326,30 @@ hda_dynamic_debug(bool enable)
 
 igt_main
 {
-	int err;
+	igt_subtest("basic-reload") {
+		int load_error;
+
+		igt_i915_driver_unload();
 
-	igt_fixture
 		hda_dynamic_debug(true);
+		load_error = igt_i915_driver_load(NULL);
+		hda_dynamic_debug(false);
 
-	igt_subtest("basic-reload") {
-		if ((err = reload(NULL)))
-			igt_fail(err);
+		igt_assert_eq(load_error, 0);
 
 		gem_sanitycheck();
 		gem_exec_store();
+
+		/* only default modparams, can leave module loaded */
 	}
 
-	igt_subtest("basic-no-display")
-		igt_assert_eq(reload("disable_display=1"), 0);
+	igt_subtest("basic-no-display") {
+		igt_i915_driver_unload();
+
+		igt_assert_eq(igt_i915_driver_load("disable_display=1"), 0);
+
+		igt_i915_driver_unload();
+	}
 
 	igt_subtest("basic-reload-inject") {
 		int i = 0;
@@ -366,14 +361,9 @@ igt_main
 
 		/* We expect to hit at least one fault! */
 		igt_assert(i > 1);
-	}
-
-	igt_fixture {
-		if ((err = reload(NULL)))
-			igt_fail(err);
 
-		gem_sanitycheck();
-		gem_exec_store();
-		hda_dynamic_debug(false);
+		/* inject_fault() leaves the module unloaded */
 	}
+
+	/* Subtests should unload the module themselves if they use modparams */
 }
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
  2018-07-27 10:04 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
@ 2018-07-27 10:14 ` Petri Latvala
  2018-07-27 11:51 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Don't reload on exit (rev2) Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Petri Latvala @ 2018-07-27 10:14 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Fri, Jul 27, 2018 at 11:04:35AM +0100, Chris Wilson wrote:
> The next test will happily load whatever module it requires.
> 
> v2: Unload at start, mandate tests cleanup after themselves.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>


Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  tests/drv_module_reload.c | 48 ++++++++++++++++-----------------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 34f55eab1..cf4aea80d 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -220,20 +220,6 @@ static void store_all(int fd)
>  	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
>  }
>  
> -static int
> -reload(const char *opts_i915)
> -{
> -	int err = IGT_EXIT_SUCCESS;
> -
> -	if ((err = igt_i915_driver_unload()))
> -		return err;
> -
> -	if ((err = igt_i915_driver_load(opts_i915)))
> -		return err;
> -
> -	return err;
> -}
> -
>  static int open_parameters(const char *module_name)
>  {
>  	char path[256];
> @@ -340,21 +326,30 @@ hda_dynamic_debug(bool enable)
>  
>  igt_main
>  {
> -	int err;
> +	igt_subtest("basic-reload") {
> +		int load_error;
> +
> +		igt_i915_driver_unload();
>  
> -	igt_fixture
>  		hda_dynamic_debug(true);
> +		load_error = igt_i915_driver_load(NULL);
> +		hda_dynamic_debug(false);
>  
> -	igt_subtest("basic-reload") {
> -		if ((err = reload(NULL)))
> -			igt_fail(err);
> +		igt_assert_eq(load_error, 0);
>  
>  		gem_sanitycheck();
>  		gem_exec_store();
> +
> +		/* only default modparams, can leave module loaded */
>  	}
>  
> -	igt_subtest("basic-no-display")
> -		igt_assert_eq(reload("disable_display=1"), 0);
> +	igt_subtest("basic-no-display") {
> +		igt_i915_driver_unload();
> +
> +		igt_assert_eq(igt_i915_driver_load("disable_display=1"), 0);
> +
> +		igt_i915_driver_unload();
> +	}
>  
>  	igt_subtest("basic-reload-inject") {
>  		int i = 0;
> @@ -366,14 +361,9 @@ igt_main
>  
>  		/* We expect to hit at least one fault! */
>  		igt_assert(i > 1);
> -	}
> -
> -	igt_fixture {
> -		if ((err = reload(NULL)))
> -			igt_fail(err);
>  
> -		gem_sanitycheck();
> -		gem_exec_store();
> -		hda_dynamic_debug(false);
> +		/* inject_fault() leaves the module unloaded */
>  	}
> +
> +	/* Subtests should unload the module themselves if they use modparams */
>  }
> -- 
> 2.18.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Don't reload on exit (rev2)
  2018-07-27 10:04 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
  2018-07-27 10:14 ` Petri Latvala
@ 2018-07-27 11:51 ` Patchwork
  2018-07-27 16:07 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
  2018-07-27 17:39 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-07-27 11:51 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: igt/drv_module_reload: Don't reload on exit (rev2)
URL   : https://patchwork.freedesktop.org/series/47326/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4547 -> IGTPW_1660 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1660 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1660, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47326/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1660:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload-inject:
      fi-pnv-d510:        NOTRUN -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in IGTPW_1660 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      {fi-icl-u}:         NOTRUN -> DMESG-WARN (fdo#107396)

    igt@drv_module_reload@basic-no-display:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#105395) +2

    igt@gem_exec_suspend@basic-s3:
      {fi-skl-caroline}:  NOTRUN -> INCOMPLETE (fdo#104108)

    igt@gem_workarounds@basic-read:
      {fi-icl-u}:         NOTRUN -> FAIL (fdo#107338)

    igt@kms_chamelium@hdmi-hpd-fast:
      fi-kbl-7500u:       SKIP -> FAIL (fdo#102672, fdo#103841)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
      {fi-icl-u}:         NOTRUN -> DMESG-WARN (fdo#107382) +4

    {igt@kms_psr@cursor_plane_move}:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107372) +1

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#107372)

    {igt@kms_psr@primary_page_flip}:
      {fi-icl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       DMESG-WARN (fdo#106725, fdo#106248) -> PASS

    igt@gem_exec_create@basic:
      fi-glk-j4005:       DMESG-WARN (fdo#106745) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       FAIL (fdo#100368) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#106745 https://bugs.freedesktop.org/show_bug.cgi?id=106745
  fdo#107338 https://bugs.freedesktop.org/show_bug.cgi?id=107338
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107382 https://bugs.freedesktop.org/show_bug.cgi?id=107382
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  fdo#107396 https://bugs.freedesktop.org/show_bug.cgi?id=107396


== Participating hosts (48 -> 46) ==

  Additional (5): fi-byt-j1900 fi-skl-caroline fi-icl-u fi-cnl-psr fi-pnv-d510 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * IGT: IGT_4575 -> IGTPW_1660

  CI_DRM_4547: 0a7ab192a697e951b2404f3c1ce42c5fa74f9ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1660: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1660/
  IGT_4575: fe908a01012c9daafafb3410b9407725ca9d4f21 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1660/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for igt/drv_module_reload: Don't reload on exit (rev2)
  2018-07-27 10:04 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
  2018-07-27 10:14 ` Petri Latvala
  2018-07-27 11:51 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Don't reload on exit (rev2) Patchwork
@ 2018-07-27 16:07 ` Patchwork
  2018-07-27 17:39 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-07-27 16:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: igt/drv_module_reload: Don't reload on exit (rev2)
URL   : https://patchwork.freedesktop.org/series/47326/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4560 -> IGTPW_1665 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47326/revisions/2/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1665 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       DMESG-FAIL (fdo#103841) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (51 -> 47) ==

  Additional (1): fi-skl-guc 
  Missing    (5): fi-byt-clapper fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4576 -> IGTPW_1665

  CI_DRM_4560: b73c0ddef408783e556741ac9d3679b7d153e3e1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1665: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1665/
  IGT_4576: bcb37a9b20eeec97f15fac2222408cc2e0b77631 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1665/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.IGT: failure for igt/drv_module_reload: Don't reload on exit (rev2)
  2018-07-27 10:04 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
                   ` (2 preceding siblings ...)
  2018-07-27 16:07 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-27 17:39 ` Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-07-27 17:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: igt/drv_module_reload: Don't reload on exit (rev2)
URL   : https://patchwork.freedesktop.org/series/47326/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4576_full -> IGTPW_1665_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1665_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1665_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47326/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1665_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@perf_pmu@busy-start-vcs0:
      shard-snb:          PASS -> DMESG-WARN

    
    ==== Warnings ====

    igt@gem_exec_reloc@basic-write-cpu:
      shard-snb:          PASS -> SKIP

    igt@perf_pmu@rc6-runtime-pm:
      shard-glk:          PASS -> SKIP
      shard-apl:          PASS -> SKIP
      shard-kbl:          PASS -> SKIP
      shard-hsw:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1665_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_reloc@basic-gtt-read-noreloc:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    {igt@gem_userptr_blits@readonly-unsync}:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_plane_lowres@pipe-a-tiling-x:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          PASS -> FAIL (fdo#103925)

    igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
      shard-glk:          PASS -> FAIL (fdo#103375)

    
    ==== Possible fixes ====

    igt@gem_softpin@evict-active-interruptible:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled:
      shard-glk:          FAIL (fdo#107401) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
      shard-glk:          FAIL (fdo#103375) -> PASS

    igt@prime_vgem@basic-fence-flip:
      shard-kbl:          DMESG-WARN (fdo#106247) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  fdo#107401 https://bugs.freedesktop.org/show_bug.cgi?id=107401
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4576 -> IGTPW_1665
    * Linux: CI_DRM_4547 -> CI_DRM_4560

  CI_DRM_4547: 0a7ab192a697e951b2404f3c1ce42c5fa74f9ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4560: b73c0ddef408783e556741ac9d3679b7d153e3e1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1665: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1665/
  IGT_4576: bcb37a9b20eeec97f15fac2222408cc2e0b77631 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1665/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
  2018-07-27  9:23 ` Petri Latvala
  2018-07-27  9:42   ` Petri Latvala
@ 2018-07-27  9:45   ` Chris Wilson
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-07-27  9:45 UTC (permalink / raw)
  To: Petri Latvala; +Cc: igt-dev

Quoting Petri Latvala (2018-07-27 10:23:28)
> > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > index 34f55eab1..4c06e4caf 100644
> > --- a/tests/drv_module_reload.c
> > +++ b/tests/drv_module_reload.c
> > @@ -340,21 +340,21 @@ hda_dynamic_debug(bool enable)
> >  
> >  igt_main
> >  {
> > -     int err;
> > -
> > -     igt_fixture
> > +     igt_subtest("basic-reload") {
> >               hda_dynamic_debug(true);
> >  
> > -     igt_subtest("basic-reload") {
> > -             if ((err = reload(NULL)))
> > -                     igt_fail(err);
> > +             igt_assert_eq(reload(NULL), 0);
> >  
> >               gem_sanitycheck();
> >               gem_exec_store();
> > +
> > +             hda_dynamic_debug(false);
> >       }
> 
> 
> hda_dynamic_debug is no longer active for other subtests then. Is it
> not useful for them?

Tbh, not sure what value hda_dynamic_debug provides. My presumption was
that it helps show the module ordering on load, in which case we do only
need it once around the basic reload. The purpose of the other tests is
not so much checking the ordinary module load but to investigate other
side-effects. Since it is not the purpose of the other tests to debug
hda, I'd rather not have any extra noise.

> > -     igt_subtest("basic-no-display")
> > +     igt_subtest("basic-no-display") {
> >               igt_assert_eq(reload("disable_display=1"), 0);
> > +             igt_i915_driver_unload();
> > +     }
> >  
> >       igt_subtest("basic-reload-inject") {
> >               int i = 0;
> > @@ -367,13 +367,4 @@ igt_main
> >               /* We expect to hit at least one fault! */
> >               igt_assert(i > 1);
> 
> 
> Tautological assert here btw.

Is it? If inject_load() breaks on the first iteration (no module, or no
fault injection modparam), i is left at 1.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
  2018-07-27  9:23 ` Petri Latvala
@ 2018-07-27  9:42   ` Petri Latvala
  2018-07-27  9:45   ` Chris Wilson
  1 sibling, 0 replies; 9+ messages in thread
From: Petri Latvala @ 2018-07-27  9:42 UTC (permalink / raw)
  To: Chris Wilson, igt-dev

On Fri, Jul 27, 2018 at 12:23:28PM +0300, Petri Latvala wrote:
> >  
> >  	igt_subtest("basic-reload-inject") {
> >  		int i = 0;
> > @@ -367,13 +367,4 @@ igt_main
> >  		/* We expect to hit at least one fault! */
> >  		igt_assert(i > 1);
> 
> 
> Tautological assert here btw.


Okay, scratch that. I had a sip of coffee and managed to read the code
right this time.



-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
  2018-07-27  8:06 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
@ 2018-07-27  9:23 ` Petri Latvala
  2018-07-27  9:42   ` Petri Latvala
  2018-07-27  9:45   ` Chris Wilson
  0 siblings, 2 replies; 9+ messages in thread
From: Petri Latvala @ 2018-07-27  9:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

On Fri, Jul 27, 2018 at 09:06:57AM +0100, Chris Wilson wrote:
> The next test will happily load whatever module it requires.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/drv_module_reload.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)


I was going to comment that leaving a driver loaded with whatever
errorly parameters the tests use is bad... But you changed the
no-display subtest to also ensure the driver is unloaded when it's
done. I tend to forget conventions like that easily, can we have
comments in there stating that subtests should make sure to leave the
driver in either a valid reloaded state or unloaded?


> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 34f55eab1..4c06e4caf 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -340,21 +340,21 @@ hda_dynamic_debug(bool enable)
>  
>  igt_main
>  {
> -	int err;
> -
> -	igt_fixture
> +	igt_subtest("basic-reload") {
>  		hda_dynamic_debug(true);
>  
> -	igt_subtest("basic-reload") {
> -		if ((err = reload(NULL)))
> -			igt_fail(err);
> +		igt_assert_eq(reload(NULL), 0);
>  
>  		gem_sanitycheck();
>  		gem_exec_store();
> +
> +		hda_dynamic_debug(false);
>  	}


hda_dynamic_debug is no longer active for other subtests then. Is it
not useful for them?




>  
> -	igt_subtest("basic-no-display")
> +	igt_subtest("basic-no-display") {
>  		igt_assert_eq(reload("disable_display=1"), 0);
> +		igt_i915_driver_unload();
> +	}
>  
>  	igt_subtest("basic-reload-inject") {
>  		int i = 0;
> @@ -367,13 +367,4 @@ igt_main
>  		/* We expect to hit at least one fault! */
>  		igt_assert(i > 1);


Tautological assert here btw.



-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit
@ 2018-07-27  8:06 Chris Wilson
  2018-07-27  9:23 ` Petri Latvala
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2018-07-27  8:06 UTC (permalink / raw)
  To: igt-dev

The next test will happily load whatever module it requires.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/drv_module_reload.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 34f55eab1..4c06e4caf 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -340,21 +340,21 @@ hda_dynamic_debug(bool enable)
 
 igt_main
 {
-	int err;
-
-	igt_fixture
+	igt_subtest("basic-reload") {
 		hda_dynamic_debug(true);
 
-	igt_subtest("basic-reload") {
-		if ((err = reload(NULL)))
-			igt_fail(err);
+		igt_assert_eq(reload(NULL), 0);
 
 		gem_sanitycheck();
 		gem_exec_store();
+
+		hda_dynamic_debug(false);
 	}
 
-	igt_subtest("basic-no-display")
+	igt_subtest("basic-no-display") {
 		igt_assert_eq(reload("disable_display=1"), 0);
+		igt_i915_driver_unload();
+	}
 
 	igt_subtest("basic-reload-inject") {
 		int i = 0;
@@ -367,13 +367,4 @@ igt_main
 		/* We expect to hit at least one fault! */
 		igt_assert(i > 1);
 	}
-
-	igt_fixture {
-		if ((err = reload(NULL)))
-			igt_fail(err);
-
-		gem_sanitycheck();
-		gem_exec_store();
-		hda_dynamic_debug(false);
-	}
 }
-- 
2.18.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-07-27 17:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 10:04 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
2018-07-27 10:14 ` Petri Latvala
2018-07-27 11:51 ` [igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Don't reload on exit (rev2) Patchwork
2018-07-27 16:07 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2018-07-27 17:39 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2018-07-27  8:06 [igt-dev] [PATCH i-g-t] igt/drv_module_reload: Don't reload on exit Chris Wilson
2018-07-27  9:23 ` Petri Latvala
2018-07-27  9:42   ` Petri Latvala
2018-07-27  9:45   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.