All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC PATH i-g-t 00/15] tests/core_hotunplug: Fixes and enhancements
@ 2020-07-20 12:18 ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since the test is still blocklisted due to driver issues and won't be
executed on CI, I'm providing a link where results obtained from a 
rybot run with the test removed from the blocklist can be found:
https://patchwork.freedesktop.org/series/79662/
Failures reported there come from driver and not test issues, I
believe.

Thanks,
Janusz

Janusz Krzysztofik (15):
  tests/core_hotunplug: Use igt_assert_fd()
  tests/core_hotunplug: Constify dev_bus_addr string
  tests/core_hotunplug: Consolidate duplicated debug messages
  tests/core_hotunplug: Assert successful device filter application
  tests/core_hotunplug: Fix missing newline
  tests/core_hotunplug: Maintain a single data structure instance
  tests/core_hotunplug: Pass errors via a data structure field
  tests/core_hotunplug: Handle device close errors
  tests/core_hotunplug: Prepare invariant data once per test run
  tests/core_hotunplug: Skip selectively on sysfs close errors
  tests/core_hotunplug: Follow failed subtests with health checks
  tests/core_hotunplug: Fail subtests on device close errors
  tests/core_hotunplug: Process return values of sysfs operations
  tests/core_hotunplug: Assert expected device presence/absence
  tests/core_hotunplug: Explicitly ignore unused return values

 tests/core_hotunplug.c | 333 +++++++++++++++++++++++++----------------
 1 file changed, 202 insertions(+), 131 deletions(-)

-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 00/15] tests/core_hotunplug: Fixes and enhancements
@ 2020-07-20 12:18 ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since the test is still blocklisted due to driver issues and won't be
executed on CI, I'm providing a link where results obtained from a 
rybot run with the test removed from the blocklist can be found:
https://patchwork.freedesktop.org/series/79662/
Failures reported there come from driver and not test issues, I
believe.

Thanks,
Janusz

Janusz Krzysztofik (15):
  tests/core_hotunplug: Use igt_assert_fd()
  tests/core_hotunplug: Constify dev_bus_addr string
  tests/core_hotunplug: Consolidate duplicated debug messages
  tests/core_hotunplug: Assert successful device filter application
  tests/core_hotunplug: Fix missing newline
  tests/core_hotunplug: Maintain a single data structure instance
  tests/core_hotunplug: Pass errors via a data structure field
  tests/core_hotunplug: Handle device close errors
  tests/core_hotunplug: Prepare invariant data once per test run
  tests/core_hotunplug: Skip selectively on sysfs close errors
  tests/core_hotunplug: Follow failed subtests with health checks
  tests/core_hotunplug: Fail subtests on device close errors
  tests/core_hotunplug: Process return values of sysfs operations
  tests/core_hotunplug: Assert expected device presence/absence
  tests/core_hotunplug: Explicitly ignore unused return values

 tests/core_hotunplug.c | 333 +++++++++++++++++++++++++----------------
 1 file changed, 202 insertions(+), 131 deletions(-)

-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 01/15] tests/core_hotunplug: Use igt_assert_fd()
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

There is a new library helper that asserts validity of open file
descriptors.  Use it instead of open coding.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e03f3b945..7431346b1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -57,7 +57,7 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 
 	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
 				    O_DIRECTORY);
-	igt_assert(priv->fd.sysfs_drv >= 0);
+	igt_assert_fd(priv->fd.sysfs_drv);
 
 	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
 	buf[len] = '\0';
@@ -72,10 +72,10 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
 	igt_debug("opening device\n");
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert(priv->fd.drm >= 0);
+	igt_assert_fd(priv->fd.drm);
 
 	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert(priv->fd.sysfs_dev >= 0);
+	igt_assert_fd(priv->fd.sysfs_dev);
 
 	if (buf) {
 		prepare_for_unbind(priv, buf, buflen);
@@ -83,7 +83,7 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 		/* prepare for bus rescan */
 		priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
 					    "device/subsystem", O_DIRECTORY);
-		igt_assert(priv->fd.sysfs_bus >= 0);
+		igt_assert_fd(priv->fd.sysfs_bus);
 	}
 }
 
@@ -261,7 +261,7 @@ igt_main
 		 * a device file descriptor open for exit handler use.
 		 */
 		fd_drm = __drm_open_driver(DRIVER_ANY);
-		igt_assert(fd_drm >= 0);
+		igt_assert_fd(fd_drm);
 
 		if (is_i915_device(fd_drm))
 			igt_require_gem(fd_drm);
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 01/15] tests/core_hotunplug: Use igt_assert_fd()
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

There is a new library helper that asserts validity of open file
descriptors.  Use it instead of open coding.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e03f3b945..7431346b1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -57,7 +57,7 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 
 	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
 				    O_DIRECTORY);
-	igt_assert(priv->fd.sysfs_drv >= 0);
+	igt_assert_fd(priv->fd.sysfs_drv);
 
 	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
 	buf[len] = '\0';
@@ -72,10 +72,10 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
 	igt_debug("opening device\n");
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert(priv->fd.drm >= 0);
+	igt_assert_fd(priv->fd.drm);
 
 	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert(priv->fd.sysfs_dev >= 0);
+	igt_assert_fd(priv->fd.sysfs_dev);
 
 	if (buf) {
 		prepare_for_unbind(priv, buf, buflen);
@@ -83,7 +83,7 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 		/* prepare for bus rescan */
 		priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
 					    "device/subsystem", O_DIRECTORY);
-		igt_assert(priv->fd.sysfs_bus >= 0);
+		igt_assert_fd(priv->fd.sysfs_bus);
 	}
 }
 
@@ -261,7 +261,7 @@ igt_main
 		 * a device file descriptor open for exit handler use.
 		 */
 		fd_drm = __drm_open_driver(DRIVER_ANY);
-		igt_assert(fd_drm >= 0);
+		igt_assert_fd(fd_drm);
 
 		if (is_i915_device(fd_drm))
 			igt_require_gem(fd_drm);
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 02/15] tests/core_hotunplug: Constify dev_bus_addr string
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Device bus address structure field is always initialized with a pointer
to a substring of the device sysfs path and never used for its
modification.  Declare it as a constant string.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7431346b1..a4071f51e 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -44,7 +44,7 @@ struct hotunplug {
 		int sysfs_bus;
 		int sysfs_drv;
 	} fd;
-	char *dev_bus_addr;
+	const char *dev_bus_addr;
 };
 
 /* Helpers */
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 02/15] tests/core_hotunplug: Constify dev_bus_addr string
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Device bus address structure field is always initialized with a pointer
to a substring of the device sysfs path and never used for its
modification.  Declare it as a constant string.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7431346b1..a4071f51e 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -44,7 +44,7 @@ struct hotunplug {
 		int sysfs_bus;
 		int sysfs_drv;
 	} fd;
-	char *dev_bus_addr;
+	const char *dev_bus_addr;
 };
 
 /* Helpers */
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 03/15] tests/core_hotunplug: Consolidate duplicated debug messages
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
                   ` (2 preceding siblings ...)
  (?)
@ 2020-07-20 12:18 ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Some debug messages which designate specific test operations, or their
greater parts at least, sound always the same, no matter which subtest
they are called from.  Emit them, possibly updated with subtest
specified modifiers, from inside respective helpers instead of
duplicating them in subtest bodies.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index a4071f51e..557f9e3fa 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -90,8 +90,11 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 static const char *failure;
 
 /* Unbind the driver from the device */
-static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr)
+static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr,
+			  const char *prefix)
 {
+	igt_debug("%sunbinding the driver from the device\n", prefix);
+
 	failure = "Driver unbind timeout!";
 	igt_set_timeout(60, failure);
 	igt_sysfs_set(fd_sysfs_drv, "unbind", dev_bus_addr);
@@ -104,6 +107,8 @@ static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr)
 /* Re-bind the driver to the device */
 static void driver_bind(int fd_sysfs_drv, const char *dev_bus_addr)
 {
+	igt_debug("rebinding the driver to the device\n");
+
 	failure = "Driver re-bind timeout!";
 	igt_set_timeout(60, failure);
 	igt_sysfs_set(fd_sysfs_drv, "bind", dev_bus_addr);
@@ -114,8 +119,10 @@ static void driver_bind(int fd_sysfs_drv, const char *dev_bus_addr)
 }
 
 /* Remove (virtually unplug) the device from its bus */
-static void device_unplug(int fd_sysfs_dev)
+static void device_unplug(int fd_sysfs_dev, const char *prefix)
 {
+	igt_debug("%sunplugging the device\n", prefix);
+
 	failure = "Device unplug timeout!";
 	igt_set_timeout(60, failure);
 	igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
@@ -128,6 +135,8 @@ static void device_unplug(int fd_sysfs_dev)
 /* Re-discover the device by rescanning its bus */
 static void bus_rescan(int fd_sysfs_bus)
 {
+	igt_debug("recovering the device\n");
+
 	failure = "Bus rescan timeout!";
 	igt_set_timeout(60, failure);
 	igt_sysfs_set(fd_sysfs_bus, "rescan", "1");
@@ -184,10 +193,8 @@ static void unbind_rebind(void)
 	igt_debug("closing the device\n");
 	close(priv.fd.drm);
 
-	igt_debug("unbinding the driver from the device\n");
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "");
 
-	igt_debug("rebinding the driver to the device\n");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
 	healthcheck();
@@ -202,10 +209,8 @@ static void unplug_rescan(void)
 	igt_debug("closing the device\n");
 	close(priv.fd.drm);
 
-	igt_debug("unplugging the device\n");
-	device_unplug(priv.fd.sysfs_dev);
+	device_unplug(priv.fd.sysfs_dev, "");
 
-	igt_debug("recovering the device\n");
 	bus_rescan(priv.fd.sysfs_bus);
 
 	healthcheck();
@@ -218,10 +223,8 @@ static void hotunbind_lateclose(void)
 
 	prepare(&priv, buf, sizeof(buf));
 
-	igt_debug("hot unbinding the driver from the device\n");
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "hot ");
 
-	igt_debug("rebinding the driver to the device\n");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
 	igt_debug("late closing the unbound device instance\n");
@@ -236,10 +239,8 @@ static void hotunplug_lateclose(void)
 
 	prepare(&priv, NULL, 0);
 
-	igt_debug("hot unplugging the device\n");
-	device_unplug(priv.fd.sysfs_dev);
+	device_unplug(priv.fd.sysfs_dev, "hot ");
 
-	igt_debug("recovering the device\n");
 	bus_rescan(priv.fd.sysfs_bus);
 
 	igt_debug("late closing the removed device instance\n");
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 04/15] tests/core_hotunplug: Assert successful device filter application
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Return value of igt_device_filter_add() representing a number of
successfully installed device filters is now ignored.  Fail if not 1.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 557f9e3fa..6ceb325ad 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -178,7 +178,7 @@ static void set_filter_from_device(int fd)
 	igt_assert(realpath(path, dst));
 
 	igt_device_filter_free_all();
-	igt_device_filter_add(filter);
+	igt_assert_eq(igt_device_filter_add(filter), 1);
 }
 
 /* Subtests */
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 04/15] tests/core_hotunplug: Assert successful device filter application
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Return value of igt_device_filter_add() representing a number of
successfully installed device filters is now ignored.  Fail if not 1.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 557f9e3fa..6ceb325ad 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -178,7 +178,7 @@ static void set_filter_from_device(int fd)
 	igt_assert(realpath(path, dst));
 
 	igt_device_filter_free_all();
-	igt_device_filter_add(filter);
+	igt_assert_eq(igt_device_filter_add(filter), 1);
 }
 
 /* Subtests */
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 05/15] tests/core_hotunplug: Fix missing newline
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

A trailing newline is missing from one of fatal error messages, fix it.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 6ceb325ad..cac88c2f3 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -155,7 +155,7 @@ static void healthcheck(void)
 
 	igt_debug("reopening the device\n");
 	fd_drm = __drm_open_driver(DRIVER_ANY);
-	igt_abort_on_f(fd_drm < 0, "Device reopen failure");
+	igt_abort_on_f(fd_drm < 0, "Device reopen failure\n");
 
 	if (is_i915_device(fd_drm)) {
 		failure = "GEM failure";
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 05/15] tests/core_hotunplug: Fix missing newline
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

A trailing newline is missing from one of fatal error messages, fix it.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 6ceb325ad..cac88c2f3 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -155,7 +155,7 @@ static void healthcheck(void)
 
 	igt_debug("reopening the device\n");
 	fd_drm = __drm_open_driver(DRIVER_ANY);
-	igt_abort_on_f(fd_drm < 0, "Device reopen failure");
+	igt_abort_on_f(fd_drm < 0, "Device reopen failure\n");
 
 	if (is_i915_device(fd_drm)) {
 		failure = "GEM failure";
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 06/15] tests/core_hotunplug: Maintain a single data structure instance
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

The following changes to the test are planned:
- avoid global variables,
- skip subtest after device close errors,
- prepare invariant data only once per test run,
- move device health checks to igt_fixture sections,
- try to recover from subtest failures instead of aborting.
For that to be possible, maintain a single instance of hotunplug
structure at igt_main level and pass it down to subtests.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 56 ++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index cac88c2f3..0e61da2d0 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -183,68 +183,62 @@ static void set_filter_from_device(int fd)
 
 /* Subtests */
 
-static void unbind_rebind(void)
+static void unbind_rebind(struct hotunplug *priv)
 {
-	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare(priv, buf, sizeof(buf));
 
 	igt_debug("closing the device\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "");
+	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "");
 
-	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
 	healthcheck();
 }
 
-static void unplug_rescan(void)
+static void unplug_rescan(struct hotunplug *priv)
 {
-	struct hotunplug priv;
-
-	prepare(&priv, NULL, 0);
+	prepare(priv, NULL, 0);
 
 	igt_debug("closing the device\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
-	device_unplug(priv.fd.sysfs_dev, "");
+	device_unplug(priv->fd.sysfs_dev, "");
 
-	bus_rescan(priv.fd.sysfs_bus);
+	bus_rescan(priv->fd.sysfs_bus);
 
 	healthcheck();
 }
 
-static void hotunbind_lateclose(void)
+static void hotunbind_lateclose(struct hotunplug *priv)
 {
-	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare(priv, buf, sizeof(buf));
 
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "hot ");
+	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "hot ");
 
-	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
 	igt_debug("late closing the unbound device instance\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
 	healthcheck();
 }
 
-static void hotunplug_lateclose(void)
+static void hotunplug_lateclose(struct hotunplug *priv)
 {
-	struct hotunplug priv;
-
-	prepare(&priv, NULL, 0);
+	prepare(priv, NULL, 0);
 
-	device_unplug(priv.fd.sysfs_dev, "hot ");
+	device_unplug(priv->fd.sysfs_dev, "hot ");
 
-	bus_rescan(priv.fd.sysfs_bus);
+	bus_rescan(priv->fd.sysfs_bus);
 
 	igt_debug("late closing the removed device instance\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
 	healthcheck();
 }
@@ -253,6 +247,8 @@ static void hotunplug_lateclose(void)
 
 igt_main
 {
+	struct hotunplug priv;
+
 	igt_fixture {
 		int fd_drm;
 
@@ -275,28 +271,28 @@ igt_main
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
 	igt_subtest("unbind-rebind")
-		unbind_rebind();
+		unbind_rebind(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
 	igt_subtest("unplug-rescan")
-		unplug_rescan();
+		unplug_rescan(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
 	igt_subtest("hotunbind-lateclose")
-		hotunbind_lateclose();
+		hotunbind_lateclose(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if a still open device can be cleanly unplugged, then released");
 	igt_subtest("hotunplug-lateclose")
-		hotunplug_lateclose();
+		hotunplug_lateclose(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 06/15] tests/core_hotunplug: Maintain a single data structure instance
@ 2020-07-20 12:18   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:18 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

The following changes to the test are planned:
- avoid global variables,
- skip subtest after device close errors,
- prepare invariant data only once per test run,
- move device health checks to igt_fixture sections,
- try to recover from subtest failures instead of aborting.
For that to be possible, maintain a single instance of hotunplug
structure at igt_main level and pass it down to subtests.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 56 ++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index cac88c2f3..0e61da2d0 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -183,68 +183,62 @@ static void set_filter_from_device(int fd)
 
 /* Subtests */
 
-static void unbind_rebind(void)
+static void unbind_rebind(struct hotunplug *priv)
 {
-	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare(priv, buf, sizeof(buf));
 
 	igt_debug("closing the device\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "");
+	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "");
 
-	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
 	healthcheck();
 }
 
-static void unplug_rescan(void)
+static void unplug_rescan(struct hotunplug *priv)
 {
-	struct hotunplug priv;
-
-	prepare(&priv, NULL, 0);
+	prepare(priv, NULL, 0);
 
 	igt_debug("closing the device\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
-	device_unplug(priv.fd.sysfs_dev, "");
+	device_unplug(priv->fd.sysfs_dev, "");
 
-	bus_rescan(priv.fd.sysfs_bus);
+	bus_rescan(priv->fd.sysfs_bus);
 
 	healthcheck();
 }
 
-static void hotunbind_lateclose(void)
+static void hotunbind_lateclose(struct hotunplug *priv)
 {
-	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare(priv, buf, sizeof(buf));
 
-	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr, "hot ");
+	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "hot ");
 
-	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
 
 	igt_debug("late closing the unbound device instance\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
 	healthcheck();
 }
 
-static void hotunplug_lateclose(void)
+static void hotunplug_lateclose(struct hotunplug *priv)
 {
-	struct hotunplug priv;
-
-	prepare(&priv, NULL, 0);
+	prepare(priv, NULL, 0);
 
-	device_unplug(priv.fd.sysfs_dev, "hot ");
+	device_unplug(priv->fd.sysfs_dev, "hot ");
 
-	bus_rescan(priv.fd.sysfs_bus);
+	bus_rescan(priv->fd.sysfs_bus);
 
 	igt_debug("late closing the removed device instance\n");
-	close(priv.fd.drm);
+	close(priv->fd.drm);
 
 	healthcheck();
 }
@@ -253,6 +247,8 @@ static void hotunplug_lateclose(void)
 
 igt_main
 {
+	struct hotunplug priv;
+
 	igt_fixture {
 		int fd_drm;
 
@@ -275,28 +271,28 @@ igt_main
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
 	igt_subtest("unbind-rebind")
-		unbind_rebind();
+		unbind_rebind(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
 	igt_subtest("unplug-rescan")
-		unplug_rescan();
+		unplug_rescan(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
 	igt_subtest("hotunbind-lateclose")
-		hotunbind_lateclose();
+		hotunbind_lateclose(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
 
 	igt_describe("Check if a still open device can be cleanly unplugged, then released");
 	igt_subtest("hotunplug-lateclose")
-		hotunplug_lateclose();
+		hotunplug_lateclose(&priv);
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 07/15] tests/core_hotunplug: Pass errors via a data structure field
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
                   ` (6 preceding siblings ...)
  (?)
@ 2020-07-20 12:19 ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

A pointer to fatal error messages can be passed around via hotunplug
structure, no need to declare it as global.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 92 +++++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 47 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 0e61da2d0..6070b7d95 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -45,6 +45,7 @@ struct hotunplug {
 		int sysfs_drv;
 	} fd;
 	const char *dev_bus_addr;
+	const char *failure;
 };
 
 /* Helpers */
@@ -87,66 +88,63 @@ static void prepare(struct hotunplug *priv, char *buf, int buflen)
 	}
 }
 
-static const char *failure;
-
 /* Unbind the driver from the device */
-static void driver_unbind(int fd_sysfs_drv, const char *dev_bus_addr,
-			  const char *prefix)
+static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
 	igt_debug("%sunbinding the driver from the device\n", prefix);
 
-	failure = "Driver unbind timeout!";
-	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_drv, "unbind", dev_bus_addr);
+	priv->failure = "Driver unbind timeout!";
+	igt_set_timeout(60, priv->failure);
+	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	failure = NULL;
+	priv->failure = NULL;
 
-	/* don't close fd_sysfs_drv, it will be used for driver rebinding */
+	/* don't close fd.sysfs_drv, it will be used for driver rebinding */
 }
 
 /* Re-bind the driver to the device */
-static void driver_bind(int fd_sysfs_drv, const char *dev_bus_addr)
+static void driver_bind(struct hotunplug *priv)
 {
 	igt_debug("rebinding the driver to the device\n");
 
-	failure = "Driver re-bind timeout!";
-	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_drv, "bind", dev_bus_addr);
+	priv->failure = "Driver re-bind timeout!";
+	igt_set_timeout(60, priv->failure);
+	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	failure = NULL;
+	priv->failure = NULL;
 
-	close(fd_sysfs_drv);
+	close(priv->fd.sysfs_drv);
 }
 
 /* Remove (virtually unplug) the device from its bus */
-static void device_unplug(int fd_sysfs_dev, const char *prefix)
+static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
 	igt_debug("%sunplugging the device\n", prefix);
 
-	failure = "Device unplug timeout!";
-	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
+	priv->failure = "Device unplug timeout!";
+	igt_set_timeout(60, priv->failure);
+	igt_sysfs_set(priv->fd.sysfs_dev, "device/remove", "1");
 	igt_reset_timeout();
-	failure = NULL;
+	priv->failure = NULL;
 
-	close(fd_sysfs_dev);
+	close(priv->fd.sysfs_dev);
 }
 
 /* Re-discover the device by rescanning its bus */
-static void bus_rescan(int fd_sysfs_bus)
+static void bus_rescan(struct hotunplug *priv)
 {
 	igt_debug("recovering the device\n");
 
-	failure = "Bus rescan timeout!";
-	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_bus, "rescan", "1");
+	priv->failure = "Bus rescan timeout!";
+	igt_set_timeout(60, priv->failure);
+	igt_sysfs_set(priv->fd.sysfs_bus, "rescan", "1");
 	igt_reset_timeout();
-	failure = NULL;
+	priv->failure = NULL;
 
-	close(fd_sysfs_bus);
+	close(priv->fd.sysfs_bus);
 }
 
-static void healthcheck(void)
+static void healthcheck(struct hotunplug *priv)
 {
 	int fd_drm;
 
@@ -158,9 +156,9 @@ static void healthcheck(void)
 	igt_abort_on_f(fd_drm < 0, "Device reopen failure\n");
 
 	if (is_i915_device(fd_drm)) {
-		failure = "GEM failure";
+		priv->failure = "GEM failure";
 		igt_require_gem(fd_drm);
-		failure = NULL;
+		priv->failure = NULL;
 	}
 
 	close(fd_drm);
@@ -192,11 +190,11 @@ static void unbind_rebind(struct hotunplug *priv)
 	igt_debug("closing the device\n");
 	close(priv->fd.drm);
 
-	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "");
+	driver_unbind(priv, "");
 
-	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
+	driver_bind(priv);
 
-	healthcheck();
+	healthcheck(priv);
 }
 
 static void unplug_rescan(struct hotunplug *priv)
@@ -206,11 +204,11 @@ static void unplug_rescan(struct hotunplug *priv)
 	igt_debug("closing the device\n");
 	close(priv->fd.drm);
 
-	device_unplug(priv->fd.sysfs_dev, "");
+	device_unplug(priv, "");
 
-	bus_rescan(priv->fd.sysfs_bus);
+	bus_rescan(priv);
 
-	healthcheck();
+	healthcheck(priv);
 }
 
 static void hotunbind_lateclose(struct hotunplug *priv)
@@ -219,35 +217,35 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
 	prepare(priv, buf, sizeof(buf));
 
-	driver_unbind(priv->fd.sysfs_drv, priv->dev_bus_addr, "hot ");
+	driver_unbind(priv, "hot ");
 
-	driver_bind(priv->fd.sysfs_drv, priv->dev_bus_addr);
+	driver_bind(priv);
 
 	igt_debug("late closing the unbound device instance\n");
 	close(priv->fd.drm);
 
-	healthcheck();
+	healthcheck(priv);
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
 {
 	prepare(priv, NULL, 0);
 
-	device_unplug(priv->fd.sysfs_dev, "hot ");
+	device_unplug(priv, "hot ");
 
-	bus_rescan(priv->fd.sysfs_bus);
+	bus_rescan(priv);
 
 	igt_debug("late closing the removed device instance\n");
 	close(priv->fd.drm);
 
-	healthcheck();
+	healthcheck(priv);
 }
 
 /* Main */
 
 igt_main
 {
-	struct hotunplug priv;
+	struct hotunplug priv = { .failure = NULL, };
 
 	igt_fixture {
 		int fd_drm;
@@ -274,26 +272,26 @@ igt_main
 		unbind_rebind(&priv);
 
 	igt_fixture
-		igt_abort_on_f(failure, "%s\n", failure);
+		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
 
 	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
 	igt_subtest("unplug-rescan")
 		unplug_rescan(&priv);
 
 	igt_fixture
-		igt_abort_on_f(failure, "%s\n", failure);
+		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
 
 	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
 	igt_subtest("hotunbind-lateclose")
 		hotunbind_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(failure, "%s\n", failure);
+		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
 
 	igt_describe("Check if a still open device can be cleanly unplugged, then released");
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(failure, "%s\n", failure);
+		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
 }
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 08/15] tests/core_hotunplug: Handle device close errors
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

The test now ignores device close errors.  Those errors are believed to
have no influence on device health so there is no need to process them
the same way as we mostly do on errors, i.e., notify CI about a problem
via igt_abort.  However, those errors may indicate issues with the test
itself.  Moreover, impact of those errors on operations performed by
subtests, like driver unbind or device remove, should be perceived as
undefined.  Then, we should fail as soon as a device or device sysfs
node close error occurs and also skip subsequent subtests.  However,
once a driver unbind or device unplug operation has been attempted by a
subtest, we can't just fail without checking the device health.

When in a subtest, store results of device close operations for future
reference.  Reuse file descriptor fields of the hotunplug structure for
that.  Unless in between of a driver remove or device unplug operation
and a successful device health check, fail current test section right
after a device close error occurs, warn otherwise.  If still running,
examine device file descriptor fields in subsequent igt_fixture
sections and skip on errors.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 61 ++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 6070b7d95..cdb07a97c 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -43,13 +43,22 @@ struct hotunplug {
 		int sysfs_dev;
 		int sysfs_bus;
 		int sysfs_drv;
-	} fd;
+	} fd;	/* >= 0: valid fd, == -1: closed, < -1: close failed */
 	const char *dev_bus_addr;
 	const char *failure;
 };
 
 /* Helpers */
 
+static int local_close(int fd)
+{
+	errno = 0;
+	if (close(fd))	/* close failure - return -errno (never -1) */
+		return -errno;
+
+	return -1;	/* success - return 'closed' */
+}
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
 	int len;
@@ -66,7 +75,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 	igt_assert(priv->dev_bus_addr++);
 
 	/* sysfs_dev no longer needed */
-	close(priv->fd.sysfs_dev);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
+	igt_assert_f(priv->fd.sysfs_dev == -1,
+		     "Device sysfs node close failed\n");
 }
 
 static void prepare(struct hotunplug *priv, char *buf, int buflen)
@@ -127,7 +138,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_reset_timeout();
 	priv->failure = NULL;
 
-	close(priv->fd.sysfs_dev);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
+	igt_warn_on_f(priv->fd.sysfs_dev != -1,
+		      "Device sysfs node close failed\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -146,6 +159,7 @@ static void bus_rescan(struct hotunplug *priv)
 
 static void healthcheck(struct hotunplug *priv)
 {
+	/* preserve error code potentially stored before in priv->fd.drm */
 	int fd_drm;
 
 	/* device name may have changed, rebuild IGT device list */
@@ -161,7 +175,19 @@ static void healthcheck(struct hotunplug *priv)
 		priv->failure = NULL;
 	}
 
-	close(fd_drm);
+	fd_drm = local_close(fd_drm);
+	if (priv->fd.drm == -1)
+		priv->fd.drm = fd_drm;
+	igt_assert_f(fd_drm == -1, "Device close failed\n");
+}
+
+static void post_healthckeck(struct hotunplug *priv)
+{
+	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
+
+	igt_require_f(priv->fd.drm == -1, "Device not closed properly\n");
+	igt_require_f(priv->fd.sysfs_dev == -1,
+		      "Device sysfs node not closed properly\n");
 }
 
 static void set_filter_from_device(int fd)
@@ -188,7 +214,8 @@ static void unbind_rebind(struct hotunplug *priv)
 	prepare(priv, buf, sizeof(buf));
 
 	igt_debug("closing the device\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 
 	driver_unbind(priv, "");
 
@@ -202,7 +229,8 @@ static void unplug_rescan(struct hotunplug *priv)
 	prepare(priv, NULL, 0);
 
 	igt_debug("closing the device\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 
 	device_unplug(priv, "");
 
@@ -222,7 +250,8 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	driver_bind(priv);
 
 	igt_debug("late closing the unbound device instance\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
 
 	healthcheck(priv);
 }
@@ -236,7 +265,8 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	bus_rescan(priv);
 
 	igt_debug("late closing the removed device instance\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
 
 	healthcheck(priv);
 }
@@ -245,7 +275,10 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
 igt_main
 {
-	struct hotunplug priv = { .failure = NULL, };
+	struct hotunplug priv = {
+		.fd		= { .drm = -1, .sysfs_dev = -1, },
+		.failure	= NULL,
+	};
 
 	igt_fixture {
 		int fd_drm;
@@ -264,7 +297,7 @@ igt_main
 		/* Make sure subtests always reopen the same device */
 		set_filter_from_device(fd_drm);
 
-		close(fd_drm);
+		igt_fail_on_f(close(fd_drm), "Device close failed\n");
 	}
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
@@ -272,26 +305,26 @@ igt_main
 		unbind_rebind(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
 	igt_subtest("unplug-rescan")
 		unplug_rescan(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
 	igt_subtest("hotunbind-lateclose")
 		hotunbind_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if a still open device can be cleanly unplugged, then released");
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 }
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 08/15] tests/core_hotunplug: Handle device close errors
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

The test now ignores device close errors.  Those errors are believed to
have no influence on device health so there is no need to process them
the same way as we mostly do on errors, i.e., notify CI about a problem
via igt_abort.  However, those errors may indicate issues with the test
itself.  Moreover, impact of those errors on operations performed by
subtests, like driver unbind or device remove, should be perceived as
undefined.  Then, we should fail as soon as a device or device sysfs
node close error occurs and also skip subsequent subtests.  However,
once a driver unbind or device unplug operation has been attempted by a
subtest, we can't just fail without checking the device health.

When in a subtest, store results of device close operations for future
reference.  Reuse file descriptor fields of the hotunplug structure for
that.  Unless in between of a driver remove or device unplug operation
and a successful device health check, fail current test section right
after a device close error occurs, warn otherwise.  If still running,
examine device file descriptor fields in subsequent igt_fixture
sections and skip on errors.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 61 ++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 6070b7d95..cdb07a97c 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -43,13 +43,22 @@ struct hotunplug {
 		int sysfs_dev;
 		int sysfs_bus;
 		int sysfs_drv;
-	} fd;
+	} fd;	/* >= 0: valid fd, == -1: closed, < -1: close failed */
 	const char *dev_bus_addr;
 	const char *failure;
 };
 
 /* Helpers */
 
+static int local_close(int fd)
+{
+	errno = 0;
+	if (close(fd))	/* close failure - return -errno (never -1) */
+		return -errno;
+
+	return -1;	/* success - return 'closed' */
+}
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
 	int len;
@@ -66,7 +75,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 	igt_assert(priv->dev_bus_addr++);
 
 	/* sysfs_dev no longer needed */
-	close(priv->fd.sysfs_dev);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
+	igt_assert_f(priv->fd.sysfs_dev == -1,
+		     "Device sysfs node close failed\n");
 }
 
 static void prepare(struct hotunplug *priv, char *buf, int buflen)
@@ -127,7 +138,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_reset_timeout();
 	priv->failure = NULL;
 
-	close(priv->fd.sysfs_dev);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
+	igt_warn_on_f(priv->fd.sysfs_dev != -1,
+		      "Device sysfs node close failed\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -146,6 +159,7 @@ static void bus_rescan(struct hotunplug *priv)
 
 static void healthcheck(struct hotunplug *priv)
 {
+	/* preserve error code potentially stored before in priv->fd.drm */
 	int fd_drm;
 
 	/* device name may have changed, rebuild IGT device list */
@@ -161,7 +175,19 @@ static void healthcheck(struct hotunplug *priv)
 		priv->failure = NULL;
 	}
 
-	close(fd_drm);
+	fd_drm = local_close(fd_drm);
+	if (priv->fd.drm == -1)
+		priv->fd.drm = fd_drm;
+	igt_assert_f(fd_drm == -1, "Device close failed\n");
+}
+
+static void post_healthckeck(struct hotunplug *priv)
+{
+	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
+
+	igt_require_f(priv->fd.drm == -1, "Device not closed properly\n");
+	igt_require_f(priv->fd.sysfs_dev == -1,
+		      "Device sysfs node not closed properly\n");
 }
 
 static void set_filter_from_device(int fd)
@@ -188,7 +214,8 @@ static void unbind_rebind(struct hotunplug *priv)
 	prepare(priv, buf, sizeof(buf));
 
 	igt_debug("closing the device\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 
 	driver_unbind(priv, "");
 
@@ -202,7 +229,8 @@ static void unplug_rescan(struct hotunplug *priv)
 	prepare(priv, NULL, 0);
 
 	igt_debug("closing the device\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 
 	device_unplug(priv, "");
 
@@ -222,7 +250,8 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	driver_bind(priv);
 
 	igt_debug("late closing the unbound device instance\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
 
 	healthcheck(priv);
 }
@@ -236,7 +265,8 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	bus_rescan(priv);
 
 	igt_debug("late closing the removed device instance\n");
-	close(priv->fd.drm);
+	priv->fd.drm = local_close(priv->fd.drm);
+	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
 
 	healthcheck(priv);
 }
@@ -245,7 +275,10 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
 igt_main
 {
-	struct hotunplug priv = { .failure = NULL, };
+	struct hotunplug priv = {
+		.fd		= { .drm = -1, .sysfs_dev = -1, },
+		.failure	= NULL,
+	};
 
 	igt_fixture {
 		int fd_drm;
@@ -264,7 +297,7 @@ igt_main
 		/* Make sure subtests always reopen the same device */
 		set_filter_from_device(fd_drm);
 
-		close(fd_drm);
+		igt_fail_on_f(close(fd_drm), "Device close failed\n");
 	}
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
@@ -272,26 +305,26 @@ igt_main
 		unbind_rebind(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
 	igt_subtest("unplug-rescan")
 		unplug_rescan(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
 	igt_subtest("hotunbind-lateclose")
 		hotunbind_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 
 	igt_describe("Check if a still open device can be cleanly unplugged, then released");
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose(&priv);
 
 	igt_fixture
-		igt_abort_on_f(priv.failure, "%s\n", priv.failure);
+		post_healthckeck(&priv);
 }
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 09/15] tests/core_hotunplug: Prepare invariant data once per test run
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Each subtest now calls a prepare() helper which opens a couple of files
required by that subtest.  Those files are then closed after use,
either directly from the subtest body, or indirectly from inside one of
helper functions called during the subtest execution.  That approach
not only makes lifecycle of individual file descriptors difficult to
follow but also prevents us from re-running health checks on subtest
failures from follow up igt_fixture sections since we may need to retry
bus rescan or driver rebind operations.

Two of those files - device bus and driver sysfs nodes - are not
affected nor interfere with driver unbind / device unplug operations
performed by subtests.  Then, there is not much sense in closing and
reopening those nodes.  Open them once at the beginning of a test run,
then close them as late as on test completion.

The prepare() helper also populates a device bus address string used by
driver unbind / rebind operations.  Since the bus address of an
exercised device never changes, also prepare that string only once at
the beginning of a test run.  Note that it is the same as the last
component of a device filter string which is already resolved and
installed from an initial igt_fixture section of the test.  Then,
initialize the device bus address field of a hotunplug structure
instance with a pointer to the respective substring of that filter
rather than resolving it again from the device sysfs node pathname.

There is one more sysfs node - a DRM device node - now opened by the
prepare() helper for subtests which perform device remove operations.
That node can't be opened only once at the beginning of a test run
because its open file descriptor is no longer usable as soon as a
driver unbind operation is performed.  On the other hand, it can't be
opened easily from inside a device_remove() helper since some subtests
just don't open the device so its file descriptor used by
igt_sysfs_open() may just not be available.  However, note that only a
PCI sysfs node of the device, not necessarily the DRM one, is actually
required for a successful device remove operation, and that node can be
opened easily from a bus file descriptor using a device bus address
string, both already available.  Then, change the semantics of a
.fd.sysfs_dev field of the hotunplug structure from DRM to PCI device
sysfs file descriptor, then let the device_remove() helper open the
device PCI node by itself and store its file descriptor in that field.
Also, for still more easy access to the device PCI node, use a
'subsystem/devices' subnode of the PCI device as its bus sysfs location
instead of just 'subsystem', then adjust a relative path to the bus
'rescan' function accordingly.

A side benefit of using the PCI device sysfs node, not the DRM one,
while removing the device is that a future subtest may now easily
perform both driver unbind and device remove operations in a row.

Suggested-by: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 91 ++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 56 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index cdb07a97c..daf0bf745 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -59,46 +59,33 @@ static int local_close(int fd)
 	return -1;	/* success - return 'closed' */
 }
 
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static void prepare(struct hotunplug *priv)
 {
-	int len;
+	const char *filter = igt_device_filter_get(0), *sysfs_path;
 
-	igt_assert(buflen);
+	igt_assert(filter);
 
-	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
-				    O_DIRECTORY);
+	priv->dev_bus_addr = strrchr(filter, '/');
+	igt_assert(priv->dev_bus_addr++);
+
+	sysfs_path = strchr(filter, ':');
+	igt_assert(sysfs_path++);
+
+	priv->fd.sysfs_dev = open(sysfs_path, O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_dev);
+
+	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "driver", O_DIRECTORY);
 	igt_assert_fd(priv->fd.sysfs_drv);
 
-	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
-	buf[len] = '\0';
-	priv->dev_bus_addr = strrchr(buf, '/');
-	igt_assert(priv->dev_bus_addr++);
+	priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev, "subsystem/devices",
+				    O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_bus);
 
-	/* sysfs_dev no longer needed */
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_assert_f(priv->fd.sysfs_dev == -1,
 		     "Device sysfs node close failed\n");
 }
 
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
-{
-	igt_debug("opening device\n");
-	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert_fd(priv->fd.drm);
-
-	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert_fd(priv->fd.sysfs_dev);
-
-	if (buf) {
-		prepare_for_unbind(priv, buf, buflen);
-	} else {
-		/* prepare for bus rescan */
-		priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
-					    "device/subsystem", O_DIRECTORY);
-		igt_assert_fd(priv->fd.sysfs_bus);
-	}
-}
-
 /* Unbind the driver from the device */
 static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
@@ -109,8 +96,6 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	/* don't close fd.sysfs_drv, it will be used for driver rebinding */
 }
 
 /* Re-bind the driver to the device */
@@ -123,18 +108,20 @@ static void driver_bind(struct hotunplug *priv)
 	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	close(priv->fd.sysfs_drv);
 }
 
 /* Remove (virtually unplug) the device from its bus */
 static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
+	priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+				    O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_dev);
+
 	igt_debug("%sunplugging the device\n", prefix);
 
 	priv->failure = "Device unplug timeout!";
 	igt_set_timeout(60, priv->failure);
-	igt_sysfs_set(priv->fd.sysfs_dev, "device/remove", "1");
+	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
 	igt_reset_timeout();
 	priv->failure = NULL;
 
@@ -150,11 +137,9 @@ static void bus_rescan(struct hotunplug *priv)
 
 	priv->failure = "Bus rescan timeout!";
 	igt_set_timeout(60, priv->failure);
-	igt_sysfs_set(priv->fd.sysfs_bus, "rescan", "1");
+	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	close(priv->fd.sysfs_bus);
 }
 
 static void healthcheck(struct hotunplug *priv)
@@ -209,14 +194,6 @@ static void set_filter_from_device(int fd)
 
 static void unbind_rebind(struct hotunplug *priv)
 {
-	char buf[PATH_MAX];
-
-	prepare(priv, buf, sizeof(buf));
-
-	igt_debug("closing the device\n");
-	priv->fd.drm = local_close(priv->fd.drm);
-	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
-
 	driver_unbind(priv, "");
 
 	driver_bind(priv);
@@ -226,12 +203,6 @@ static void unbind_rebind(struct hotunplug *priv)
 
 static void unplug_rescan(struct hotunplug *priv)
 {
-	prepare(priv, NULL, 0);
-
-	igt_debug("closing the device\n");
-	priv->fd.drm = local_close(priv->fd.drm);
-	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
-
 	device_unplug(priv, "");
 
 	bus_rescan(priv);
@@ -241,9 +212,9 @@ static void unplug_rescan(struct hotunplug *priv)
 
 static void hotunbind_lateclose(struct hotunplug *priv)
 {
-	char buf[PATH_MAX];
-
-	prepare(priv, buf, sizeof(buf));
+	igt_debug("opening device\n");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(priv->fd.drm);
 
 	driver_unbind(priv, "hot ");
 
@@ -258,7 +229,9 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
 static void hotunplug_lateclose(struct hotunplug *priv)
 {
-	prepare(priv, NULL, 0);
+	igt_debug("opening device\n");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(priv->fd.drm);
 
 	device_unplug(priv, "hot ");
 
@@ -298,6 +271,8 @@ igt_main
 		set_filter_from_device(fd_drm);
 
 		igt_fail_on_f(close(fd_drm), "Device close failed\n");
+
+		prepare(&priv);
 	}
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
@@ -325,6 +300,10 @@ igt_main
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose(&priv);
 
-	igt_fixture
+	igt_fixture {
 		post_healthckeck(&priv);
+
+		close(priv.fd.sysfs_bus);
+		close(priv.fd.sysfs_drv);
+	}
 }
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 09/15] tests/core_hotunplug: Prepare invariant data once per test run
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Each subtest now calls a prepare() helper which opens a couple of files
required by that subtest.  Those files are then closed after use,
either directly from the subtest body, or indirectly from inside one of
helper functions called during the subtest execution.  That approach
not only makes lifecycle of individual file descriptors difficult to
follow but also prevents us from re-running health checks on subtest
failures from follow up igt_fixture sections since we may need to retry
bus rescan or driver rebind operations.

Two of those files - device bus and driver sysfs nodes - are not
affected nor interfere with driver unbind / device unplug operations
performed by subtests.  Then, there is not much sense in closing and
reopening those nodes.  Open them once at the beginning of a test run,
then close them as late as on test completion.

The prepare() helper also populates a device bus address string used by
driver unbind / rebind operations.  Since the bus address of an
exercised device never changes, also prepare that string only once at
the beginning of a test run.  Note that it is the same as the last
component of a device filter string which is already resolved and
installed from an initial igt_fixture section of the test.  Then,
initialize the device bus address field of a hotunplug structure
instance with a pointer to the respective substring of that filter
rather than resolving it again from the device sysfs node pathname.

There is one more sysfs node - a DRM device node - now opened by the
prepare() helper for subtests which perform device remove operations.
That node can't be opened only once at the beginning of a test run
because its open file descriptor is no longer usable as soon as a
driver unbind operation is performed.  On the other hand, it can't be
opened easily from inside a device_remove() helper since some subtests
just don't open the device so its file descriptor used by
igt_sysfs_open() may just not be available.  However, note that only a
PCI sysfs node of the device, not necessarily the DRM one, is actually
required for a successful device remove operation, and that node can be
opened easily from a bus file descriptor using a device bus address
string, both already available.  Then, change the semantics of a
.fd.sysfs_dev field of the hotunplug structure from DRM to PCI device
sysfs file descriptor, then let the device_remove() helper open the
device PCI node by itself and store its file descriptor in that field.
Also, for still more easy access to the device PCI node, use a
'subsystem/devices' subnode of the PCI device as its bus sysfs location
instead of just 'subsystem', then adjust a relative path to the bus
'rescan' function accordingly.

A side benefit of using the PCI device sysfs node, not the DRM one,
while removing the device is that a future subtest may now easily
perform both driver unbind and device remove operations in a row.

Suggested-by: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 91 ++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 56 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index cdb07a97c..daf0bf745 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -59,46 +59,33 @@ static int local_close(int fd)
 	return -1;	/* success - return 'closed' */
 }
 
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static void prepare(struct hotunplug *priv)
 {
-	int len;
+	const char *filter = igt_device_filter_get(0), *sysfs_path;
 
-	igt_assert(buflen);
+	igt_assert(filter);
 
-	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
-				    O_DIRECTORY);
+	priv->dev_bus_addr = strrchr(filter, '/');
+	igt_assert(priv->dev_bus_addr++);
+
+	sysfs_path = strchr(filter, ':');
+	igt_assert(sysfs_path++);
+
+	priv->fd.sysfs_dev = open(sysfs_path, O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_dev);
+
+	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "driver", O_DIRECTORY);
 	igt_assert_fd(priv->fd.sysfs_drv);
 
-	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
-	buf[len] = '\0';
-	priv->dev_bus_addr = strrchr(buf, '/');
-	igt_assert(priv->dev_bus_addr++);
+	priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev, "subsystem/devices",
+				    O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_bus);
 
-	/* sysfs_dev no longer needed */
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_assert_f(priv->fd.sysfs_dev == -1,
 		     "Device sysfs node close failed\n");
 }
 
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
-{
-	igt_debug("opening device\n");
-	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert_fd(priv->fd.drm);
-
-	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert_fd(priv->fd.sysfs_dev);
-
-	if (buf) {
-		prepare_for_unbind(priv, buf, buflen);
-	} else {
-		/* prepare for bus rescan */
-		priv->fd.sysfs_bus = openat(priv->fd.sysfs_dev,
-					    "device/subsystem", O_DIRECTORY);
-		igt_assert_fd(priv->fd.sysfs_bus);
-	}
-}
-
 /* Unbind the driver from the device */
 static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
@@ -109,8 +96,6 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	/* don't close fd.sysfs_drv, it will be used for driver rebinding */
 }
 
 /* Re-bind the driver to the device */
@@ -123,18 +108,20 @@ static void driver_bind(struct hotunplug *priv)
 	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	close(priv->fd.sysfs_drv);
 }
 
 /* Remove (virtually unplug) the device from its bus */
 static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
+	priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+				    O_DIRECTORY);
+	igt_assert_fd(priv->fd.sysfs_dev);
+
 	igt_debug("%sunplugging the device\n", prefix);
 
 	priv->failure = "Device unplug timeout!";
 	igt_set_timeout(60, priv->failure);
-	igt_sysfs_set(priv->fd.sysfs_dev, "device/remove", "1");
+	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
 	igt_reset_timeout();
 	priv->failure = NULL;
 
@@ -150,11 +137,9 @@ static void bus_rescan(struct hotunplug *priv)
 
 	priv->failure = "Bus rescan timeout!";
 	igt_set_timeout(60, priv->failure);
-	igt_sysfs_set(priv->fd.sysfs_bus, "rescan", "1");
+	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
 	igt_reset_timeout();
 	priv->failure = NULL;
-
-	close(priv->fd.sysfs_bus);
 }
 
 static void healthcheck(struct hotunplug *priv)
@@ -209,14 +194,6 @@ static void set_filter_from_device(int fd)
 
 static void unbind_rebind(struct hotunplug *priv)
 {
-	char buf[PATH_MAX];
-
-	prepare(priv, buf, sizeof(buf));
-
-	igt_debug("closing the device\n");
-	priv->fd.drm = local_close(priv->fd.drm);
-	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
-
 	driver_unbind(priv, "");
 
 	driver_bind(priv);
@@ -226,12 +203,6 @@ static void unbind_rebind(struct hotunplug *priv)
 
 static void unplug_rescan(struct hotunplug *priv)
 {
-	prepare(priv, NULL, 0);
-
-	igt_debug("closing the device\n");
-	priv->fd.drm = local_close(priv->fd.drm);
-	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
-
 	device_unplug(priv, "");
 
 	bus_rescan(priv);
@@ -241,9 +212,9 @@ static void unplug_rescan(struct hotunplug *priv)
 
 static void hotunbind_lateclose(struct hotunplug *priv)
 {
-	char buf[PATH_MAX];
-
-	prepare(priv, buf, sizeof(buf));
+	igt_debug("opening device\n");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(priv->fd.drm);
 
 	driver_unbind(priv, "hot ");
 
@@ -258,7 +229,9 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
 static void hotunplug_lateclose(struct hotunplug *priv)
 {
-	prepare(priv, NULL, 0);
+	igt_debug("opening device\n");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert_fd(priv->fd.drm);
 
 	device_unplug(priv, "hot ");
 
@@ -298,6 +271,8 @@ igt_main
 		set_filter_from_device(fd_drm);
 
 		igt_fail_on_f(close(fd_drm), "Device close failed\n");
+
+		prepare(&priv);
 	}
 
 	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
@@ -325,6 +300,10 @@ igt_main
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose(&priv);
 
-	igt_fixture
+	igt_fixture {
 		post_healthckeck(&priv);
+
+		close(priv.fd.sysfs_bus);
+		close(priv.fd.sysfs_drv);
+	}
 }
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 10/15] tests/core_hotunplug: Skip selectively on sysfs close errors
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since we no longer open a device DRM sysfs node, only a PCI one, driver
unbind operations are no longer affected by missed or unsuccessful
sysfs file close attempts.  Skip only affected subtests if that
happens.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index daf0bf745..71da8f2a1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -82,8 +82,8 @@ static void prepare(struct hotunplug *priv)
 	igt_assert_fd(priv->fd.sysfs_bus);
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
-	igt_assert_f(priv->fd.sysfs_dev == -1,
-		     "Device sysfs node close failed\n");
+	igt_warn_on_f(priv->fd.sysfs_dev != -1,
+		      "Device sysfs node close failed\n");
 }
 
 /* Unbind the driver from the device */
@@ -113,6 +113,9 @@ static void driver_bind(struct hotunplug *priv)
 /* Remove (virtually unplug) the device from its bus */
 static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
+	igt_require_f(priv->fd.sysfs_dev == -1,
+		      "Device sysfs node not closed properly\n");
+
 	priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
 				    O_DIRECTORY);
 	igt_assert_fd(priv->fd.sysfs_dev);
@@ -171,8 +174,6 @@ static void post_healthckeck(struct hotunplug *priv)
 	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
 
 	igt_require_f(priv->fd.drm == -1, "Device not closed properly\n");
-	igt_require_f(priv->fd.sysfs_dev == -1,
-		      "Device sysfs node not closed properly\n");
 }
 
 static void set_filter_from_device(int fd)
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 10/15] tests/core_hotunplug: Skip selectively on sysfs close errors
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since we no longer open a device DRM sysfs node, only a PCI one, driver
unbind operations are no longer affected by missed or unsuccessful
sysfs file close attempts.  Skip only affected subtests if that
happens.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index daf0bf745..71da8f2a1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -82,8 +82,8 @@ static void prepare(struct hotunplug *priv)
 	igt_assert_fd(priv->fd.sysfs_bus);
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
-	igt_assert_f(priv->fd.sysfs_dev == -1,
-		     "Device sysfs node close failed\n");
+	igt_warn_on_f(priv->fd.sysfs_dev != -1,
+		      "Device sysfs node close failed\n");
 }
 
 /* Unbind the driver from the device */
@@ -113,6 +113,9 @@ static void driver_bind(struct hotunplug *priv)
 /* Remove (virtually unplug) the device from its bus */
 static void device_unplug(struct hotunplug *priv, const char *prefix)
 {
+	igt_require_f(priv->fd.sysfs_dev == -1,
+		      "Device sysfs node not closed properly\n");
+
 	priv->fd.sysfs_dev = openat(priv->fd.sysfs_bus, priv->dev_bus_addr,
 				    O_DIRECTORY);
 	igt_assert_fd(priv->fd.sysfs_dev);
@@ -171,8 +174,6 @@ static void post_healthckeck(struct hotunplug *priv)
 	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
 
 	igt_require_f(priv->fd.drm == -1, "Device not closed properly\n");
-	igt_require_f(priv->fd.sysfs_dev == -1,
-		      "Device sysfs node not closed properly\n");
 }
 
 static void set_filter_from_device(int fd)
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 11/15] tests/core_hotunplug: Follow failed subtests with health checks
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Subtests now forcibly call or request igt_abort on failures in order to
avoid silently leaving an exercised device in an unusable state.
However, a failure inside a subtest doesn't always mean the device is
no longer working correctly and reboot is needed.  On the other hand,
if a subtest just fails without aborting, that doesn't mean in turn the
device is healthy.  We should still perform a device health check
in that case before deciding on next steps.

Reuse the 'failure' structure field as a mark which is set when a
subtest enters a chunk of critical steps which must be followed by a
successful health check in order to avoid aborting the test execution.
Then, move health checks from inside each subtest body to its
individual follow-up igt_fixture section from where device file
descriptors potentially left open are closed and the health check is
run if theigt_subtest section has been exited with the marker set.
Also, precede health check operations with a driver rebind or bus
rescan attempt as needed.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 107 +++++++++++++++++++++++++++++------------
 1 file changed, 75 insertions(+), 32 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 71da8f2a1..082bcc306 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -52,6 +52,9 @@ struct hotunplug {
 
 static int local_close(int fd)
 {
+	if (fd < 0)	/* not open - return current status */
+		return fd;
+
 	errno = 0;
 	if (close(fd))	/* close failure - return -errno (never -1) */
 		return -errno;
@@ -91,11 +94,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
 	igt_debug("%sunbinding the driver from the device\n", prefix);
 
-	priv->failure = "Driver unbind timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Driver unbind timeout!");
 	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	priv->failure = NULL;
 }
 
 /* Re-bind the driver to the device */
@@ -103,11 +104,9 @@ static void driver_bind(struct hotunplug *priv)
 {
 	igt_debug("rebinding the driver to the device\n");
 
-	priv->failure = "Driver re-bind timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Driver re-bind timeout!");
 	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	priv->failure = NULL;
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -122,11 +121,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 
 	igt_debug("%sunplugging the device\n", prefix);
 
-	priv->failure = "Device unplug timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Device unplug timeout!");
 	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
 	igt_reset_timeout();
-	priv->failure = NULL;
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_warn_on_f(priv->fd.sysfs_dev != -1,
@@ -138,11 +135,15 @@ static void bus_rescan(struct hotunplug *priv)
 {
 	igt_debug("recovering the device\n");
 
-	priv->failure = "Bus rescan timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Bus rescan timeout!");
 	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
 	igt_reset_timeout();
-	priv->failure = NULL;
+}
+
+static void cleanup(struct hotunplug *priv)
+{
+	priv->fd.drm = local_close(priv->fd.drm);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 }
 
 static void healthcheck(struct hotunplug *priv)
@@ -150,6 +151,18 @@ static void healthcheck(struct hotunplug *priv)
 	/* preserve error code potentially stored before in priv->fd.drm */
 	int fd_drm;
 
+	if (faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0)) {
+		priv->failure = "Bus rescan failed!";
+		bus_rescan(priv);
+		priv->failure = NULL;
+	}
+
+	if (faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0)) {
+		priv->failure = "Driver re-bind failed!";
+		driver_bind(priv);
+		priv->failure = NULL;
+	}
+
 	/* device name may have changed, rebuild IGT device list */
 	igt_devices_scan(true);
 
@@ -169,6 +182,16 @@ static void healthcheck(struct hotunplug *priv)
 	igt_assert_f(fd_drm == -1, "Device close failed\n");
 }
 
+static void recover(struct hotunplug *priv)
+{
+	igt_require(priv->failure);
+	priv->failure = NULL;
+
+	cleanup(priv);
+
+	healthcheck(priv);
+}
+
 static void post_healthckeck(struct hotunplug *priv)
 {
 	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
@@ -195,20 +218,20 @@ static void set_filter_from_device(int fd)
 
 static void unbind_rebind(struct hotunplug *priv)
 {
+	priv->failure = "subtest failure";
+
 	driver_unbind(priv, "");
 
 	driver_bind(priv);
-
-	healthcheck(priv);
 }
 
 static void unplug_rescan(struct hotunplug *priv)
 {
+	priv->failure = "subtest failure";
+
 	device_unplug(priv, "");
 
 	bus_rescan(priv);
-
-	healthcheck(priv);
 }
 
 static void hotunbind_lateclose(struct hotunplug *priv)
@@ -217,6 +240,8 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert_fd(priv->fd.drm);
 
+	priv->failure = "subtest failure";
+
 	driver_unbind(priv, "hot ");
 
 	driver_bind(priv);
@@ -224,8 +249,6 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	igt_debug("late closing the unbound device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
 	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
-
-	healthcheck(priv);
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
@@ -234,6 +257,8 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert_fd(priv->fd.drm);
 
+	priv->failure = "subtest failure";
+
 	device_unplug(priv, "hot ");
 
 	bus_rescan(priv);
@@ -241,8 +266,6 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	igt_debug("late closing the removed device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
 	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
-
-	healthcheck(priv);
 }
 
 /* Main */
@@ -276,30 +299,50 @@ igt_main
 		prepare(&priv);
 	}
 
-	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
-	igt_subtest("unbind-rebind")
-		unbind_rebind(&priv);
+	igt_subtest_group {
+		igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
+		igt_subtest("unbind-rebind")
+			unbind_rebind(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
-	igt_subtest("unplug-rescan")
-		unplug_rescan(&priv);
+	igt_subtest_group {
+		igt_describe("Check if a device believed to be closed can be cleanly unplugged");
+		igt_subtest("unplug-rescan")
+			unplug_rescan(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
-	igt_subtest("hotunbind-lateclose")
-		hotunbind_lateclose(&priv);
+	igt_subtest_group {
+		igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
+		igt_subtest("hotunbind-lateclose")
+			hotunbind_lateclose(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if a still open device can be cleanly unplugged, then released");
-	igt_subtest("hotunplug-lateclose")
-		hotunplug_lateclose(&priv);
+	igt_subtest_group {
+		igt_describe("Check if a still open device can be cleanly unplugged, then released");
+		igt_subtest("hotunplug-lateclose")
+			hotunplug_lateclose(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture {
 		post_healthckeck(&priv);
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 11/15] tests/core_hotunplug: Follow failed subtests with health checks
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Subtests now forcibly call or request igt_abort on failures in order to
avoid silently leaving an exercised device in an unusable state.
However, a failure inside a subtest doesn't always mean the device is
no longer working correctly and reboot is needed.  On the other hand,
if a subtest just fails without aborting, that doesn't mean in turn the
device is healthy.  We should still perform a device health check
in that case before deciding on next steps.

Reuse the 'failure' structure field as a mark which is set when a
subtest enters a chunk of critical steps which must be followed by a
successful health check in order to avoid aborting the test execution.
Then, move health checks from inside each subtest body to its
individual follow-up igt_fixture section from where device file
descriptors potentially left open are closed and the health check is
run if theigt_subtest section has been exited with the marker set.
Also, precede health check operations with a driver rebind or bus
rescan attempt as needed.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 107 +++++++++++++++++++++++++++++------------
 1 file changed, 75 insertions(+), 32 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 71da8f2a1..082bcc306 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -52,6 +52,9 @@ struct hotunplug {
 
 static int local_close(int fd)
 {
+	if (fd < 0)	/* not open - return current status */
+		return fd;
+
 	errno = 0;
 	if (close(fd))	/* close failure - return -errno (never -1) */
 		return -errno;
@@ -91,11 +94,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 {
 	igt_debug("%sunbinding the driver from the device\n", prefix);
 
-	priv->failure = "Driver unbind timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Driver unbind timeout!");
 	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	priv->failure = NULL;
 }
 
 /* Re-bind the driver to the device */
@@ -103,11 +104,9 @@ static void driver_bind(struct hotunplug *priv)
 {
 	igt_debug("rebinding the driver to the device\n");
 
-	priv->failure = "Driver re-bind timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Driver re-bind timeout!");
 	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
 	igt_reset_timeout();
-	priv->failure = NULL;
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -122,11 +121,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 
 	igt_debug("%sunplugging the device\n", prefix);
 
-	priv->failure = "Device unplug timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Device unplug timeout!");
 	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
 	igt_reset_timeout();
-	priv->failure = NULL;
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_warn_on_f(priv->fd.sysfs_dev != -1,
@@ -138,11 +135,15 @@ static void bus_rescan(struct hotunplug *priv)
 {
 	igt_debug("recovering the device\n");
 
-	priv->failure = "Bus rescan timeout!";
-	igt_set_timeout(60, priv->failure);
+	igt_set_timeout(60, "Bus rescan timeout!");
 	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
 	igt_reset_timeout();
-	priv->failure = NULL;
+}
+
+static void cleanup(struct hotunplug *priv)
+{
+	priv->fd.drm = local_close(priv->fd.drm);
+	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 }
 
 static void healthcheck(struct hotunplug *priv)
@@ -150,6 +151,18 @@ static void healthcheck(struct hotunplug *priv)
 	/* preserve error code potentially stored before in priv->fd.drm */
 	int fd_drm;
 
+	if (faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0)) {
+		priv->failure = "Bus rescan failed!";
+		bus_rescan(priv);
+		priv->failure = NULL;
+	}
+
+	if (faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0)) {
+		priv->failure = "Driver re-bind failed!";
+		driver_bind(priv);
+		priv->failure = NULL;
+	}
+
 	/* device name may have changed, rebuild IGT device list */
 	igt_devices_scan(true);
 
@@ -169,6 +182,16 @@ static void healthcheck(struct hotunplug *priv)
 	igt_assert_f(fd_drm == -1, "Device close failed\n");
 }
 
+static void recover(struct hotunplug *priv)
+{
+	igt_require(priv->failure);
+	priv->failure = NULL;
+
+	cleanup(priv);
+
+	healthcheck(priv);
+}
+
 static void post_healthckeck(struct hotunplug *priv)
 {
 	igt_abort_on_f(priv->failure, "%s\n", priv->failure);
@@ -195,20 +218,20 @@ static void set_filter_from_device(int fd)
 
 static void unbind_rebind(struct hotunplug *priv)
 {
+	priv->failure = "subtest failure";
+
 	driver_unbind(priv, "");
 
 	driver_bind(priv);
-
-	healthcheck(priv);
 }
 
 static void unplug_rescan(struct hotunplug *priv)
 {
+	priv->failure = "subtest failure";
+
 	device_unplug(priv, "");
 
 	bus_rescan(priv);
-
-	healthcheck(priv);
 }
 
 static void hotunbind_lateclose(struct hotunplug *priv)
@@ -217,6 +240,8 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert_fd(priv->fd.drm);
 
+	priv->failure = "subtest failure";
+
 	driver_unbind(priv, "hot ");
 
 	driver_bind(priv);
@@ -224,8 +249,6 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 	igt_debug("late closing the unbound device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
 	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
-
-	healthcheck(priv);
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
@@ -234,6 +257,8 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert_fd(priv->fd.drm);
 
+	priv->failure = "subtest failure";
+
 	device_unplug(priv, "hot ");
 
 	bus_rescan(priv);
@@ -241,8 +266,6 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 	igt_debug("late closing the removed device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
 	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
-
-	healthcheck(priv);
 }
 
 /* Main */
@@ -276,30 +299,50 @@ igt_main
 		prepare(&priv);
 	}
 
-	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
-	igt_subtest("unbind-rebind")
-		unbind_rebind(&priv);
+	igt_subtest_group {
+		igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
+		igt_subtest("unbind-rebind")
+			unbind_rebind(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if a device believed to be closed can be cleanly unplugged");
-	igt_subtest("unplug-rescan")
-		unplug_rescan(&priv);
+	igt_subtest_group {
+		igt_describe("Check if a device believed to be closed can be cleanly unplugged");
+		igt_subtest("unplug-rescan")
+			unplug_rescan(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
-	igt_subtest("hotunbind-lateclose")
-		hotunbind_lateclose(&priv);
+	igt_subtest_group {
+		igt_describe("Check if the driver can be cleanly unbound from a still open device, then released");
+		igt_subtest("hotunbind-lateclose")
+			hotunbind_lateclose(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture
 		post_healthckeck(&priv);
 
-	igt_describe("Check if a still open device can be cleanly unplugged, then released");
-	igt_subtest("hotunplug-lateclose")
-		hotunplug_lateclose(&priv);
+	igt_subtest_group {
+		igt_describe("Check if a still open device can be cleanly unplugged, then released");
+		igt_subtest("hotunplug-lateclose")
+			hotunplug_lateclose(&priv);
+
+		igt_fixture
+			recover(&priv);
+	}
 
 	igt_fixture {
 		post_healthckeck(&priv);
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 12/15] tests/core_hotunplug: Fail subtests on device close errors
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since health checks are now run from follow-up fixture sections, it is
safe to fail subtests without the need to abort the test execution.  Do
that on device close errors instead of emitting warnings.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 082bcc306..ec2d891e6 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -126,8 +126,8 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_reset_timeout();
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
-	igt_warn_on_f(priv->fd.sysfs_dev != -1,
-		      "Device sysfs node close failed\n");
+	igt_assert_f(priv->fd.sysfs_dev == -1,
+		     "Device sysfs node close failed\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -248,7 +248,7 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
 	igt_debug("late closing the unbound device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
-	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
@@ -265,7 +265,7 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
 	igt_debug("late closing the removed device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
-	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 }
 
 /* Main */
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 12/15] tests/core_hotunplug: Fail subtests on device close errors
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Since health checks are now run from follow-up fixture sections, it is
safe to fail subtests without the need to abort the test execution.  Do
that on device close errors instead of emitting warnings.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 082bcc306..ec2d891e6 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -126,8 +126,8 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_reset_timeout();
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
-	igt_warn_on_f(priv->fd.sysfs_dev != -1,
-		      "Device sysfs node close failed\n");
+	igt_assert_f(priv->fd.sysfs_dev == -1,
+		     "Device sysfs node close failed\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -248,7 +248,7 @@ static void hotunbind_lateclose(struct hotunplug *priv)
 
 	igt_debug("late closing the unbound device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
-	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 }
 
 static void hotunplug_lateclose(struct hotunplug *priv)
@@ -265,7 +265,7 @@ static void hotunplug_lateclose(struct hotunplug *priv)
 
 	igt_debug("late closing the removed device instance\n");
 	priv->fd.drm = local_close(priv->fd.drm);
-	igt_warn_on_f(priv->fd.drm != -1, "Device close failed\n");
+	igt_assert_f(priv->fd.drm == -1, "Device close failed\n");
 }
 
 /* Main */
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 13/15] tests/core_hotunplug: Process return values of sysfs operations
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Return values of driver bind/unbind / device remove/recover sysfs
operations are now ignored.  Assert their correctness.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index ec2d891e6..16b3a244f 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -95,7 +95,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 	igt_debug("%sunbinding the driver from the device\n", prefix);
 
 	igt_set_timeout(60, "Driver unbind timeout!");
-	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "unbind",
+				   priv->dev_bus_addr),
+		     "Driver unbind failure!");
 	igt_reset_timeout();
 }
 
@@ -105,7 +107,9 @@ static void driver_bind(struct hotunplug *priv)
 	igt_debug("rebinding the driver to the device\n");
 
 	igt_set_timeout(60, "Driver re-bind timeout!");
-	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "bind",
+				   priv->dev_bus_addr),
+		     "Driver re-bind failure!");
 	igt_reset_timeout();
 }
 
@@ -122,7 +126,8 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_debug("%sunplugging the device\n", prefix);
 
 	igt_set_timeout(60, "Device unplug timeout!");
-	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1"),
+		     "Device unplug failure!");
 	igt_reset_timeout();
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
@@ -136,7 +141,8 @@ static void bus_rescan(struct hotunplug *priv)
 	igt_debug("recovering the device\n");
 
 	igt_set_timeout(60, "Bus rescan timeout!");
-	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
+		       "Bus rescan failure!");
 	igt_reset_timeout();
 }
 
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 13/15] tests/core_hotunplug: Process return values of sysfs operations
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Return values of driver bind/unbind / device remove/recover sysfs
operations are now ignored.  Assert their correctness.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index ec2d891e6..16b3a244f 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -95,7 +95,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 	igt_debug("%sunbinding the driver from the device\n", prefix);
 
 	igt_set_timeout(60, "Driver unbind timeout!");
-	igt_sysfs_set(priv->fd.sysfs_drv, "unbind", priv->dev_bus_addr);
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "unbind",
+				   priv->dev_bus_addr),
+		     "Driver unbind failure!");
 	igt_reset_timeout();
 }
 
@@ -105,7 +107,9 @@ static void driver_bind(struct hotunplug *priv)
 	igt_debug("rebinding the driver to the device\n");
 
 	igt_set_timeout(60, "Driver re-bind timeout!");
-	igt_sysfs_set(priv->fd.sysfs_drv, "bind", priv->dev_bus_addr);
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_drv, "bind",
+				   priv->dev_bus_addr),
+		     "Driver re-bind failure!");
 	igt_reset_timeout();
 }
 
@@ -122,7 +126,8 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	igt_debug("%sunplugging the device\n", prefix);
 
 	igt_set_timeout(60, "Device unplug timeout!");
-	igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1");
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_dev, "remove", "1"),
+		     "Device unplug failure!");
 	igt_reset_timeout();
 
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
@@ -136,7 +141,8 @@ static void bus_rescan(struct hotunplug *priv)
 	igt_debug("recovering the device\n");
 
 	igt_set_timeout(60, "Bus rescan timeout!");
-	igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1");
+	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
+		       "Bus rescan failure!");
 	igt_reset_timeout();
 }
 
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 14/15] tests/core_hotunplug: Assert expected device presence/absence
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Don't rely on successful write to sysfs control files, assert existence
/ non-existence of a respective device sysfs node as well.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 16b3a244f..9bca967c1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -99,6 +99,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 				   priv->dev_bus_addr),
 		     "Driver unbind failure!");
 	igt_reset_timeout();
+
+	igt_assert_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0),
+		     "Unbound device still present\n");
 }
 
 /* Re-bind the driver to the device */
@@ -111,6 +114,10 @@ static void driver_bind(struct hotunplug *priv)
 				   priv->dev_bus_addr),
 		     "Driver re-bind failure!");
 	igt_reset_timeout();
+
+	igt_fail_on_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr,
+				F_OK, 0),
+		      "Rebound device not present!\n");
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -133,6 +140,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_assert_f(priv->fd.sysfs_dev == -1,
 		     "Device sysfs node close failed\n");
+
+	igt_assert_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0),
+		     "Unplugged device still present\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -144,6 +154,10 @@ static void bus_rescan(struct hotunplug *priv)
 	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
 		       "Bus rescan failure!");
 	igt_reset_timeout();
+
+	igt_fail_on_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+				F_OK, 0),
+		      "Fakely unplugged device not rediscovered!\n");
 }
 
 static void cleanup(struct hotunplug *priv)
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 14/15] tests/core_hotunplug: Assert expected device presence/absence
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Don't rely on successful write to sysfs control files, assert existence
/ non-existence of a respective device sysfs node as well.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 16b3a244f..9bca967c1 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -99,6 +99,9 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix)
 				   priv->dev_bus_addr),
 		     "Driver unbind failure!");
 	igt_reset_timeout();
+
+	igt_assert_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr, F_OK, 0),
+		     "Unbound device still present\n");
 }
 
 /* Re-bind the driver to the device */
@@ -111,6 +114,10 @@ static void driver_bind(struct hotunplug *priv)
 				   priv->dev_bus_addr),
 		     "Driver re-bind failure!");
 	igt_reset_timeout();
+
+	igt_fail_on_f(faccessat(priv->fd.sysfs_drv, priv->dev_bus_addr,
+				F_OK, 0),
+		      "Rebound device not present!\n");
 }
 
 /* Remove (virtually unplug) the device from its bus */
@@ -133,6 +140,9 @@ static void device_unplug(struct hotunplug *priv, const char *prefix)
 	priv->fd.sysfs_dev = local_close(priv->fd.sysfs_dev);
 	igt_assert_f(priv->fd.sysfs_dev == -1,
 		     "Device sysfs node close failed\n");
+
+	igt_assert_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr, F_OK, 0),
+		     "Unplugged device still present\n");
 }
 
 /* Re-discover the device by rescanning its bus */
@@ -144,6 +154,10 @@ static void bus_rescan(struct hotunplug *priv)
 	igt_assert_f(igt_sysfs_set(priv->fd.sysfs_bus, "../rescan", "1"),
 		       "Bus rescan failure!");
 	igt_reset_timeout();
+
+	igt_fail_on_f(faccessat(priv->fd.sysfs_bus, priv->dev_bus_addr,
+				F_OK, 0),
+		      "Fakely unplugged device not rediscovered!\n");
 }
 
 static void cleanup(struct hotunplug *priv)
-- 
2.21.1

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

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

* [Intel-gfx] [RFC PATH i-g-t 15/15] tests/core_hotunplug: Explicitly ignore unused return values
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  -1 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Some return values are not useful and can be ignored.  Wrap those cases
inside igt_ignore_warn(), not only to make sure compilers are happy but
also to clearly document our decisions.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 9bca967c1..9d4543185 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -227,7 +227,7 @@ static void set_filter_from_device(int fd)
 	char path[PATH_MAX + 1];
 
 	igt_assert(igt_sysfs_path(fd, path, PATH_MAX));
-	strncat(path, "/device", PATH_MAX - strlen(path));
+	igt_ignore_warn(strncat(path, "/device", PATH_MAX - strlen(path)));
 	igt_assert(realpath(path, dst));
 
 	igt_device_filter_free_all();
@@ -367,7 +367,7 @@ igt_main
 	igt_fixture {
 		post_healthckeck(&priv);
 
-		close(priv.fd.sysfs_bus);
-		close(priv.fd.sysfs_drv);
+		igt_ignore_warn(close(priv.fd.sysfs_bus));
+		igt_ignore_warn(close(priv.fd.sysfs_drv));
 	}
 }
-- 
2.21.1

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

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

* [igt-dev] [RFC PATH i-g-t 15/15] tests/core_hotunplug: Explicitly ignore unused return values
@ 2020-07-20 12:19   ` Janusz Krzysztofik
  0 siblings, 0 replies; 34+ messages in thread
From: Janusz Krzysztofik @ 2020-07-20 12:19 UTC (permalink / raw)
  To: igt-dev; +Cc: intel-gfx, Michał Winiarski

Some return values are not useful and can be ignored.  Wrap those cases
inside igt_ignore_warn(), not only to make sure compilers are happy but
also to clearly document our decisions.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 9bca967c1..9d4543185 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -227,7 +227,7 @@ static void set_filter_from_device(int fd)
 	char path[PATH_MAX + 1];
 
 	igt_assert(igt_sysfs_path(fd, path, PATH_MAX));
-	strncat(path, "/device", PATH_MAX - strlen(path));
+	igt_ignore_warn(strncat(path, "/device", PATH_MAX - strlen(path)));
 	igt_assert(realpath(path, dst));
 
 	igt_device_filter_free_all();
@@ -367,7 +367,7 @@ igt_main
 	igt_fixture {
 		post_healthckeck(&priv);
 
-		close(priv.fd.sysfs_bus);
-		close(priv.fd.sysfs_drv);
+		igt_ignore_warn(close(priv.fd.sysfs_bus));
+		igt_ignore_warn(close(priv.fd.sysfs_drv));
 	}
 }
-- 
2.21.1

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

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

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/core_hotunplug: Fixes and enhancements
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
                   ` (15 preceding siblings ...)
  (?)
@ 2020-07-20 13:00 ` Patchwork
  -1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2020-07-20 13:00 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev


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

== Series Details ==

Series: tests/core_hotunplug: Fixes and enhancements
URL   : https://patchwork.freedesktop.org/series/79671/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8766 -> IGTPW_4780
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4780 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4780, 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_4780/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@blt:
    - fi-tgl-u2:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-tgl-u2/igt@i915_selftest@live@blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-tgl-u2/igt@i915_selftest@live@blt.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-tgl-y:           [PASS][3] -> [DMESG-WARN][4] ([i915#402])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       [PASS][7] -> [DMESG-WARN][8] ([i915#2203])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-kbl-7500u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-tgl-u2:          [FAIL][11] ([i915#1888]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-tgl-u2/igt@gem_exec_suspend@basic-s0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-tgl-u2/igt@gem_exec_suspend@basic-s0.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - {fi-kbl-7560u}:     [DMESG-WARN][15] ([i915#1982]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
    - fi-skl-guc:         [DMESG-WARN][17] ([i915#2203]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-tgl-u2:          [DMESG-WARN][19] ([i915#402]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html

  * igt@vgem_basic@sysfs:
    - fi-tgl-y:           [DMESG-WARN][21] ([i915#402]) -> [PASS][22] +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-tgl-y/igt@vgem_basic@sysfs.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-tgl-y/igt@vgem_basic@sysfs.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][23] ([i915#62] / [i915#92]) -> [DMESG-WARN][24] ([i915#62] / [i915#92] / [i915#95]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-modeset@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][25] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][26] ([i915#62] / [i915#92]) +4 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html

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

  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (47 -> 40)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5740 -> IGTPW_4780

  CI-20190529: 20190529
  CI_DRM_8766: 947ce595ea05b4baaea060a7e018cc3f49eaf413 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4780: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/index.html
  IGT_5740: 6663e3ab5f77add7077711c2b649caf2bd7903c4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 8864 bytes --]

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

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

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

* [igt-dev] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: Fixes and enhancements
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
                   ` (16 preceding siblings ...)
  (?)
@ 2020-07-20 16:31 ` Patchwork
  -1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2020-07-20 16:31 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev


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

== Series Details ==

Series: tests/core_hotunplug: Fixes and enhancements
URL   : https://patchwork.freedesktop.org/series/79671/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8766_full -> IGTPW_4780_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_4780_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_4780_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_4780/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_userptr_blits@invalid-mmap-offset-unsync@wb:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb2/igt@gem_userptr_blits@invalid-mmap-offset-unsync@wb.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb5/igt@gem_userptr_blits@invalid-mmap-offset-unsync@wb.html

  * igt@perf@mi-rpc:
    - shard-hsw:          [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw4/igt@perf@mi-rpc.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw7/igt@perf@mi-rpc.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@bonded-early:
    - shard-kbl:          [PASS][5] -> [FAIL][6] ([i915#2079])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-kbl1/igt@gem_exec_balancer@bonded-early.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-kbl4/igt@gem_exec_balancer@bonded-early.html

  * igt@gem_exec_params@rel-constants-invalid-ring:
    - shard-hsw:          [PASS][7] -> [TIMEOUT][8] ([i915#1958] / [i915#2119]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw1/igt@gem_exec_params@rel-constants-invalid-ring.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw7/igt@gem_exec_params@rel-constants-invalid-ring.html

  * igt@gem_exec_reloc@basic-concurrent0:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#1930])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk2/igt@gem_exec_reloc@basic-concurrent0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk5/igt@gem_exec_reloc@basic-concurrent0.html

  * igt@gem_exec_whisper@basic-queues-priority-all:
    - shard-glk:          [PASS][11] -> [DMESG-WARN][12] ([i915#118] / [i915#95])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk8/igt@gem_exec_whisper@basic-queues-priority-all.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk9/igt@gem_exec_whisper@basic-queues-priority-all.html

  * igt@i915_selftest@mock@requests:
    - shard-apl:          [PASS][13] -> [INCOMPLETE][14] ([i915#1635] / [i915#2110])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-apl3/igt@i915_selftest@mock@requests.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-apl4/igt@i915_selftest@mock@requests.html
    - shard-snb:          [PASS][15] -> [INCOMPLETE][16] ([i915#2110] / [i915#82])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb6/igt@i915_selftest@mock@requests.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb4/igt@i915_selftest@mock@requests.html

  * igt@kms_big_fb@y-tiled-64bpp-rotate-0:
    - shard-glk:          [PASS][17] -> [DMESG-FAIL][18] ([i915#118] / [i915#95])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk1/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk8/igt@kms_big_fb@y-tiled-64bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +4 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-glk:          [PASS][21] -> [DMESG-WARN][22] ([i915#1982])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk1/igt@kms_frontbuffer_tracking@fbc-stridechange.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk4/igt@kms_frontbuffer_tracking@fbc-stridechange.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc:
    - shard-tglb:         [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb6/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-mmap-wc.html

  * igt@kms_hdmi_inject@inject-audio:
    - shard-tglb:         [PASS][25] -> [SKIP][26] ([i915#433])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb8/igt@kms_hdmi_inject@inject-audio.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb7/igt@kms_hdmi_inject@inject-audio.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-glk:          [PASS][27] -> [FAIL][28] ([i915#1036])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-tglb:         [PASS][29] -> [SKIP][30] ([i915#1911])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb3/igt@kms_psr2_su@frontbuffer.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][33] -> [FAIL][34] ([i915#31])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-kbl1/igt@kms_setmode@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-kbl7/igt@kms_setmode@basic.html

  * igt@perf@unprivileged-single-ctx-counters:
    - shard-hsw:          [PASS][35] -> [SKIP][36] ([fdo#109271])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw2/igt@perf@unprivileged-single-ctx-counters.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw2/igt@perf@unprivileged-single-ctx-counters.html

  * igt@vgem_basic@dmabuf-export:
    - shard-snb:          [PASS][37] -> [TIMEOUT][38] ([i915#1958] / [i915#2119]) +3 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb6/igt@vgem_basic@dmabuf-export.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb1/igt@vgem_basic@dmabuf-export.html

  
#### Possible fixes ####

  * igt@gem_exec_params@sol-reset-not-gen7:
    - shard-snb:          [TIMEOUT][39] ([i915#1958] / [i915#2119]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb4/igt@gem_exec_params@sol-reset-not-gen7.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb2/igt@gem_exec_params@sol-reset-not-gen7.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][41] ([i915#454]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb8/igt@i915_pm_dc@dc6-psr.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb7/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          [FAIL][43] ([i915#72]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk8/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-hsw:          [FAIL][45] ([i915#57]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw1/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-render:
    - shard-tglb:         [DMESG-WARN][47] ([i915#402]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-render.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt:
    - shard-glk:          [FAIL][49] ([i915#49]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-glk6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-glk2/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-farfromfence:
    - shard-tglb:         [DMESG-WARN][51] ([i915#1982]) -> [PASS][52] +1 similar issue
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb1/igt@kms_frontbuffer_tracking@psr-farfromfence.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb2/igt@kms_frontbuffer_tracking@psr-farfromfence.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [DMESG-WARN][53] ([i915#180]) -> [PASS][54] +3 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [SKIP][55] ([fdo#109441]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb7/igt@kms_psr@psr2_cursor_plane_move.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@perf@blocking-parameterized:
    - shard-iclb:         [FAIL][57] ([i915#1542]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb1/igt@perf@blocking-parameterized.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb6/igt@perf@blocking-parameterized.html

  * igt@perf@global-sseu-config:
    - shard-hsw:          [INCOMPLETE][59] ([i915#1958]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw2/igt@perf@global-sseu-config.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw2/igt@perf@global-sseu-config.html

  * igt@perf@polling-parameterized:
    - shard-tglb:         [FAIL][61] ([i915#1542]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb7/igt@perf@polling-parameterized.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb8/igt@perf@polling-parameterized.html

  * igt@perf_pmu@module-unload:
    - shard-apl:          [DMESG-WARN][63] ([i915#1635] / [i915#1982]) -> [PASS][64] +1 similar issue
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-apl1/igt@perf_pmu@module-unload.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-apl2/igt@perf_pmu@module-unload.html

  
#### Warnings ####

  * igt@gem_exec_params@sol-reset-not-gen7:
    - shard-hsw:          [TIMEOUT][65] ([i915#1958] / [i915#2119]) -> [SKIP][66] ([fdo#109271]) +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw2/igt@gem_exec_params@sol-reset-not-gen7.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw6/igt@gem_exec_params@sol-reset-not-gen7.html

  * igt@gem_exec_reloc@basic-spin-others@vcs0:
    - shard-snb:          [WARN][67] ([i915#2036]) -> [WARN][68] ([i915#2021])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb5/igt@gem_exec_reloc@basic-spin-others@vcs0.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb6/igt@gem_exec_reloc@basic-spin-others@vcs0.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][69] ([i915#658]) -> [SKIP][70] ([i915#588])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb6/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-kbl:          [TIMEOUT][71] ([i915#1319] / [i915#2119]) -> [TIMEOUT][72] ([i915#1319] / [i915#1958] / [i915#2119])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-kbl2/igt@kms_content_protection@atomic-dpms.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-kbl2/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-snb:          [TIMEOUT][73] ([i915#1958] / [i915#2119]) -> [SKIP][74] ([fdo#109271]) +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_psr@primary_mmap_gtt:
    - shard-snb:          [SKIP][75] ([fdo#109271]) -> [TIMEOUT][76] ([i915#1958] / [i915#2119]) +1 similar issue
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-snb4/igt@kms_psr@primary_mmap_gtt.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-snb1/igt@kms_psr@primary_mmap_gtt.html
    - shard-hsw:          [SKIP][77] ([fdo#109271]) -> [TIMEOUT][78] ([i915#1958] / [i915#2119])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-hsw8/igt@kms_psr@primary_mmap_gtt.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-hsw7/igt@kms_psr@primary_mmap_gtt.html

  * igt@perf@unprivileged-single-ctx-counters:
    - shard-iclb:         [SKIP][79] ([fdo#109289]) -> [SKIP][80] ([i915#405])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-iclb8/igt@perf@unprivileged-single-ctx-counters.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-iclb6/igt@perf@unprivileged-single-ctx-counters.html
    - shard-tglb:         [SKIP][81] ([fdo#109289]) -> [SKIP][82] ([i915#405])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-tglb2/igt@perf@unprivileged-single-ctx-counters.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-tglb8/igt@perf@unprivileged-single-ctx-counters.html

  * igt@runner@aborted:
    - shard-apl:          [FAIL][83] ([i915#1610] / [i915#1635] / [i915#2110]) -> ([FAIL][84], [FAIL][85]) ([fdo#109271] / [i915#1635] / [i915#2110] / [i915#716])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8766/shard-apl6/igt@runner@aborted.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-apl4/igt@runner@aborted.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/shard-apl7/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1036]: https://gitlab.freedesktop.org/drm/intel/issues/1036
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1911]: https://gitlab.freedesktop.org/drm/intel/issues/1911
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2021]: https://gitlab.freedesktop.org/drm/intel/issues/2021
  [i915#2036]: https://gitlab.freedesktop.org/drm/intel/issues/2036
  [i915#2079]: https://gitlab.freedesktop.org/drm/intel/issues/2079
  [i915#2110]: https://gitlab.freedesktop.org/drm/intel/issues/2110
  [i915#2119]: https://gitlab.freedesktop.org/drm/intel/issues/2119
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#405]: https://gitlab.freedesktop.org/drm/intel/issues/405
  [i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


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

  Missing    (2): pig-skl-6260u pig-glk-j5005 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5740 -> IGTPW_4780
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_8766: 947ce595ea05b4baaea060a7e018cc3f49eaf413 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4780: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4780/index.html
  IGT_5740: 6663e3ab5f77add7077711c2b649caf2bd7903c4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 23432 bytes --]

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

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

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

* Re: [Intel-gfx] [RFC PATH i-g-t 00/15] tests/core_hotunplug: Fixes and enhancements
  2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
@ 2020-07-22 16:47   ` Michał Winiarski
  -1 siblings, 0 replies; 34+ messages in thread
From: Michał Winiarski @ 2020-07-22 16:47 UTC (permalink / raw)
  To: Janusz Krzysztofik, igt-dev; +Cc: intel-gfx, Michał Winiarski

Quoting Janusz Krzysztofik (2020-07-20 14:18:53)
> Since the test is still blocklisted due to driver issues and won't be
> executed on CI, I'm providing a link where results obtained from a 
> rybot run with the test removed from the blocklist can be found:
> https://patchwork.freedesktop.org/series/79662/
> Failures reported there come from driver and not test issues, I
> believe.
> 
> Thanks,
> Janusz

I'd probably squash some of the patches (e.g. 06/15 with 07/15, maybe more),
but that's just a matter of opinion (and I guess such fine-grained split makes
it easier to review).

LGTM.

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> 
> Janusz Krzysztofik (15):
>   tests/core_hotunplug: Use igt_assert_fd()
>   tests/core_hotunplug: Constify dev_bus_addr string
>   tests/core_hotunplug: Consolidate duplicated debug messages
>   tests/core_hotunplug: Assert successful device filter application
>   tests/core_hotunplug: Fix missing newline
>   tests/core_hotunplug: Maintain a single data structure instance
>   tests/core_hotunplug: Pass errors via a data structure field
>   tests/core_hotunplug: Handle device close errors
>   tests/core_hotunplug: Prepare invariant data once per test run
>   tests/core_hotunplug: Skip selectively on sysfs close errors
>   tests/core_hotunplug: Follow failed subtests with health checks
>   tests/core_hotunplug: Fail subtests on device close errors
>   tests/core_hotunplug: Process return values of sysfs operations
>   tests/core_hotunplug: Assert expected device presence/absence
>   tests/core_hotunplug: Explicitly ignore unused return values
> 
>  tests/core_hotunplug.c | 333 +++++++++++++++++++++++++----------------
>  1 file changed, 202 insertions(+), 131 deletions(-)
> 
> -- 
> 2.21.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [igt-dev] [Intel-gfx] [RFC PATH i-g-t 00/15] tests/core_hotunplug: Fixes and enhancements
@ 2020-07-22 16:47   ` Michał Winiarski
  0 siblings, 0 replies; 34+ messages in thread
From: Michał Winiarski @ 2020-07-22 16:47 UTC (permalink / raw)
  To: Janusz Krzysztofik, igt-dev; +Cc: intel-gfx, Michał Winiarski

Quoting Janusz Krzysztofik (2020-07-20 14:18:53)
> Since the test is still blocklisted due to driver issues and won't be
> executed on CI, I'm providing a link where results obtained from a 
> rybot run with the test removed from the blocklist can be found:
> https://patchwork.freedesktop.org/series/79662/
> Failures reported there come from driver and not test issues, I
> believe.
> 
> Thanks,
> Janusz

I'd probably squash some of the patches (e.g. 06/15 with 07/15, maybe more),
but that's just a matter of opinion (and I guess such fine-grained split makes
it easier to review).

LGTM.

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> 
> Janusz Krzysztofik (15):
>   tests/core_hotunplug: Use igt_assert_fd()
>   tests/core_hotunplug: Constify dev_bus_addr string
>   tests/core_hotunplug: Consolidate duplicated debug messages
>   tests/core_hotunplug: Assert successful device filter application
>   tests/core_hotunplug: Fix missing newline
>   tests/core_hotunplug: Maintain a single data structure instance
>   tests/core_hotunplug: Pass errors via a data structure field
>   tests/core_hotunplug: Handle device close errors
>   tests/core_hotunplug: Prepare invariant data once per test run
>   tests/core_hotunplug: Skip selectively on sysfs close errors
>   tests/core_hotunplug: Follow failed subtests with health checks
>   tests/core_hotunplug: Fail subtests on device close errors
>   tests/core_hotunplug: Process return values of sysfs operations
>   tests/core_hotunplug: Assert expected device presence/absence
>   tests/core_hotunplug: Explicitly ignore unused return values
> 
>  tests/core_hotunplug.c | 333 +++++++++++++++++++++++++----------------
>  1 file changed, 202 insertions(+), 131 deletions(-)
> 
> -- 
> 2.21.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2020-07-22 16:47 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 12:18 [Intel-gfx] [RFC PATH i-g-t 00/15] tests/core_hotunplug: Fixes and enhancements Janusz Krzysztofik
2020-07-20 12:18 ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 01/15] tests/core_hotunplug: Use igt_assert_fd() Janusz Krzysztofik
2020-07-20 12:18   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 02/15] tests/core_hotunplug: Constify dev_bus_addr string Janusz Krzysztofik
2020-07-20 12:18   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 03/15] tests/core_hotunplug: Consolidate duplicated debug messages Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 04/15] tests/core_hotunplug: Assert successful device filter application Janusz Krzysztofik
2020-07-20 12:18   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 05/15] tests/core_hotunplug: Fix missing newline Janusz Krzysztofik
2020-07-20 12:18   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:18 ` [Intel-gfx] [RFC PATH i-g-t 06/15] tests/core_hotunplug: Maintain a single data structure instance Janusz Krzysztofik
2020-07-20 12:18   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 07/15] tests/core_hotunplug: Pass errors via a data structure field Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 08/15] tests/core_hotunplug: Handle device close errors Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 09/15] tests/core_hotunplug: Prepare invariant data once per test run Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 10/15] tests/core_hotunplug: Skip selectively on sysfs close errors Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 11/15] tests/core_hotunplug: Follow failed subtests with health checks Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 12/15] tests/core_hotunplug: Fail subtests on device close errors Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 13/15] tests/core_hotunplug: Process return values of sysfs operations Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 14/15] tests/core_hotunplug: Assert expected device presence/absence Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 12:19 ` [Intel-gfx] [RFC PATH i-g-t 15/15] tests/core_hotunplug: Explicitly ignore unused return values Janusz Krzysztofik
2020-07-20 12:19   ` [igt-dev] " Janusz Krzysztofik
2020-07-20 13:00 ` [igt-dev] ✗ Fi.CI.BAT: failure for tests/core_hotunplug: Fixes and enhancements Patchwork
2020-07-20 16:31 ` [igt-dev] ✗ Fi.CI.IGT: " Patchwork
2020-07-22 16:47 ` [Intel-gfx] [RFC PATH i-g-t 00/15] " Michał Winiarski
2020-07-22 16:47   ` [igt-dev] " Michał Winiarski

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.