Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
@ 2023-02-09 19:32 Janusz Krzysztofik
  2023-02-09 21:03 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Janusz Krzysztofik @ 2023-02-09 19:32 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx

If any of *-without-i915 subtests fails or skips for any reason, it may
leave the i915 module unloaded while keeping our device list populated
with initially collected data.  In a follow up igt_fixture section we then
try to reopen the device.  If the test has been executed with a device
filter specified, an attempt to open the device finds a matching entry
that belongs to the no longer existing device in that initially collected
device list, fails to stat() it, concludes that's because of the device
having been already open, and returns an error.

Fix this potentially confusing test result by freeing the potentially
outdated device list before continuing with drm_open_driver().

While being at it, add a comment that explains why we call
igt_device_scan() from __igt_device_card_match() but don't force device
rescan, and emit a debug message if we fail in _is_already_opened() on
unsuccessful device stat().

Subtest basic-s3-without-i915: FAIL (9.572s)
(i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
(i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
(i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
(i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
(i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
Test i915_suspend failed.
**** DEBUG ****
(i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
(i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
(i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
(i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
(i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
(i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
(i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
(i915_suspend:9050) igt_core-INFO: Stack trace:
(i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
(i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
(i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
(i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
(i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
(i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
(i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
****  END  ****

Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Riana Tauro <riana.tauro@intel.com>
---
 lib/drmtest.c             |  2 +-
 lib/igt_device_scan.c     |  4 ++++
 tests/i915/i915_suspend.c | 10 +++++++++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 16e80bdfcf..8e2d1ac50b 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
 	 * we cannot even stat the device, so it's of no use - let's claim it's
 	 * already opened
 	 */
-	if (stat(path, &new) != 0)
+	if (igt_debug_on(stat(path, &new) != 0))
 		return true;
 
 	for (int i = 0; i < as_idx; ++i) {
diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 8b767eed20..ae69ed09f1 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
 		return false;
 	memset(card, 0, sizeof(*card));
 
+	/*
+	 * Scan devices in case the user hasn't yet,
+	 * but leave a decision on forced rescan on the user side.
+	 */
 	igt_devices_scan(false);
 
 	if (igt_device_filter_apply(filter) == false)
diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
index 815f1c8a2c..c68110476e 100644
--- a/tests/i915/i915_suspend.c
+++ b/tests/i915/i915_suspend.c
@@ -253,8 +253,16 @@ igt_main
 	igt_subtest("basic-s3-without-i915")
 		test_suspend_without_i915(SUSPEND_STATE_S3);
 
-	igt_fixture
+	igt_fixture {
+		/*
+		 * Since above tests may fail leaving the i915 module unloaded,
+		 * force refresh of device list before opening an i915 device
+		 * by cleaning up the current device list, otherwise we can fail
+		 * if we have been called with a device filter specified.
+		 */
+		igt_devices_free();
 		fd = drm_open_driver(DRIVER_INTEL);
+	}
 
 	igt_subtest("fence-restore-tiled2untiled") {
 		gem_require_mappable_ggtt(fd);
-- 
2.25.1

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

* [igt-dev] ✓ Fi.CI.BAT: success for tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-09 19:32 [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests Janusz Krzysztofik
@ 2023-02-09 21:03 ` Patchwork
  2023-02-10 14:02 ` [igt-dev] [PATCH i-g-t] " Kamil Konieczny
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2023-02-09 21:03 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 4044 bytes --]

== Series Details ==

Series: tests/i915_suspend: Free device list after *-without-i915 subtests
URL   : https://patchwork.freedesktop.org/series/113854/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12722 -> IGTPW_8480
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html

Participating hosts (38 -> 35)
------------------------------

  Missing    (3): fi-kbl-soraka bat-kbl-2 fi-snb-2520m 

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - fi-bsw-nick:        NOTRUN -> [SKIP][1] ([fdo#109271]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-bsw-nick/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
    - fi-skl-6600u:       NOTRUN -> [SKIP][2] ([fdo#109271])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-skl-6600u/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  
#### Possible fixes ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-bsw-nick:        [ABORT][3] -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@dmabuf@all-tests@dma_fence_unwrap:
    - fi-bsw-nick:        [DMESG-FAIL][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_unwrap.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_unwrap.html

  * igt@gem_exec_suspend@basic-s0@smem:
    - fi-skl-6600u:       [ABORT][7] ([i915#5122]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/fi-skl-6600u/igt@gem_exec_suspend@basic-s0@smem.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-skl-6600u/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-n3050:       [FAIL][9] ([i915#6298]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5251]: https://gitlab.freedesktop.org/drm/intel/issues/5251
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6311]: https://gitlab.freedesktop.org/drm/intel/issues/6311
  [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7155 -> IGTPW_8480

  CI-20190529: 20190529
  CI_DRM_12722: ec3cb908765a89bf72518590473c464a543372ff @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8480: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html
  IGT_7155: 75c508d4e19c65683d4060cb3a772df600aaf23e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

-igt@kms_plane_scaling@invalid-parameters

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html

[-- Attachment #2: Type: text/html, Size: 4382 bytes --]

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-09 19:32 [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests Janusz Krzysztofik
  2023-02-09 21:03 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
@ 2023-02-10 14:02 ` Kamil Konieczny
  2023-02-10 14:21   ` Janusz Krzysztofik
  2023-02-10 15:30 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork
  2023-02-10 21:33 ` [igt-dev] [PATCH i-g-t] " Janusz Krzysztofik
  3 siblings, 1 reply; 10+ messages in thread
From: Kamil Konieczny @ 2023-02-10 14:02 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx

Hi Janusz,

I have one nit, see below.

On 2023-02-09 at 20:32:31 +0100, Janusz Krzysztofik wrote:
> If any of *-without-i915 subtests fails or skips for any reason, it may
> leave the i915 module unloaded while keeping our device list populated
> with initially collected data.  In a follow up igt_fixture section we then
> try to reopen the device.  If the test has been executed with a device
> filter specified, an attempt to open the device finds a matching entry
> that belongs to the no longer existing device in that initially collected
> device list, fails to stat() it, concludes that's because of the device
> having been already open, and returns an error.
> 
> Fix this potentially confusing test result by freeing the potentially
> outdated device list before continuing with drm_open_driver().
> 
> While being at it, add a comment that explains why we call
> igt_device_scan() from __igt_device_card_match() but don't force device
> rescan, and emit a debug message if we fail in _is_already_opened() on
> unsuccessful device stat().
> 
> Subtest basic-s3-without-i915: FAIL (9.572s)
> (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> Test i915_suspend failed.
> **** DEBUG ****
> (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
> (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
> (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> (i915_suspend:9050) igt_core-INFO: Stack trace:
> (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
> (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
> (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
> (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
> (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
> (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> ****  END  ****
> 
> Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Riana Tauro <riana.tauro@intel.com>
> ---
>  lib/drmtest.c             |  2 +-
>  lib/igt_device_scan.c     |  4 ++++
>  tests/i915/i915_suspend.c | 10 +++++++++-
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 16e80bdfcf..8e2d1ac50b 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
>  	 * we cannot even stat the device, so it's of no use - let's claim it's
>  	 * already opened
>  	 */
> -	if (stat(path, &new) != 0)
> +	if (igt_debug_on(stat(path, &new) != 0))

This one looks a little too much, for example it can return -ENOENT

Rest looks ok.

Regards,
Kamil

>  		return true;
>  
>  	for (int i = 0; i < as_idx; ++i) {
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 8b767eed20..ae69ed09f1 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
>  		return false;
>  	memset(card, 0, sizeof(*card));
>  
> +	/*
> +	 * Scan devices in case the user hasn't yet,
> +	 * but leave a decision on forced rescan on the user side.
> +	 */
>  	igt_devices_scan(false);
>  
>  	if (igt_device_filter_apply(filter) == false)
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> index 815f1c8a2c..c68110476e 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -253,8 +253,16 @@ igt_main
>  	igt_subtest("basic-s3-without-i915")
>  		test_suspend_without_i915(SUSPEND_STATE_S3);
>  
> -	igt_fixture
> +	igt_fixture {
> +		/*
> +		 * Since above tests may fail leaving the i915 module unloaded,
> +		 * force refresh of device list before opening an i915 device
> +		 * by cleaning up the current device list, otherwise we can fail
> +		 * if we have been called with a device filter specified.
> +		 */
> +		igt_devices_free();
>  		fd = drm_open_driver(DRIVER_INTEL);
> +	}
>  
>  	igt_subtest("fence-restore-tiled2untiled") {
>  		gem_require_mappable_ggtt(fd);
> -- 
> 2.25.1
> 

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-10 14:02 ` [igt-dev] [PATCH i-g-t] " Kamil Konieczny
@ 2023-02-10 14:21   ` Janusz Krzysztofik
  2023-02-10 16:28     ` Kamil Konieczny
  0 siblings, 1 reply; 10+ messages in thread
From: Janusz Krzysztofik @ 2023-02-10 14:21 UTC (permalink / raw)
  To: Kamil Konieczny, igt-dev, Riana Tauro, Janusz Krzysztofik,
	intel-gfx

On Friday, 10 February 2023 15:02:59 CET Kamil Konieczny wrote:
> Hi Janusz,
> 
> I have one nit, see below.
> 
> On 2023-02-09 at 20:32:31 +0100, Janusz Krzysztofik wrote:
> > If any of *-without-i915 subtests fails or skips for any reason, it may
> > leave the i915 module unloaded while keeping our device list populated
> > with initially collected data.  In a follow up igt_fixture section we then
> > try to reopen the device.  If the test has been executed with a device
> > filter specified, an attempt to open the device finds a matching entry
> > that belongs to the no longer existing device in that initially collected
> > device list, fails to stat() it, concludes that's because of the device
> > having been already open, and returns an error.
> > 
> > Fix this potentially confusing test result by freeing the potentially
> > outdated device list before continuing with drm_open_driver().
> > 
> > While being at it, add a comment that explains why we call
> > igt_device_scan() from __igt_device_card_match() but don't force device
> > rescan, and emit a debug message if we fail in _is_already_opened() on
> > unsuccessful device stat().
> > 
> > Subtest basic-s3-without-i915: FAIL (9.572s)
> > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > Test i915_suspend failed.
> > **** DEBUG ****
> > (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
> > (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
> > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > (i915_suspend:9050) igt_core-INFO: Stack trace:
> > (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
> > (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
> > (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
> > (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
> > (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
> > (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> > (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> > ****  END  ****
> > 
> > Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Riana Tauro <riana.tauro@intel.com>
> > ---
> >  lib/drmtest.c             |  2 +-
> >  lib/igt_device_scan.c     |  4 ++++
> >  tests/i915/i915_suspend.c | 10 +++++++++-
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 16e80bdfcf..8e2d1ac50b 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
> >  	 * we cannot even stat the device, so it's of no use - let's claim it's
> >  	 * already opened
> >  	 */
> > -	if (stat(path, &new) != 0)
> > +	if (igt_debug_on(stat(path, &new) != 0))
> 
> This one looks a little too much, for example it can return -ENOENT

I'm not sure what you mean by "too much", and why you think that's wrong.  
Maybe if you proposed some other approach, more proper in your opinion, not 
only suggest that something is wrong, than I would be more clear about your 
point.

I've surrounded the comparison of stat() return value against 0 with 
igt_debug_on().  The only effect of this is that a debug message will be 
emitted should stat() return an error.  Otherwise this is completely 
transparent, i.e., this doesn't affect the algorithm in any way.  The purpose 
of emitting the debug message is to make root cause analysis easier by 
providing a hint which exit point from _is_already_opened() was followed, IOW, 
why the device was believed to be open.

Thanks,
Janusz


> 
> Rest looks ok.
> 
> Regards,
> Kamil
> 
> >  		return true;
> >  
> >  	for (int i = 0; i < as_idx; ++i) {
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 8b767eed20..ae69ed09f1 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
> >  		return false;
> >  	memset(card, 0, sizeof(*card));
> >  
> > +	/*
> > +	 * Scan devices in case the user hasn't yet,
> > +	 * but leave a decision on forced rescan on the user side.
> > +	 */
> >  	igt_devices_scan(false);
> >  
> >  	if (igt_device_filter_apply(filter) == false)
> > diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> > index 815f1c8a2c..c68110476e 100644
> > --- a/tests/i915/i915_suspend.c
> > +++ b/tests/i915/i915_suspend.c
> > @@ -253,8 +253,16 @@ igt_main
> >  	igt_subtest("basic-s3-without-i915")
> >  		test_suspend_without_i915(SUSPEND_STATE_S3);
> >  
> > -	igt_fixture
> > +	igt_fixture {
> > +		/*
> > +		 * Since above tests may fail leaving the i915 module unloaded,
> > +		 * force refresh of device list before opening an i915 device
> > +		 * by cleaning up the current device list, otherwise we can fail
> > +		 * if we have been called with a device filter specified.
> > +		 */
> > +		igt_devices_free();
> >  		fd = drm_open_driver(DRIVER_INTEL);
> > +	}
> >  
> >  	igt_subtest("fence-restore-tiled2untiled") {
> >  		gem_require_mappable_ggtt(fd);
> 




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

* [igt-dev] ✗ Fi.CI.IGT: failure for tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-09 19:32 [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests Janusz Krzysztofik
  2023-02-09 21:03 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
  2023-02-10 14:02 ` [igt-dev] [PATCH i-g-t] " Kamil Konieczny
@ 2023-02-10 15:30 ` Patchwork
  2023-02-10 21:33 ` [igt-dev] [PATCH i-g-t] " Janusz Krzysztofik
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2023-02-10 15:30 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 24003 bytes --]

== Series Details ==

Series: tests/i915_suspend: Free device list after *-without-i915 subtests
URL   : https://patchwork.freedesktop.org/series/113854/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12722_full -> IGTPW_8480_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_8480_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_8480_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://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html

Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-snb:          [PASS][1] -> [FAIL][2] +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-snb7/igt@i915_suspend@fence-restore-tiled2untiled.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-snb5/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@i915_suspend@forcewake:
    - shard-glk:          [PASS][3] -> [FAIL][4] +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-glk7/igt@i915_suspend@forcewake.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-glk9/igt@i915_suspend@forcewake.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_suspend@debugfs-reader:
    - {shard-rkl}:        [PASS][5] -> [FAIL][6] +4 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-5/igt@i915_suspend@debugfs-reader.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@i915_suspend@debugfs-reader.html
    - {shard-tglu-9}:     NOTRUN -> [FAIL][7] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-9/igt@i915_suspend@debugfs-reader.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - {shard-dg1}:        NOTRUN -> [FAIL][8]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-dg1-16/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@i915_suspend@sysfs-reader:
    - {shard-dg1}:        [PASS][9] -> [FAIL][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-dg1-15/igt@i915_suspend@sysfs-reader.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-dg1-18/igt@i915_suspend@sysfs-reader.html
    - {shard-tglu}:       [PASS][11] -> [FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-tglu-8/igt@i915_suspend@sysfs-reader.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-6/igt@i915_suspend@sysfs-reader.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gen9_exec_parse@allowed-single:
    - shard-glk:          [PASS][13] -> [ABORT][14] ([i915#5566])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-glk3/igt@gen9_exec_parse@allowed-single.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-glk1/igt@gen9_exec_parse@allowed-single.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ac-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([i915#79])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ac-hdmi-a1-hdmi-a2.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ac-hdmi-a1-hdmi-a2.html

  * igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][17] ([fdo#109271]) +20 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-snb6/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1.html

  
#### Possible fixes ####

  * igt@drm_fdinfo@idle@rcs0:
    - {shard-rkl}:        [FAIL][18] ([i915#7742]) -> [PASS][19] +1 similar issue
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@drm_fdinfo@idle@rcs0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@drm_fdinfo@idle@rcs0.html

  * igt@drm_read@empty-block:
    - {shard-rkl}:        [SKIP][20] ([i915#4098]) -> [PASS][21] +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@drm_read@empty-block.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@drm_read@empty-block.html

  * igt@drm_read@invalid-buffer:
    - {shard-tglu}:       [SKIP][22] ([i915#1845]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-tglu-6/igt@drm_read@invalid-buffer.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-8/igt@drm_read@invalid-buffer.html

  * igt@fbdev@nullptr:
    - {shard-rkl}:        [SKIP][24] ([i915#2582]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@fbdev@nullptr.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@fbdev@nullptr.html

  * igt@gem_ctx_exec@basic-nohangcheck:
    - {shard-rkl}:        [FAIL][26] ([i915#6268]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@gem_ctx_exec@basic-nohangcheck.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - {shard-rkl}:        [FAIL][28] ([i915#2842]) -> [PASS][29] +2 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@gem_exec_fair@basic-none-vip@rcs0.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@gem_exec_reloc@basic-gtt-cpu-active:
    - {shard-rkl}:        [SKIP][30] ([i915#3281]) -> [PASS][31] +8 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@gem_exec_reloc@basic-gtt-cpu-active.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@gem_exec_reloc@basic-gtt-cpu-active.html

  * igt@gem_tiled_partial_pwrite_pread@writes:
    - {shard-rkl}:        [SKIP][32] ([i915#3282]) -> [PASS][33] +2 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@gem_tiled_partial_pwrite_pread@writes.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@gem_tiled_partial_pwrite_pread@writes.html

  * igt@gen9_exec_parse@unaligned-access:
    - {shard-rkl}:        [SKIP][34] ([i915#2527]) -> [PASS][35] +1 similar issue
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-6/igt@gen9_exec_parse@unaligned-access.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@gen9_exec_parse@unaligned-access.html

  * igt@i915_pm_dc@dc6-psr:
    - {shard-rkl}:        [SKIP][36] ([i915#658]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@i915_pm_dc@dc6-psr.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@drm-resources-equal:
    - {shard-tglu}:       [SKIP][38] ([i915#3547]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-tglu-6/igt@i915_pm_rpm@drm-resources-equal.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-2/igt@i915_pm_rpm@drm-resources-equal.html

  * igt@i915_pm_sseu@full-enable:
    - {shard-rkl}:        [SKIP][40] ([i915#4387]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@i915_pm_sseu@full-enable.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@i915_pm_sseu@full-enable.html

  * igt@i915_suspend@basic-s3-without-i915:
    - {shard-rkl}:        [FAIL][42] ([fdo#103375]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@i915_suspend@basic-s3-without-i915.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_rc_ccs_cc:
    - {shard-tglu}:       [SKIP][44] ([i915#7651]) -> [PASS][45] +3 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-tglu-6/igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_rc_ccs_cc.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-1/igt@kms_ccs@pipe-b-bad-rotation-90-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs_cc:
    - {shard-tglu}:       [SKIP][46] ([i915#1845] / [i915#7651]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-tglu-6/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs_cc.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-tglu-2/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
    - shard-glk:          [FAIL][48] ([i915#2346]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html

  * igt@kms_frontbuffer_tracking@psr-modesetfrombusy:
    - {shard-rkl}:        [SKIP][50] ([i915#1849] / [i915#4098]) -> [PASS][51] +13 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@kms_frontbuffer_tracking@psr-modesetfrombusy.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@kms_frontbuffer_tracking@psr-modesetfrombusy.html

  * igt@kms_plane@plane-position-hole-dpms@pipe-b-planes:
    - {shard-rkl}:        [SKIP][52] ([i915#1849]) -> [PASS][53] +1 similar issue
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-3/igt@kms_plane@plane-position-hole-dpms@pipe-b-planes.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@kms_plane@plane-position-hole-dpms@pipe-b-planes.html

  * igt@kms_psr@sprite_mmap_cpu:
    - {shard-rkl}:        [SKIP][54] ([i915#1072]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-5/igt@kms_psr@sprite_mmap_cpu.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@kms_psr@sprite_mmap_cpu.html

  * igt@kms_universal_plane@universal-plane-pipe-a-sanity:
    - {shard-rkl}:        [SKIP][56] ([i915#1845] / [i915#4070] / [i915#4098]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@kms_universal_plane@universal-plane-pipe-a-sanity.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@kms_universal_plane@universal-plane-pipe-a-sanity.html

  * igt@kms_vblank@pipe-b-query-idle:
    - {shard-rkl}:        [SKIP][58] ([i915#1845] / [i915#4098]) -> [PASS][59] +38 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-1/igt@kms_vblank@pipe-b-query-idle.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-6/igt@kms_vblank@pipe-b-query-idle.html

  * igt@prime_vgem@basic-fence-read:
    - {shard-rkl}:        [SKIP][60] ([fdo#109295] / [i915#3291] / [i915#3708]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-6/igt@prime_vgem@basic-fence-read.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@coherency-gtt:
    - {shard-rkl}:        [SKIP][62] ([fdo#109295] / [fdo#111656] / [i915#3708]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12722/shard-rkl-2/igt@prime_vgem@coherency-gtt.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/shard-rkl-5/igt@prime_vgem@coherency-gtt.html

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

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109303]: https://bugs.freedesktop.org/show_bug.cgi?id=109303
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110542]: https://bugs.freedesktop.org/show_bug.cgi?id=110542
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1722]: https://gitlab.freedesktop.org/drm/intel/issues/1722
  [i915#1755]: https://gitlab.freedesktop.org/drm/intel/issues/1755
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2532]: https://gitlab.freedesktop.org/drm/intel/issues/2532
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3318]: https://gitlab.freedesktop.org/drm/intel/issues/3318
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
  [i915#3536]: https://gitlab.freedesktop.org/drm/intel/issues/3536
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3558]: https://gitlab.freedesktop.org/drm/intel/issues/3558
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3938]: https://gitlab.freedesktop.org/drm/intel/issues/3938
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4818]: https://gitlab.freedesktop.org/drm/intel/issues/4818
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4884]: https://gitlab.freedesktop.org/drm/intel/issues/4884
  [i915#5115]: https://gitlab.freedesktop.org/drm/intel/issues/5115
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6230]: https://gitlab.freedesktop.org/drm/intel/issues/6230
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6258]: https://gitlab.freedesktop.org/drm/intel/issues/6258
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
  [i915#6335]: https://gitlab.freedesktop.org/drm/intel/issues/6335
  [i915#6344]: https://gitlab.freedesktop.org/drm/intel/issues/6344
  [i915#6412]: https://gitlab.freedesktop.org/drm/intel/issues/6412
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7052]: https://gitlab.freedesktop.org/drm/intel/issues/7052
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7582]: https://gitlab.freedesktop.org/drm/intel/issues/7582
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7949]: https://gitlab.freedesktop.org/drm/intel/issues/7949
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#7984]: https://gitlab.freedesktop.org/drm/intel/issues/7984
  [i915#8018]: https://gitlab.freedesktop.org/drm/intel/issues/8018


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7155 -> IGTPW_8480

  CI-20190529: 20190529
  CI_DRM_12722: ec3cb908765a89bf72518590473c464a543372ff @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8480: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html
  IGT_7155: 75c508d4e19c65683d4060cb3a772df600aaf23e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8480/index.html

[-- Attachment #2: Type: text/html, Size: 17093 bytes --]

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-10 14:21   ` Janusz Krzysztofik
@ 2023-02-10 16:28     ` Kamil Konieczny
  0 siblings, 0 replies; 10+ messages in thread
From: Kamil Konieczny @ 2023-02-10 16:28 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx

Hi Janusz,

On 2023-02-10 at 15:21:20 +0100, Janusz Krzysztofik wrote:
> On Friday, 10 February 2023 15:02:59 CET Kamil Konieczny wrote:
> > Hi Janusz,
> > 
> > I have one nit, see below.
> > 
> > On 2023-02-09 at 20:32:31 +0100, Janusz Krzysztofik wrote:
> > > If any of *-without-i915 subtests fails or skips for any reason, it may
> > > leave the i915 module unloaded while keeping our device list populated
> > > with initially collected data.  In a follow up igt_fixture section we then
> > > try to reopen the device.  If the test has been executed with a device
> > > filter specified, an attempt to open the device finds a matching entry
> > > that belongs to the no longer existing device in that initially collected
> > > device list, fails to stat() it, concludes that's because of the device
> > > having been already open, and returns an error.
> > > 
> > > Fix this potentially confusing test result by freeing the potentially
> > > outdated device list before continuing with drm_open_driver().
> > > 
> > > While being at it, add a comment that explains why we call
> > > igt_device_scan() from __igt_device_card_match() but don't force device
> > > rescan, and emit a debug message if we fail in _is_already_opened() on
> > > unsuccessful device stat().
> > > 
> > > Subtest basic-s3-without-i915: FAIL (9.572s)
> > > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > > Test i915_suspend failed.
> > > **** DEBUG ****
> > > (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
> > > (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
> > > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > > (i915_suspend:9050) igt_core-INFO: Stack trace:
> > > (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
> > > (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
> > > (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
> > > (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
> > > (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
> > > (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> > > (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> > > ****  END  ****
> > > 
> > > Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
> > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > Cc: Riana Tauro <riana.tauro@intel.com>
> > > ---
> > >  lib/drmtest.c             |  2 +-
> > >  lib/igt_device_scan.c     |  4 ++++
> > >  tests/i915/i915_suspend.c | 10 +++++++++-
> > >  3 files changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index 16e80bdfcf..8e2d1ac50b 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
> > >  	 * we cannot even stat the device, so it's of no use - let's claim it's
> > >  	 * already opened
> > >  	 */
> > > -	if (stat(path, &new) != 0)
> > > +	if (igt_debug_on(stat(path, &new) != 0))
> > 
> > This one looks a little too much, for example it can return -ENOENT
> 
> I'm not sure what you mean by "too much", and why you think that's wrong.  
> Maybe if you proposed some other approach, more proper in your opinion, not 
> only suggest that something is wrong, than I would be more clear about your 
> point.
> 
> I've surrounded the comparison of stat() return value against 0 with 
> igt_debug_on().  The only effect of this is that a debug message will be 
> emitted should stat() return an error.  Otherwise this is completely 
> transparent, i.e., this doesn't affect the algorithm in any way.  The purpose 
> of emitting the debug message is to make root cause analysis easier by 
> providing a hint which exit point from _is_already_opened() was followed, IOW, 
> why the device was believed to be open.
> 
> Thanks,
> Janusz

Now I looked and checked igt_debug_on and it looks ok here, sorry.

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Regards,
Kamil

> 
> 
> > 
> > Rest looks ok.
> > 
> > Regards,
> > Kamil
> > 
> > >  		return true;
> > >  
> > >  	for (int i = 0; i < as_idx; ++i) {
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index 8b767eed20..ae69ed09f1 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
> > >  		return false;
> > >  	memset(card, 0, sizeof(*card));
> > >  
> > > +	/*
> > > +	 * Scan devices in case the user hasn't yet,
> > > +	 * but leave a decision on forced rescan on the user side.
> > > +	 */
> > >  	igt_devices_scan(false);
> > >  
> > >  	if (igt_device_filter_apply(filter) == false)
> > > diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> > > index 815f1c8a2c..c68110476e 100644
> > > --- a/tests/i915/i915_suspend.c
> > > +++ b/tests/i915/i915_suspend.c
> > > @@ -253,8 +253,16 @@ igt_main
> > >  	igt_subtest("basic-s3-without-i915")
> > >  		test_suspend_without_i915(SUSPEND_STATE_S3);
> > >  
> > > -	igt_fixture
> > > +	igt_fixture {
> > > +		/*
> > > +		 * Since above tests may fail leaving the i915 module unloaded,
> > > +		 * force refresh of device list before opening an i915 device
> > > +		 * by cleaning up the current device list, otherwise we can fail
> > > +		 * if we have been called with a device filter specified.
> > > +		 */
> > > +		igt_devices_free();
> > >  		fd = drm_open_driver(DRIVER_INTEL);
> > > +	}
> > >  
> > >  	igt_subtest("fence-restore-tiled2untiled") {
> > >  		gem_require_mappable_ggtt(fd);
> > 
> 
> 
> 
> 

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

* Re: [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-09 19:32 [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests Janusz Krzysztofik
                   ` (2 preceding siblings ...)
  2023-02-10 15:30 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork
@ 2023-02-10 21:33 ` Janusz Krzysztofik
  2023-02-13  9:51   ` [igt-dev] [Intel-gfx] " Zbigniew Kempczyński
  3 siblings, 1 reply; 10+ messages in thread
From: Janusz Krzysztofik @ 2023-02-10 21:33 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx

On Thursday, 9 February 2023 20:32:31 CET Janusz Krzysztofik wrote:
> If any of *-without-i915 subtests fails or skips for any reason, it may
> leave the i915 module unloaded while keeping our device list populated
> with initially collected data.  In a follow up igt_fixture section we then
> try to reopen the device.  If the test has been executed with a device
> filter specified, an attempt to open the device finds a matching entry
> that belongs to the no longer existing device in that initially collected
> device list, fails to stat() it, concludes that's because of the device
> having been already open, and returns an error.
> 
> Fix this potentially confusing test result by freeing the potentially
> outdated device list before continuing with drm_open_driver().

Freeing device list occurred not safe if device scan was not performed before.  
I can see 3 potential solutions:
1) force device rescan instead of free before calling drm_open_driver(),
2) teach igt_device_free() to return immediately if the device list has not 
   been allocated,
3) provide a has_device_list() helper for to be used if not sure before 
   calling igt_device_free().

Any preferences?

Thanks,
Janusz

> 
> While being at it, add a comment that explains why we call
> igt_device_scan() from __igt_device_card_match() but don't force device
> rescan, and emit a debug message if we fail in _is_already_opened() on
> unsuccessful device stat().
> 
> Subtest basic-s3-without-i915: FAIL (9.572s)
> (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> Test i915_suspend failed.
> **** DEBUG ****
> (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
> (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
> (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> (i915_suspend:9050) igt_core-INFO: Stack trace:
> (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
> (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
> (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
> (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
> (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
> (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> ****  END  ****
> 
> Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Riana Tauro <riana.tauro@intel.com>
> ---
>  lib/drmtest.c             |  2 +-
>  lib/igt_device_scan.c     |  4 ++++
>  tests/i915/i915_suspend.c | 10 +++++++++-
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 16e80bdfcf..8e2d1ac50b 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
>  	 * we cannot even stat the device, so it's of no use - let's claim it's
>  	 * already opened
>  	 */
> -	if (stat(path, &new) != 0)
> +	if (igt_debug_on(stat(path, &new) != 0))
>  		return true;
>  
>  	for (int i = 0; i < as_idx; ++i) {
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 8b767eed20..ae69ed09f1 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
>  		return false;
>  	memset(card, 0, sizeof(*card));
>  
> +	/*
> +	 * Scan devices in case the user hasn't yet,
> +	 * but leave a decision on forced rescan on the user side.
> +	 */
>  	igt_devices_scan(false);
>  
>  	if (igt_device_filter_apply(filter) == false)
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> index 815f1c8a2c..c68110476e 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -253,8 +253,16 @@ igt_main
>  	igt_subtest("basic-s3-without-i915")
>  		test_suspend_without_i915(SUSPEND_STATE_S3);
>  
> -	igt_fixture
> +	igt_fixture {
> +		/*
> +		 * Since above tests may fail leaving the i915 module unloaded,
> +		 * force refresh of device list before opening an i915 device
> +		 * by cleaning up the current device list, otherwise we can fail
> +		 * if we have been called with a device filter specified.
> +		 */
> +		igt_devices_free();
>  		fd = drm_open_driver(DRIVER_INTEL);
> +	}
>  
>  	igt_subtest("fence-restore-tiled2untiled") {
>  		gem_require_mappable_ggtt(fd);
> 




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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-10 21:33 ` [igt-dev] [PATCH i-g-t] " Janusz Krzysztofik
@ 2023-02-13  9:51   ` Zbigniew Kempczyński
  2023-02-13 10:10     ` Janusz Krzysztofik
  2023-02-14  6:15     ` Petri Latvala
  0 siblings, 2 replies; 10+ messages in thread
From: Zbigniew Kempczyński @ 2023-02-13  9:51 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev, intel-gfx

On Fri, Feb 10, 2023 at 10:33:21PM +0100, Janusz Krzysztofik wrote:
> On Thursday, 9 February 2023 20:32:31 CET Janusz Krzysztofik wrote:
> > If any of *-without-i915 subtests fails or skips for any reason, it may
> > leave the i915 module unloaded while keeping our device list populated
> > with initially collected data.  In a follow up igt_fixture section we then
> > try to reopen the device.  If the test has been executed with a device
> > filter specified, an attempt to open the device finds a matching entry
> > that belongs to the no longer existing device in that initially collected
> > device list, fails to stat() it, concludes that's because of the device
> > having been already open, and returns an error.
> > 
> > Fix this potentially confusing test result by freeing the potentially
> > outdated device list before continuing with drm_open_driver().
> 
> Freeing device list occurred not safe if device scan was not performed before.  
> I can see 3 potential solutions:
> 1) force device rescan instead of free before calling drm_open_driver(),
> 2) teach igt_device_free() to return immediately if the device list has not 
>    been allocated,
> 3) provide a has_device_list() helper for to be used if not sure before 
>    calling igt_device_free().
> 
> Any preferences?

I would enforce rescan.

BTW I wonder how it can happen if runner is executing each subtest
in new process so you're starting from scratch and rescan should be
executed automatically.

Is is the case you're running few tests from the console?

--
Zbigniew


> 
> Thanks,
> Janusz
> 
> > 
> > While being at it, add a comment that explains why we call
> > igt_device_scan() from __igt_device_card_match() but don't force device
> > rescan, and emit a debug message if we fail in _is_already_opened() on
> > unsuccessful device stat().
> > 
> > Subtest basic-s3-without-i915: FAIL (9.572s)
> > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > Test i915_suspend failed.
> > **** DEBUG ****
> > (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using filter 0: pci:vendor=intel,device=dg2
> > (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/dri/renderD128
> > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already opened
> > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function drm_open_driver, file ../lib/drmtest.c:639:
> > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or directory
> > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset flags 0x1 (intel)
> > (i915_suspend:9050) igt_core-INFO: Stack trace:
> > (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 __igt_abort()
> > (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 drm_open_driver()
> > (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 __igt_unique____real_main245()
> > (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 main()
> > (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
> > (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
> > (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> > ****  END  ****
> > 
> > Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed for basic-s2idle-without-i915")
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Riana Tauro <riana.tauro@intel.com>
> > ---
> >  lib/drmtest.c             |  2 +-
> >  lib/igt_device_scan.c     |  4 ++++
> >  tests/i915/i915_suspend.c | 10 +++++++++-
> >  3 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 16e80bdfcf..8e2d1ac50b 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int as_idx)
> >  	 * we cannot even stat the device, so it's of no use - let's claim it's
> >  	 * already opened
> >  	 */
> > -	if (stat(path, &new) != 0)
> > +	if (igt_debug_on(stat(path, &new) != 0))
> >  		return true;
> >  
> >  	for (int i = 0; i < as_idx; ++i) {
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 8b767eed20..ae69ed09f1 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char *filter,
> >  		return false;
> >  	memset(card, 0, sizeof(*card));
> >  
> > +	/*
> > +	 * Scan devices in case the user hasn't yet,
> > +	 * but leave a decision on forced rescan on the user side.
> > +	 */
> >  	igt_devices_scan(false);
> >  
> >  	if (igt_device_filter_apply(filter) == false)
> > diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> > index 815f1c8a2c..c68110476e 100644
> > --- a/tests/i915/i915_suspend.c
> > +++ b/tests/i915/i915_suspend.c
> > @@ -253,8 +253,16 @@ igt_main
> >  	igt_subtest("basic-s3-without-i915")
> >  		test_suspend_without_i915(SUSPEND_STATE_S3);
> >  
> > -	igt_fixture
> > +	igt_fixture {
> > +		/*
> > +		 * Since above tests may fail leaving the i915 module unloaded,
> > +		 * force refresh of device list before opening an i915 device
> > +		 * by cleaning up the current device list, otherwise we can fail
> > +		 * if we have been called with a device filter specified.
> > +		 */
> > +		igt_devices_free();
> >  		fd = drm_open_driver(DRIVER_INTEL);
> > +	}
> >  
> >  	igt_subtest("fence-restore-tiled2untiled") {
> >  		gem_require_mappable_ggtt(fd);
> > 
> 
> 
> 
> 

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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-13  9:51   ` [igt-dev] [Intel-gfx] " Zbigniew Kempczyński
@ 2023-02-13 10:10     ` Janusz Krzysztofik
  2023-02-14  6:15     ` Petri Latvala
  1 sibling, 0 replies; 10+ messages in thread
From: Janusz Krzysztofik @ 2023-02-13 10:10 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev, intel-gfx

On Monday, 13 February 2023 10:51:39 CET Zbigniew Kempczyński wrote:
> On Fri, Feb 10, 2023 at 10:33:21PM +0100, Janusz Krzysztofik wrote:
> > On Thursday, 9 February 2023 20:32:31 CET Janusz Krzysztofik wrote:
> > > If any of *-without-i915 subtests fails or skips for any reason, it may
> > > leave the i915 module unloaded while keeping our device list populated
> > > with initially collected data.  In a follow up igt_fixture section we 
then
> > > try to reopen the device.  If the test has been executed with a device
> > > filter specified, an attempt to open the device finds a matching entry
> > > that belongs to the no longer existing device in that initially 
collected
> > > device list, fails to stat() it, concludes that's because of the device
> > > having been already open, and returns an error.
> > > 
> > > Fix this potentially confusing test result by freeing the potentially
> > > outdated device list before continuing with drm_open_driver().
> > 
> > Freeing device list occurred not safe if device scan was not performed 
before.  
> > I can see 3 potential solutions:
> > 1) force device rescan instead of free before calling drm_open_driver(),
> > 2) teach igt_device_free() to return immediately if the device list has 
not 
> >    been allocated,
> > 3) provide a has_device_list() helper for to be used if not sure before 
> >    calling igt_device_free().
> > 
> > Any preferences?
> 
> I would enforce rescan.
> 
> BTW I wonder how it can happen if runner is executing each subtest
> in new process so you're starting from scratch and rescan should be
> executed automatically.

The scenario I'm trying to address is different: a subtest fails, leaving the 
i915 module unloaded but the device list populated.  Then, before the test 
exist, open_drm_driver() called from a follow up igt_fixture section falsely 
detects the (non-existent) device as already open and fails instead of 
reloading the module.

While that late failure shouldn't affect results of the subtest, debug 
messages triggered from that failed device_open_driver() can occur potentially 
misleading to anyone watching CI results, then should be avoided if possible.

Thanks,
Janusz


> 
> Is is the case you're running few tests from the console?
> 
> --
> Zbigniew
> 
> 
> > 
> > Thanks,
> > Janusz
> > 
> > > 
> > > While being at it, add a comment that explains why we call
> > > igt_device_scan() from __igt_device_card_match() but don't force device
> > > rescan, and emit a debug message if we fail in _is_already_opened() on
> > > unsuccessful device stat().
> > > 
> > > Subtest basic-s3-without-i915: FAIL (9.572s)
> > > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already 
opened
> > > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function 
drm_open_driver, file ../lib/drmtest.c:639:
> > > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or 
directory
> > > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset 
flags 0x1 (intel)
> > > Test i915_suspend failed.
> > > **** DEBUG ****
> > > (i915_suspend:9050) drmtest-DEBUG: Looking for devices to open using 
filter 0: pci:vendor=intel,device=dg2
> > > (i915_suspend:9050) drmtest-DEBUG: Filter matched /dev/dri/card0 | /dev/
dri/renderD128
> > > (i915_suspend:9050) drmtest-WARNING: card maching filter 0 is already 
opened
> > > (i915_suspend:9050) drmtest-CRITICAL: Test abort in function 
drm_open_driver, file ../lib/drmtest.c:639:
> > > (i915_suspend:9050) drmtest-CRITICAL: abort condition: fd < 0
> > > (i915_suspend:9050) drmtest-CRITICAL: Last errno: 2, No such file or 
directory
> > > (i915_suspend:9050) drmtest-CRITICAL: No known gpu found for chipset 
flags 0x1 (intel)
> > > (i915_suspend:9050) igt_core-INFO: Stack trace:
> > > (i915_suspend:9050) igt_core-INFO:   #0 ../lib/igt_core.c:2066 
__igt_abort()
> > > (i915_suspend:9050) igt_core-INFO:   #1 ../lib/drmtest.c:573 
drm_open_driver()
> > > (i915_suspend:9050) igt_core-INFO:   #2 ../tests/i915/i915_suspend.c:258 
__igt_unique____real_main245()
> > > (i915_suspend:9050) igt_core-INFO:   #3 ../tests/i915/i915_suspend.c:245 
main()
> > > (i915_suspend:9050) igt_core-INFO:   #4 ../sysdeps/nptl/
libc_start_call_main.h:58 __libc_start_call_main()
> > > (i915_suspend:9050) igt_core-INFO:   #5 ../csu/libc-start.c:128 
__libc_start_main@@GLIBC_2.34()
> > > (i915_suspend:9050) igt_core-INFO:   #6 [_start+0x2a]
> > > ****  END  ****
> > > 
> > > Fixes: f7aff600ab16 ("tests/i915/i915_suspend: Disable d3cold_allowed 
for basic-s2idle-without-i915")
> > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > Cc: Riana Tauro <riana.tauro@intel.com>
> > > ---
> > >  lib/drmtest.c             |  2 +-
> > >  lib/igt_device_scan.c     |  4 ++++
> > >  tests/i915/i915_suspend.c | 10 +++++++++-
> > >  3 files changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index 16e80bdfcf..8e2d1ac50b 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -260,7 +260,7 @@ static bool _is_already_opened(const char *path, int 
as_idx)
> > >  	 * we cannot even stat the device, so it's of no use - let's claim 
it's
> > >  	 * already opened
> > >  	 */
> > > -	if (stat(path, &new) != 0)
> > > +	if (igt_debug_on(stat(path, &new) != 0))
> > >  		return true;
> > >  
> > >  	for (int i = 0; i < as_idx; ++i) {
> > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > > index 8b767eed20..ae69ed09f1 100644
> > > --- a/lib/igt_device_scan.c
> > > +++ b/lib/igt_device_scan.c
> > > @@ -1918,6 +1918,10 @@ static bool __igt_device_card_match(const char 
*filter,
> > >  		return false;
> > >  	memset(card, 0, sizeof(*card));
> > >  
> > > +	/*
> > > +	 * Scan devices in case the user hasn't yet,
> > > +	 * but leave a decision on forced rescan on the user side.
> > > +	 */
> > >  	igt_devices_scan(false);
> > >  
> > >  	if (igt_device_filter_apply(filter) == false)
> > > diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> > > index 815f1c8a2c..c68110476e 100644
> > > --- a/tests/i915/i915_suspend.c
> > > +++ b/tests/i915/i915_suspend.c
> > > @@ -253,8 +253,16 @@ igt_main
> > >  	igt_subtest("basic-s3-without-i915")
> > >  		test_suspend_without_i915(SUSPEND_STATE_S3);
> > >  
> > > -	igt_fixture
> > > +	igt_fixture {
> > > +		/*
> > > +		 * Since above tests may fail leaving the i915 module 
unloaded,
> > > +		 * force refresh of device list before opening an i915 
device
> > > +		 * by cleaning up the current device list, otherwise we 
can fail
> > > +		 * if we have been called with a device filter 
specified.
> > > +		 */
> > > +		igt_devices_free();
> > >  		fd = drm_open_driver(DRIVER_INTEL);
> > > +	}
> > >  
> > >  	igt_subtest("fence-restore-tiled2untiled") {
> > >  		gem_require_mappable_ggtt(fd);
> > > 
> > 
> > 
> > 
> > 
> 




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

* Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests
  2023-02-13  9:51   ` [igt-dev] [Intel-gfx] " Zbigniew Kempczyński
  2023-02-13 10:10     ` Janusz Krzysztofik
@ 2023-02-14  6:15     ` Petri Latvala
  1 sibling, 0 replies; 10+ messages in thread
From: Petri Latvala @ 2023-02-14  6:15 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev, intel-gfx

On Mon, Feb 13, 2023 at 10:51:39AM +0100, Zbigniew Kempczyński wrote:
> On Fri, Feb 10, 2023 at 10:33:21PM +0100, Janusz Krzysztofik wrote:
> > On Thursday, 9 February 2023 20:32:31 CET Janusz Krzysztofik wrote:
> > > If any of *-without-i915 subtests fails or skips for any reason, it may
> > > leave the i915 module unloaded while keeping our device list populated
> > > with initially collected data.  In a follow up igt_fixture section we then
> > > try to reopen the device.  If the test has been executed with a device
> > > filter specified, an attempt to open the device finds a matching entry
> > > that belongs to the no longer existing device in that initially collected
> > > device list, fails to stat() it, concludes that's because of the device
> > > having been already open, and returns an error.
> > > 
> > > Fix this potentially confusing test result by freeing the potentially
> > > outdated device list before continuing with drm_open_driver().
> > 
> > Freeing device list occurred not safe if device scan was not performed before.  
> > I can see 3 potential solutions:
> > 1) force device rescan instead of free before calling drm_open_driver(),
> > 2) teach igt_device_free() to return immediately if the device list has not 
> >    been allocated,
> > 3) provide a has_device_list() helper for to be used if not sure before 
> >    calling igt_device_free().
> > 
> > Any preferences?
> 
> I would enforce rescan.
> 
> BTW I wonder how it can happen if runner is executing each subtest
> in new process so you're starting from scratch and rescan should be
> executed automatically.
> 
> Is is the case you're running few tests from the console?

For the record, igt_runner has --multiple-mode where multiple subtests
are executed in the same exec.


-- 
Petri Latvala

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

end of thread, other threads:[~2023-02-14  6:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-09 19:32 [igt-dev] [PATCH i-g-t] tests/i915_suspend: Free device list after *-without-i915 subtests Janusz Krzysztofik
2023-02-09 21:03 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2023-02-10 14:02 ` [igt-dev] [PATCH i-g-t] " Kamil Konieczny
2023-02-10 14:21   ` Janusz Krzysztofik
2023-02-10 16:28     ` Kamil Konieczny
2023-02-10 15:30 ` [igt-dev] ✗ Fi.CI.IGT: failure for " Patchwork
2023-02-10 21:33 ` [igt-dev] [PATCH i-g-t] " Janusz Krzysztofik
2023-02-13  9:51   ` [igt-dev] [Intel-gfx] " Zbigniew Kempczyński
2023-02-13 10:10     ` Janusz Krzysztofik
2023-02-14  6:15     ` Petri Latvala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox