[Intel-gfx] [RFC PATCH i-g-t v2 0/8] tests/core_hotunplug: New subtests and enhancements

[Intel-gfx] [RFC PATCH i-g-t v2 0/8] tests/core_hotunplug: New subtests and enhancements

[Janusz Krzysztofik]

Add a bunch of new test variants, enhance debugging of hotunplug driver
issues.

v2: rebase on upstream

Janusz Krzysztofik (8):
  tests/core_hotunplug: Duplicate debug messages in dmesg
  tests/core_hotunplug: Use PCI device sysfs entry, not DRM
  tests/core_hotunplug: Add unbind-unplug-rescan variant
  tests/core_hotunplug: Add 'lateclose before recover' variants
  tests/core_hotunplug: Add 'GEM address space' variant
  tests/core_hotunplug: Add 'GEM object' variant
  tests/core_hotunplug: Add 'PRIME handle' variant
  tests/core_hotunplug: Add 'GEM batch' variant

 tests/core_hotunplug.c | 330 +++++++++++++++++++++++++++++++++++------
 1 file changed, 283 insertions(+), 47 deletions(-)

-- 
2.21.1

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

[igt-dev] [RFC PATCH i-g-t v2 0/8] tests/core_hotunplug: New subtests and enhancements

[Janusz Krzysztofik]

Add a bunch of new test variants, enhance debugging of hotunplug driver
issues.

v2: rebase on upstream

Janusz Krzysztofik (8):
  tests/core_hotunplug: Duplicate debug messages in dmesg
  tests/core_hotunplug: Use PCI device sysfs entry, not DRM
  tests/core_hotunplug: Add unbind-unplug-rescan variant
  tests/core_hotunplug: Add 'lateclose before recover' variants
  tests/core_hotunplug: Add 'GEM address space' variant
  tests/core_hotunplug: Add 'GEM object' variant
  tests/core_hotunplug: Add 'PRIME handle' variant
  tests/core_hotunplug: Add 'GEM batch' variant

 tests/core_hotunplug.c | 330 +++++++++++++++++++++++++++++++++++------
 1 file changed, 283 insertions(+), 47 deletions(-)

-- 
2.21.1

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

[Intel-gfx] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Janusz Krzysztofik]

The purpose of debug messages displayed by the test is to make
identification of a subtest phase that fails more easy.  Since issues
exhibited by the test are mostly reported to dmesg, print those debug
messages to /dev/kmsg as well.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e03f3b945..826645b1f 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -49,6 +49,12 @@ struct hotunplug {
 
 /* Helpers */
 
+#define local_debug(msg...)						     \
+({									     \
+	igt_debug("%s: %s\n", __func__, msg);				     \
+	igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
+})
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
 	int len;
@@ -68,9 +74,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 	close(priv->fd.sysfs_dev);
 }
 
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
+static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
-	igt_debug("opening device\n");
+	local_debug("opening device");
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert(priv->fd.drm >= 0);
 
@@ -137,14 +143,14 @@ static void bus_rescan(int fd_sysfs_bus)
 	close(fd_sysfs_bus);
 }
 
-static void healthcheck(void)
+static inline void healthcheck(void)
 {
 	int fd_drm;
 
 	/* device name may have changed, rebuild IGT device list */
 	igt_devices_scan(true);
 
-	igt_debug("reopening the device\n");
+	local_debug("reopening the device");
 	fd_drm = __drm_open_driver(DRIVER_ANY);
 	igt_abort_on_f(fd_drm < 0, "Device reopen failure");
 
@@ -181,13 +187,13 @@ static void unbind_rebind(void)
 
 	prepare(&priv, buf, sizeof(buf));
 
-	igt_debug("closing the device\n");
+	local_debug("closing the device");
 	close(priv.fd.drm);
 
-	igt_debug("unbinding the driver from the device\n");
+	local_debug("unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("rebinding the driver to the device\n");
+	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
 	healthcheck();
@@ -199,13 +205,13 @@ static void unplug_rescan(void)
 
 	prepare(&priv, NULL, 0);
 
-	igt_debug("closing the device\n");
+	local_debug("closing the device");
 	close(priv.fd.drm);
 
-	igt_debug("unplugging the device\n");
+	local_debug("unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
 
-	igt_debug("recovering the device\n");
+	local_debug("recovering the device");
 	bus_rescan(priv.fd.sysfs_bus);
 
 	healthcheck();
@@ -218,13 +224,13 @@ static void hotunbind_lateclose(void)
 
 	prepare(&priv, buf, sizeof(buf));
 
-	igt_debug("hot unbinding the driver from the device\n");
+	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("rebinding the driver to the device\n");
+	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("late closing the unbound device instance\n");
+	local_debug("late closing the unbound device instance");
 	close(priv.fd.drm);
 
 	healthcheck();
@@ -236,13 +242,13 @@ static void hotunplug_lateclose(void)
 
 	prepare(&priv, NULL, 0);
 
-	igt_debug("hot unplugging the device\n");
+	local_debug("hot unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
 
-	igt_debug("recovering the device\n");
+	local_debug("recovering the device");
 	bus_rescan(priv.fd.sysfs_bus);
 
-	igt_debug("late closing the removed device instance\n");
+	local_debug("late closing the removed device instance");
 	close(priv.fd.drm);
 
 	healthcheck();
-- 
2.21.1

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

[igt-dev] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Janusz Krzysztofik]

The purpose of debug messages displayed by the test is to make
identification of a subtest phase that fails more easy.  Since issues
exhibited by the test are mostly reported to dmesg, print those debug
messages to /dev/kmsg as well.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index e03f3b945..826645b1f 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -49,6 +49,12 @@ struct hotunplug {
 
 /* Helpers */
 
+#define local_debug(msg...)						     \
+({									     \
+	igt_debug("%s: %s\n", __func__, msg);				     \
+	igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
+})
+
 static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 {
 	int len;
@@ -68,9 +74,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
 	close(priv->fd.sysfs_dev);
 }
 
-static void prepare(struct hotunplug *priv, char *buf, int buflen)
+static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
 {
-	igt_debug("opening device\n");
+	local_debug("opening device");
 	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
 	igt_assert(priv->fd.drm >= 0);
 
@@ -137,14 +143,14 @@ static void bus_rescan(int fd_sysfs_bus)
 	close(fd_sysfs_bus);
 }
 
-static void healthcheck(void)
+static inline void healthcheck(void)
 {
 	int fd_drm;
 
 	/* device name may have changed, rebuild IGT device list */
 	igt_devices_scan(true);
 
-	igt_debug("reopening the device\n");
+	local_debug("reopening the device");
 	fd_drm = __drm_open_driver(DRIVER_ANY);
 	igt_abort_on_f(fd_drm < 0, "Device reopen failure");
 
@@ -181,13 +187,13 @@ static void unbind_rebind(void)
 
 	prepare(&priv, buf, sizeof(buf));
 
-	igt_debug("closing the device\n");
+	local_debug("closing the device");
 	close(priv.fd.drm);
 
-	igt_debug("unbinding the driver from the device\n");
+	local_debug("unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("rebinding the driver to the device\n");
+	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
 	healthcheck();
@@ -199,13 +205,13 @@ static void unplug_rescan(void)
 
 	prepare(&priv, NULL, 0);
 
-	igt_debug("closing the device\n");
+	local_debug("closing the device");
 	close(priv.fd.drm);
 
-	igt_debug("unplugging the device\n");
+	local_debug("unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
 
-	igt_debug("recovering the device\n");
+	local_debug("recovering the device");
 	bus_rescan(priv.fd.sysfs_bus);
 
 	healthcheck();
@@ -218,13 +224,13 @@ static void hotunbind_lateclose(void)
 
 	prepare(&priv, buf, sizeof(buf));
 
-	igt_debug("hot unbinding the driver from the device\n");
+	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("rebinding the driver to the device\n");
+	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
-	igt_debug("late closing the unbound device instance\n");
+	local_debug("late closing the unbound device instance");
 	close(priv.fd.drm);
 
 	healthcheck();
@@ -236,13 +242,13 @@ static void hotunplug_lateclose(void)
 
 	prepare(&priv, NULL, 0);
 
-	igt_debug("hot unplugging the device\n");
+	local_debug("hot unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
 
-	igt_debug("recovering the device\n");
+	local_debug("recovering the device");
 	bus_rescan(priv.fd.sysfs_bus);
 
-	igt_debug("late closing the removed device instance\n");
+	local_debug("late closing the removed device instance");
 	close(priv.fd.drm);
 
 	healthcheck();
-- 
2.21.1

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

Re: [Intel-gfx] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> The purpose of debug messages displayed by the test is to make
> identification of a subtest phase that fails more easy.  Since issues
> exhibited by the test are mostly reported to dmesg, print those debug
> messages to /dev/kmsg as well.

I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this logging
to the kernel, but let's go over this case-by-case.

> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index e03f3b945..826645b1f 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -49,6 +49,12 @@ struct hotunplug {
>  
>  /* Helpers */
>  
> +#define local_debug(msg...)                                                 \
> +({                                                                          \
> +       igt_debug("%s: %s\n", __func__, msg);                                \
> +       igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
> +})
> +
>  static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
>  {
>         int len;
> @@ -68,9 +74,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
>         close(priv->fd.sysfs_dev);
>  }
>  
> -static void prepare(struct hotunplug *priv, char *buf, int buflen)
> +static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
>  {
> -       igt_debug("opening device\n");
> +       local_debug("opening device");

[  220.458370] [drm:drm_open] pid = 194, minor = 128
[  220.460062] [drm:i915_gem_open [i915]]

>         priv->fd.drm = __drm_open_driver(DRIVER_ANY);
>         igt_assert(priv->fd.drm >= 0);
>  
> @@ -137,14 +143,14 @@ static void bus_rescan(int fd_sysfs_bus)
>         close(fd_sysfs_bus);
>  }
>  
> -static void healthcheck(void)
> +static inline void healthcheck(void)
>  {
>         int fd_drm;
>  
>         /* device name may have changed, rebuild IGT device list */
>         igt_devices_scan(true);
>  
> -       igt_debug("reopening the device\n");
> +       local_debug("reopening the device");

Well, this is going to look the same as open, except closing it won't print
drm_lastclose.

[  293.957567] [drm:drm_release] open_count = 2
[  293.958805] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 2

>         fd_drm = __drm_open_driver(DRIVER_ANY);
>         igt_abort_on_f(fd_drm < 0, "Device reopen failure");
>  
> @@ -181,13 +187,13 @@ static void unbind_rebind(void)
>  
>         prepare(&priv, buf, sizeof(buf));
>  
> -       igt_debug("closing the device\n");
> +       local_debug("closing the device");

[  250.157568] [drm:drm_release] open_count = 1
[  250.158807] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 1
[  250.161183] [drm:drm_lastclose]
[  250.162312] [drm:drm_lastclose] driver lastclose completed

>         close(priv.fd.drm);
>  
> -       igt_debug("unbinding the driver from the device\n");
> +       local_debug("unbinding the driver from the device");
>         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);

[ 1553.868235] bus: 'event_source': remove device i915_0000_00_02.0

>  
> -       igt_debug("rebinding the driver to the device\n");
> +       local_debug("rebinding the driver to the device");
>         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);

[ 1592.758219] bus: 'pci': driver_probe_device: matched device 0000:00:02.0 with driver i915
[ 1592.760543] bus: 'pci': really_probe: probing driver i915 with device 0000:00:02.0
(...bunch of i915 logs...)
[  203.961656] driver: 'i915': driver_bound: bound to device '0000:00:02.0'
[  203.966421] bus: 'pci': really_probe: bound device 0000:00:02.0 to driver i915

>  
>         healthcheck();
> @@ -199,13 +205,13 @@ static void unplug_rescan(void)
>  
>         prepare(&priv, NULL, 0);
>  
> -       igt_debug("closing the device\n");
> +       local_debug("closing the device");
>         close(priv.fd.drm);
>  
> -       igt_debug("unplugging the device\n");
> +       local_debug("unplugging the device");
>         device_unplug(priv.fd.sysfs_dev);

[   60.664163] bus: 'pci': remove device 0000:00:02.0

> -       igt_debug("recovering the device\n");
> +       local_debug("recovering the device");
>         bus_rescan(priv.fd.sysfs_bus);

[   97.384479] bus: 'pci': add device 0000:00:02.0

>  
>         healthcheck();
> @@ -218,13 +224,13 @@ static void hotunbind_lateclose(void)
>  
>         prepare(&priv, buf, sizeof(buf));
>  
> -       igt_debug("hot unbinding the driver from the device\n");
> +       local_debug("hot unbinding the driver from the device");
>         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
>  
> -       igt_debug("rebinding the driver to the device\n");
> +       local_debug("rebinding the driver to the device");
>         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
>  
> -       igt_debug("late closing the unbound device instance\n");
> +       local_debug("late closing the unbound device instance");
>         close(priv.fd.drm);

Would it be possible to add extra logging allowing us to distinguish this from
regular unbind on i915 side?

>  
>         healthcheck();
> @@ -236,13 +242,13 @@ static void hotunplug_lateclose(void)
>  
>         prepare(&priv, NULL, 0);
>  
> -       igt_debug("hot unplugging the device\n");
> +       local_debug("hot unplugging the device");
>         device_unplug(priv.fd.sysfs_dev);
>  
> -       igt_debug("recovering the device\n");
> +       local_debug("recovering the device");
>         bus_rescan(priv.fd.sysfs_bus);
>  
> -       igt_debug("late closing the removed device instance\n");
> +       local_debug("late closing the removed device instance");
>         close(priv.fd.drm);

Same thing here.

So, not including the hot unplug/unbind, I think the logging is already there.

Also - note, the "driver core" logs are probably disabled on CI, but I still
think that figuring out how to enable those from IGT (and letting the kernel
just do its regular logging) rather than adding kmsg prints from userspace is a
better approach.

-Michał

>  
>         healthcheck();
> -- 
> 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

Re: [Intel-gfx] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Janusz Krzysztofik]

Hi Michał,

Thanks for review.

On Thu, 2020-06-25 at 17:27 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> > The purpose of debug messages displayed by the test is to make
> > identification of a subtest phase that fails more easy.  Since issues
> > exhibited by the test are mostly reported to dmesg, print those debug
> > messages to /dev/kmsg as well.
> 
> I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this logging
> to the kernel, 

The idea was to simply log IGT actions, not to log kernel reactions on
them which already happens.  Doing that from the kernel would probably
require modifications to PCI sysfs handling or to sysfs in general.

If you see no benefits from that, let's drop this patch.

Thanks,
Janusz 
 
> but let's go over this case-by-case.
> 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 38 ++++++++++++++++++++++----------------
> >  1 file changed, 22 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index e03f3b945..826645b1f 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -49,6 +49,12 @@ struct hotunplug {
> >  
> >  /* Helpers */
> >  
> > +#define local_debug(msg...)                                                 \
> > +({                                                                          \
> > +       igt_debug("%s: %s\n", __func__, msg);                                \
> > +       igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
> > +})
> > +
> >  static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> >  {
> >         int len;
> > @@ -68,9 +74,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> >         close(priv->fd.sysfs_dev);
> >  }
> >  
> > -static void prepare(struct hotunplug *priv, char *buf, int buflen)
> > +static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
> >  {
> > -       igt_debug("opening device\n");
> > +       local_debug("opening device");
> 
> [  220.458370] [drm:drm_open] pid = 194, minor = 128
> [  220.460062] [drm:i915_gem_open [i915]]
> 
> >         priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> >         igt_assert(priv->fd.drm >= 0);
> >  
> > @@ -137,14 +143,14 @@ static void bus_rescan(int fd_sysfs_bus)
> >         close(fd_sysfs_bus);
> >  }
> >  
> > -static void healthcheck(void)
> > +static inline void healthcheck(void)
> >  {
> >         int fd_drm;
> >  
> >         /* device name may have changed, rebuild IGT device list */
> >         igt_devices_scan(true);
> >  
> > -       igt_debug("reopening the device\n");
> > +       local_debug("reopening the device");
> 
> Well, this is going to look the same as open, except closing it won't print
> drm_lastclose.
> 
> [  293.957567] [drm:drm_release] open_count = 2
> [  293.958805] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 2
> 
> >         fd_drm = __drm_open_driver(DRIVER_ANY);
> >         igt_abort_on_f(fd_drm < 0, "Device reopen failure");
> >  
> > @@ -181,13 +187,13 @@ static void unbind_rebind(void)
> >  
> >         prepare(&priv, buf, sizeof(buf));
> >  
> > -       igt_debug("closing the device\n");
> > +       local_debug("closing the device");
> 
> [  250.157568] [drm:drm_release] open_count = 1
> [  250.158807] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 1
> [  250.161183] [drm:drm_lastclose]
> [  250.162312] [drm:drm_lastclose] driver lastclose completed
> 
> >         close(priv.fd.drm);
> >  
> > -       igt_debug("unbinding the driver from the device\n");
> > +       local_debug("unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> 
> [ 1553.868235] bus: 'event_source': remove device i915_0000_00_02.0
> 
> >  
> > -       igt_debug("rebinding the driver to the device\n");
> > +       local_debug("rebinding the driver to the device");
> >         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> 
> [ 1592.758219] bus: 'pci': driver_probe_device: matched device 0000:00:02.0 with driver i915
> [ 1592.760543] bus: 'pci': really_probe: probing driver i915 with device 0000:00:02.0
> (...bunch of i915 logs...)
> [  203.961656] driver: 'i915': driver_bound: bound to device '0000:00:02.0'
> [  203.966421] bus: 'pci': really_probe: bound device 0000:00:02.0 to driver i915
> 
> >  
> >         healthcheck();
> > @@ -199,13 +205,13 @@ static void unplug_rescan(void)
> >  
> >         prepare(&priv, NULL, 0);
> >  
> > -       igt_debug("closing the device\n");
> > +       local_debug("closing the device");
> >         close(priv.fd.drm);
> >  
> > -       igt_debug("unplugging the device\n");
> > +       local_debug("unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> 
> [   60.664163] bus: 'pci': remove device 0000:00:02.0
> 
> > -       igt_debug("recovering the device\n");
> > +       local_debug("recovering the device");
> >         bus_rescan(priv.fd.sysfs_bus);
> 
> [   97.384479] bus: 'pci': add device 0000:00:02.0
> 
> >  
> >         healthcheck();
> > @@ -218,13 +224,13 @@ static void hotunbind_lateclose(void)
> >  
> >         prepare(&priv, buf, sizeof(buf));
> >  
> > -       igt_debug("hot unbinding the driver from the device\n");
> > +       local_debug("hot unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> >  
> > -       igt_debug("rebinding the driver to the device\n");
> > +       local_debug("rebinding the driver to the device");
> >         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> >  
> > -       igt_debug("late closing the unbound device instance\n");
> > +       local_debug("late closing the unbound device instance");
> >         close(priv.fd.drm);
> 
> Would it be possible to add extra logging allowing us to distinguish this from
> regular unbind on i915 side?
> 
> >  
> >         healthcheck();
> > @@ -236,13 +242,13 @@ static void hotunplug_lateclose(void)
> >  
> >         prepare(&priv, NULL, 0);
> >  
> > -       igt_debug("hot unplugging the device\n");
> > +       local_debug("hot unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> >  
> > -       igt_debug("recovering the device\n");
> > +       local_debug("recovering the device");
> >         bus_rescan(priv.fd.sysfs_bus);
> >  
> > -       igt_debug("late closing the removed device instance\n");
> > +       local_debug("late closing the removed device instance");
> >         close(priv.fd.drm);
> 
> Same thing here.
> 
> So, not including the hot unplug/unbind, I think the logging is already there.
> 
> Also - note, the "driver core" logs are probably disabled on CI, but I still
> think that figuring out how to enable those from IGT (and letting the kernel
> just do its regular logging) rather than adding kmsg prints from userspace is a
> better approach.
> 
> -Michał
> 
> >  
> >         healthcheck();
> > -- 
> > 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

Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Janusz Krzysztofik]

Hi Michał,

Thanks for review.

On Thu, 2020-06-25 at 17:27 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> > The purpose of debug messages displayed by the test is to make
> > identification of a subtest phase that fails more easy.  Since issues
> > exhibited by the test are mostly reported to dmesg, print those debug
> > messages to /dev/kmsg as well.
> 
> I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this logging
> to the kernel, 

The idea was to simply log IGT actions, not to log kernel reactions on
them which already happens.  Doing that from the kernel would probably
require modifications to PCI sysfs handling or to sysfs in general.

If you see no benefits from that, let's drop this patch.

Thanks,
Janusz 
 
> but let's go over this case-by-case.
> 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 38 ++++++++++++++++++++++----------------
> >  1 file changed, 22 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index e03f3b945..826645b1f 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -49,6 +49,12 @@ struct hotunplug {
> >  
> >  /* Helpers */
> >  
> > +#define local_debug(msg...)                                                 \
> > +({                                                                          \
> > +       igt_debug("%s: %s\n", __func__, msg);                                \
> > +       igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
> > +})
> > +
> >  static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> >  {
> >         int len;
> > @@ -68,9 +74,9 @@ static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> >         close(priv->fd.sysfs_dev);
> >  }
> >  
> > -static void prepare(struct hotunplug *priv, char *buf, int buflen)
> > +static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
> >  {
> > -       igt_debug("opening device\n");
> > +       local_debug("opening device");
> 
> [  220.458370] [drm:drm_open] pid = 194, minor = 128
> [  220.460062] [drm:i915_gem_open [i915]]
> 
> >         priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> >         igt_assert(priv->fd.drm >= 0);
> >  
> > @@ -137,14 +143,14 @@ static void bus_rescan(int fd_sysfs_bus)
> >         close(fd_sysfs_bus);
> >  }
> >  
> > -static void healthcheck(void)
> > +static inline void healthcheck(void)
> >  {
> >         int fd_drm;
> >  
> >         /* device name may have changed, rebuild IGT device list */
> >         igt_devices_scan(true);
> >  
> > -       igt_debug("reopening the device\n");
> > +       local_debug("reopening the device");
> 
> Well, this is going to look the same as open, except closing it won't print
> drm_lastclose.
> 
> [  293.957567] [drm:drm_release] open_count = 2
> [  293.958805] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 2
> 
> >         fd_drm = __drm_open_driver(DRIVER_ANY);
> >         igt_abort_on_f(fd_drm < 0, "Device reopen failure");
> >  
> > @@ -181,13 +187,13 @@ static void unbind_rebind(void)
> >  
> >         prepare(&priv, buf, sizeof(buf));
> >  
> > -       igt_debug("closing the device\n");
> > +       local_debug("closing the device");
> 
> [  250.157568] [drm:drm_release] open_count = 1
> [  250.158807] [drm:drm_file_free.part.0] pid = 194, device = 0xe280, open_count = 1
> [  250.161183] [drm:drm_lastclose]
> [  250.162312] [drm:drm_lastclose] driver lastclose completed
> 
> >         close(priv.fd.drm);
> >  
> > -       igt_debug("unbinding the driver from the device\n");
> > +       local_debug("unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> 
> [ 1553.868235] bus: 'event_source': remove device i915_0000_00_02.0
> 
> >  
> > -       igt_debug("rebinding the driver to the device\n");
> > +       local_debug("rebinding the driver to the device");
> >         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> 
> [ 1592.758219] bus: 'pci': driver_probe_device: matched device 0000:00:02.0 with driver i915
> [ 1592.760543] bus: 'pci': really_probe: probing driver i915 with device 0000:00:02.0
> (...bunch of i915 logs...)
> [  203.961656] driver: 'i915': driver_bound: bound to device '0000:00:02.0'
> [  203.966421] bus: 'pci': really_probe: bound device 0000:00:02.0 to driver i915
> 
> >  
> >         healthcheck();
> > @@ -199,13 +205,13 @@ static void unplug_rescan(void)
> >  
> >         prepare(&priv, NULL, 0);
> >  
> > -       igt_debug("closing the device\n");
> > +       local_debug("closing the device");
> >         close(priv.fd.drm);
> >  
> > -       igt_debug("unplugging the device\n");
> > +       local_debug("unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> 
> [   60.664163] bus: 'pci': remove device 0000:00:02.0
> 
> > -       igt_debug("recovering the device\n");
> > +       local_debug("recovering the device");
> >         bus_rescan(priv.fd.sysfs_bus);
> 
> [   97.384479] bus: 'pci': add device 0000:00:02.0
> 
> >  
> >         healthcheck();
> > @@ -218,13 +224,13 @@ static void hotunbind_lateclose(void)
> >  
> >         prepare(&priv, buf, sizeof(buf));
> >  
> > -       igt_debug("hot unbinding the driver from the device\n");
> > +       local_debug("hot unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> >  
> > -       igt_debug("rebinding the driver to the device\n");
> > +       local_debug("rebinding the driver to the device");
> >         driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> >  
> > -       igt_debug("late closing the unbound device instance\n");
> > +       local_debug("late closing the unbound device instance");
> >         close(priv.fd.drm);
> 
> Would it be possible to add extra logging allowing us to distinguish this from
> regular unbind on i915 side?
> 
> >  
> >         healthcheck();
> > @@ -236,13 +242,13 @@ static void hotunplug_lateclose(void)
> >  
> >         prepare(&priv, NULL, 0);
> >  
> > -       igt_debug("hot unplugging the device\n");
> > +       local_debug("hot unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> >  
> > -       igt_debug("recovering the device\n");
> > +       local_debug("recovering the device");
> >         bus_rescan(priv.fd.sysfs_bus);
> >  
> > -       igt_debug("late closing the removed device instance\n");
> > +       local_debug("late closing the removed device instance");
> >         close(priv.fd.drm);
> 
> Same thing here.
> 
> So, not including the hot unplug/unbind, I think the logging is already there.
> 
> Also - note, the "driver core" logs are probably disabled on CI, but I still
> think that figuring out how to enable those from IGT (and letting the kernel
> just do its regular logging) rather than adding kmsg prints from userspace is a
> better approach.
> 
> -Michał
> 
> >  
> >         healthcheck();
> > -- 
> > 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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Arkadiusz Hiler]

On Fri, Jun 26, 2020 at 12:18:00PM +0200, Janusz Krzysztofik wrote:
> Hi Michał,
> 
> Thanks for review.
> 
> On Thu, 2020-06-25 at 17:27 +0200, Michał Winiarski wrote:
> > Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> > > The purpose of debug messages displayed by the test is to make
> > > identification of a subtest phase that fails more easy.  Since issues
> > > exhibited by the test are mostly reported to dmesg, print those debug
> > > messages to /dev/kmsg as well.
> > 
> > I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this logging
> > to the kernel, 
> The idea was to simply log IGT actions, not to log kernel reactions on
> them which already happens.  Doing that from the kernel would probably
> require modifications to PCI sysfs handling or to sysfs in general.
> 
> If you see no benefits from that, let's drop this patch.

We (me & Petri) were thinking about doing something similar, but only
for the places where kernel logs are hard to correlate with the test
actions, to have those "synchronization points" between logs for key
operations.

The main reason was Chamelium - external board that simulates display
and can cause hotplug events to happen. Logging Chamelium operations in
dmesg would make any kind of bug assessment or troubleshooting much
easier - was that a phantom hotplug? or something that was triggered by
us? Have we started doing anything else before the link has settled?
What happened during DP FSM handling?

This is a very special case as we deal with an external board that
drives our HW through wires and layers of firmware and for sure there be
dragons.


Anyway, I would like to see us having a way of logging those "sync
points" into kmesg in igt_core, e.g. by creating _kmesg suffixed version
of log functions.

But I also see Michał's point - too frivolous logging just creates
noise, and we shouldn't double log - if something is already easy to
find in the kernel logs and the correlating test action to logs is not
too hard why should we add more noise?

As of the examples in this thread - I am not very familiar with the area
and I leave it up to you two to decide what would be helpful, what would
be unnecessary repetition and what would be better off logged in the
kernel.

TL;DR: Yes for logging things into kmesg, but we should be careful about
       what we log to not make the situation any worse.

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

Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Arkadiusz Hiler]

On Fri, Jun 26, 2020 at 12:18:00PM +0200, Janusz Krzysztofik wrote:
> Hi Michał,
> 
> Thanks for review.
> 
> On Thu, 2020-06-25 at 17:27 +0200, Michał Winiarski wrote:
> > Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> > > The purpose of debug messages displayed by the test is to make
> > > identification of a subtest phase that fails more easy.  Since issues
> > > exhibited by the test are mostly reported to dmesg, print those debug
> > > messages to /dev/kmsg as well.
> > 
> > I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this logging
> > to the kernel, 
> The idea was to simply log IGT actions, not to log kernel reactions on
> them which already happens.  Doing that from the kernel would probably
> require modifications to PCI sysfs handling or to sysfs in general.
> 
> If you see no benefits from that, let's drop this patch.

We (me & Petri) were thinking about doing something similar, but only
for the places where kernel logs are hard to correlate with the test
actions, to have those "synchronization points" between logs for key
operations.

The main reason was Chamelium - external board that simulates display
and can cause hotplug events to happen. Logging Chamelium operations in
dmesg would make any kind of bug assessment or troubleshooting much
easier - was that a phantom hotplug? or something that was triggered by
us? Have we started doing anything else before the link has settled?
What happened during DP FSM handling?

This is a very special case as we deal with an external board that
drives our HW through wires and layers of firmware and for sure there be
dragons.


Anyway, I would like to see us having a way of logging those "sync
points" into kmesg in igt_core, e.g. by creating _kmesg suffixed version
of log functions.

But I also see Michał's point - too frivolous logging just creates
noise, and we shouldn't double log - if something is already easy to
find in the kernel logs and the correlating test action to logs is not
too hard why should we add more noise?

As of the examples in this thread - I am not very familiar with the area
and I leave it up to you two to decide what would be helpful, what would
be unnecessary repetition and what would be better off logged in the
kernel.

TL;DR: Yes for logging things into kmesg, but we should be careful about
       what we log to not make the situation any worse.

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

[Intel-gfx] [RFC PATCH i-g-t v2 2/8] tests/core_hotunplug: Use PCI device sysfs entry, not DRM

[Janusz Krzysztofik]

Future subtests may want to access PCI attributes of the device after
driver unbind.  Refactor prepare() helper.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 826645b1f..35eba9b8a 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -55,42 +55,54 @@ struct hotunplug {
 	igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
 })
 
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static inline int prepare_common(struct hotunplug *priv)
 {
-	int len;
+	int fd_sysfs_drm;
+
+	local_debug("opening device");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert(priv->fd.drm >= 0);
+
+	fd_sysfs_drm = igt_sysfs_open(priv->fd.drm);
+	igt_assert(fd_sysfs_drm >= 0);
+
+	return fd_sysfs_drm;
+}
+
+static inline void prepare_for_rebind(struct hotunplug *priv,
+				      char *buf, int buflen)
+{
+	int fd_sysfs_drm, len;
 
 	igt_assert(buflen);
 
-	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
-				    O_DIRECTORY);
-	igt_assert(priv->fd.sysfs_drv >= 0);
+	fd_sysfs_drm = prepare_common(priv);
+
+	priv->fd.sysfs_drv = openat(fd_sysfs_drm, "device/driver", O_DIRECTORY);
 
-	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
+	len = readlinkat(fd_sysfs_drm, "device", buf, buflen - 1);
 	buf[len] = '\0';
 	priv->dev_bus_addr = strrchr(buf, '/');
-	igt_assert(priv->dev_bus_addr++);
 
-	/* sysfs_dev no longer needed */
-	close(priv->fd.sysfs_dev);
+	close(fd_sysfs_drm);
+
+	igt_assert(priv->fd.sysfs_drv >= 0);
+	igt_assert(priv->dev_bus_addr++);
 }
 
-static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
+static inline void prepare_for_rescan(struct hotunplug *priv)
 {
-	local_debug("opening device");
-	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert(priv->fd.drm >= 0);
+	int fd_sysfs_drm = prepare_common(priv);
 
-	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert(priv->fd.sysfs_dev >= 0);
+	priv->fd.sysfs_dev = openat(fd_sysfs_drm, "device", O_DIRECTORY);
 
-	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(priv->fd.sysfs_bus >= 0);
-	}
+	priv->fd.sysfs_bus = openat(fd_sysfs_drm, "device/subsystem",
+				    O_DIRECTORY);
+
+	close(fd_sysfs_drm);
+
+	igt_assert(priv->fd.sysfs_dev >= 0);
+	igt_assert(priv->fd.sysfs_bus >= 0);
 }
 
 static const char *failure;
@@ -124,7 +136,7 @@ static void device_unplug(int fd_sysfs_dev)
 {
 	failure = "Device unplug timeout!";
 	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
+	igt_sysfs_set(fd_sysfs_dev, "remove", "1");
 	igt_reset_timeout();
 	failure = NULL;
 
@@ -185,7 +197,7 @@ static void unbind_rebind(void)
 	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare_for_rebind(&priv, buf, sizeof(buf));
 
 	local_debug("closing the device");
 	close(priv.fd.drm);
@@ -203,7 +215,7 @@ static void unplug_rescan(void)
 {
 	struct hotunplug priv;
 
-	prepare(&priv, NULL, 0);
+	prepare_for_rescan(&priv);
 
 	local_debug("closing the device");
 	close(priv.fd.drm);
@@ -222,7 +234,7 @@ static void hotunbind_lateclose(void)
 	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare_for_rebind(&priv, buf, sizeof(buf));
 
 	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
@@ -240,7 +252,7 @@ static void hotunplug_lateclose(void)
 {
 	struct hotunplug priv;
 
-	prepare(&priv, NULL, 0);
+	prepare_for_rescan(&priv);
 
 	local_debug("hot unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
-- 
2.21.1

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

[igt-dev] [RFC PATCH i-g-t v2 2/8] tests/core_hotunplug: Use PCI device sysfs entry, not DRM

[Janusz Krzysztofik]

Future subtests may want to access PCI attributes of the device after
driver unbind.  Refactor prepare() helper.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 826645b1f..35eba9b8a 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -55,42 +55,54 @@ struct hotunplug {
 	igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
 })
 
-static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
+static inline int prepare_common(struct hotunplug *priv)
 {
-	int len;
+	int fd_sysfs_drm;
+
+	local_debug("opening device");
+	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
+	igt_assert(priv->fd.drm >= 0);
+
+	fd_sysfs_drm = igt_sysfs_open(priv->fd.drm);
+	igt_assert(fd_sysfs_drm >= 0);
+
+	return fd_sysfs_drm;
+}
+
+static inline void prepare_for_rebind(struct hotunplug *priv,
+				      char *buf, int buflen)
+{
+	int fd_sysfs_drm, len;
 
 	igt_assert(buflen);
 
-	priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
-				    O_DIRECTORY);
-	igt_assert(priv->fd.sysfs_drv >= 0);
+	fd_sysfs_drm = prepare_common(priv);
+
+	priv->fd.sysfs_drv = openat(fd_sysfs_drm, "device/driver", O_DIRECTORY);
 
-	len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
+	len = readlinkat(fd_sysfs_drm, "device", buf, buflen - 1);
 	buf[len] = '\0';
 	priv->dev_bus_addr = strrchr(buf, '/');
-	igt_assert(priv->dev_bus_addr++);
 
-	/* sysfs_dev no longer needed */
-	close(priv->fd.sysfs_dev);
+	close(fd_sysfs_drm);
+
+	igt_assert(priv->fd.sysfs_drv >= 0);
+	igt_assert(priv->dev_bus_addr++);
 }
 
-static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
+static inline void prepare_for_rescan(struct hotunplug *priv)
 {
-	local_debug("opening device");
-	priv->fd.drm = __drm_open_driver(DRIVER_ANY);
-	igt_assert(priv->fd.drm >= 0);
+	int fd_sysfs_drm = prepare_common(priv);
 
-	priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
-	igt_assert(priv->fd.sysfs_dev >= 0);
+	priv->fd.sysfs_dev = openat(fd_sysfs_drm, "device", O_DIRECTORY);
 
-	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(priv->fd.sysfs_bus >= 0);
-	}
+	priv->fd.sysfs_bus = openat(fd_sysfs_drm, "device/subsystem",
+				    O_DIRECTORY);
+
+	close(fd_sysfs_drm);
+
+	igt_assert(priv->fd.sysfs_dev >= 0);
+	igt_assert(priv->fd.sysfs_bus >= 0);
 }
 
 static const char *failure;
@@ -124,7 +136,7 @@ static void device_unplug(int fd_sysfs_dev)
 {
 	failure = "Device unplug timeout!";
 	igt_set_timeout(60, failure);
-	igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
+	igt_sysfs_set(fd_sysfs_dev, "remove", "1");
 	igt_reset_timeout();
 	failure = NULL;
 
@@ -185,7 +197,7 @@ static void unbind_rebind(void)
 	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare_for_rebind(&priv, buf, sizeof(buf));
 
 	local_debug("closing the device");
 	close(priv.fd.drm);
@@ -203,7 +215,7 @@ static void unplug_rescan(void)
 {
 	struct hotunplug priv;
 
-	prepare(&priv, NULL, 0);
+	prepare_for_rescan(&priv);
 
 	local_debug("closing the device");
 	close(priv.fd.drm);
@@ -222,7 +234,7 @@ static void hotunbind_lateclose(void)
 	struct hotunplug priv;
 	char buf[PATH_MAX];
 
-	prepare(&priv, buf, sizeof(buf));
+	prepare_for_rebind(&priv, buf, sizeof(buf));
 
 	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
@@ -240,7 +252,7 @@ static void hotunplug_lateclose(void)
 {
 	struct hotunplug priv;
 
-	prepare(&priv, NULL, 0);
+	prepare_for_rescan(&priv);
 
 	local_debug("hot unplugging the device");
 	device_unplug(priv.fd.sysfs_dev);
-- 
2.21.1

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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 2/8] tests/core_hotunplug: Use PCI device sysfs entry, not DRM

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:09)
> Future subtests may want to access PCI attributes of the device after
> driver unbind.  Refactor prepare() helper.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 68 +++++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 826645b1f..35eba9b8a 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -55,42 +55,54 @@ struct hotunplug {
>         igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
>  })
>  
> -static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> +static inline int prepare_common(struct hotunplug *priv)
>  {
> -       int len;
> +       int fd_sysfs_drm;
> +
> +       local_debug("opening device");
> +       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> +       igt_assert(priv->fd.drm >= 0);
> +
> +       fd_sysfs_drm = igt_sysfs_open(priv->fd.drm);
> +       igt_assert(fd_sysfs_drm >= 0);
> +
> +       return fd_sysfs_drm;
> +}
> +
> +static inline void prepare_for_rebind(struct hotunplug *priv,
> +                                     char *buf, int buflen)
> +{
> +       int fd_sysfs_drm, len;
>  
>         igt_assert(buflen);
>  
> -       priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
> -                                   O_DIRECTORY);
> -       igt_assert(priv->fd.sysfs_drv >= 0);
> +       fd_sysfs_drm = prepare_common(priv);
> +
> +       priv->fd.sysfs_drv = openat(fd_sysfs_drm, "device/driver", O_DIRECTORY);
>  
> -       len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
> +       len = readlinkat(fd_sysfs_drm, "device", buf, buflen - 1);
>         buf[len] = '\0';
>         priv->dev_bus_addr = strrchr(buf, '/');
> -       igt_assert(priv->dev_bus_addr++);
>  
> -       /* sysfs_dev no longer needed */
> -       close(priv->fd.sysfs_dev);
> +       close(fd_sysfs_drm);
> +
> +       igt_assert(priv->fd.sysfs_drv >= 0);
> +       igt_assert(priv->dev_bus_addr++);
>  }
>  
> -static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
> +static inline void prepare_for_rescan(struct hotunplug *priv)
>  {
> -       local_debug("opening device");
> -       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> -       igt_assert(priv->fd.drm >= 0);
> +       int fd_sysfs_drm = prepare_common(priv);
>  
> -       priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
> -       igt_assert(priv->fd.sysfs_dev >= 0);
> +       priv->fd.sysfs_dev = openat(fd_sysfs_drm, "device", O_DIRECTORY);
>  
> -       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(priv->fd.sysfs_bus >= 0);
> -       }
> +       priv->fd.sysfs_bus = openat(fd_sysfs_drm, "device/subsystem",
> +                                   O_DIRECTORY);
> +
> +       close(fd_sysfs_drm);
> +
> +       igt_assert(priv->fd.sysfs_dev >= 0);
> +       igt_assert(priv->fd.sysfs_bus >= 0);
>  }

I find the lifecycle of hotunplug.fd.sysfs_* difficult to follow now...
Would it be possible to keep the "prepare" step simpler and just open everything
everytime? (perhaps closing and opening new ones when called multiple times?)
Or do we need to have drv separate from bus/dev?

-Michał

>  
>  static const char *failure;
> @@ -124,7 +136,7 @@ static void device_unplug(int fd_sysfs_dev)
>  {
>         failure = "Device unplug timeout!";
>         igt_set_timeout(60, failure);
> -       igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
> +       igt_sysfs_set(fd_sysfs_dev, "remove", "1");
>         igt_reset_timeout();
>         failure = NULL;
>  
> @@ -185,7 +197,7 @@ static void unbind_rebind(void)
>         struct hotunplug priv;
>         char buf[PATH_MAX];
>  
> -       prepare(&priv, buf, sizeof(buf));
> +       prepare_for_rebind(&priv, buf, sizeof(buf));
>  
>         local_debug("closing the device");
>         close(priv.fd.drm);
> @@ -203,7 +215,7 @@ static void unplug_rescan(void)
>  {
>         struct hotunplug priv;
>  
> -       prepare(&priv, NULL, 0);
> +       prepare_for_rescan(&priv);
>  
>         local_debug("closing the device");
>         close(priv.fd.drm);
> @@ -222,7 +234,7 @@ static void hotunbind_lateclose(void)
>         struct hotunplug priv;
>         char buf[PATH_MAX];
>  
> -       prepare(&priv, buf, sizeof(buf));
> +       prepare_for_rebind(&priv, buf, sizeof(buf));
>  
>         local_debug("hot unbinding the driver from the device");
>         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> @@ -240,7 +252,7 @@ static void hotunplug_lateclose(void)
>  {
>         struct hotunplug priv;
>  
> -       prepare(&priv, NULL, 0);
> +       prepare_for_rescan(&priv);
>  
>         local_debug("hot unplugging the device");
>         device_unplug(priv.fd.sysfs_dev);
> -- 
> 2.21.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 2/8] tests/core_hotunplug: Use PCI device sysfs entry, not DRM

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 21:23 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:09)
> > Future subtests may want to access PCI attributes of the device after
> > driver unbind.  Refactor prepare() helper.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 68 +++++++++++++++++++++++++-----------------
> >  1 file changed, 40 insertions(+), 28 deletions(-)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 826645b1f..35eba9b8a 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -55,42 +55,54 @@ struct hotunplug {
> >         igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
> >  })
> >  
> > -static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> > +static inline int prepare_common(struct hotunplug *priv)
> >  {
> > -       int len;
> > +       int fd_sysfs_drm;
> > +
> > +       local_debug("opening device");
> > +       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> > +       igt_assert(priv->fd.drm >= 0);
> > +
> > +       fd_sysfs_drm = igt_sysfs_open(priv->fd.drm);
> > +       igt_assert(fd_sysfs_drm >= 0);
> > +
> > +       return fd_sysfs_drm;
> > +}
> > +
> > +static inline void prepare_for_rebind(struct hotunplug *priv,
> > +                                     char *buf, int buflen)
> > +{
> > +       int fd_sysfs_drm, len;
> >  
> >         igt_assert(buflen);
> >  
> > -       priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
> > -                                   O_DIRECTORY);
> > -       igt_assert(priv->fd.sysfs_drv >= 0);
> > +       fd_sysfs_drm = prepare_common(priv);
> > +
> > +       priv->fd.sysfs_drv = openat(fd_sysfs_drm, "device/driver", O_DIRECTORY);
> >  
> > -       len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
> > +       len = readlinkat(fd_sysfs_drm, "device", buf, buflen - 1);
> >         buf[len] = '\0';
> >         priv->dev_bus_addr = strrchr(buf, '/');
> > -       igt_assert(priv->dev_bus_addr++);
> >  
> > -       /* sysfs_dev no longer needed */
> > -       close(priv->fd.sysfs_dev);
> > +       close(fd_sysfs_drm);
> > +
> > +       igt_assert(priv->fd.sysfs_drv >= 0);
> > +       igt_assert(priv->dev_bus_addr++);
> >  }
> >  
> > -static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
> > +static inline void prepare_for_rescan(struct hotunplug *priv)
> >  {
> > -       local_debug("opening device");
> > -       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> > -       igt_assert(priv->fd.drm >= 0);
> > +       int fd_sysfs_drm = prepare_common(priv);
> >  
> > -       priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
> > -       igt_assert(priv->fd.sysfs_dev >= 0);
> > +       priv->fd.sysfs_dev = openat(fd_sysfs_drm, "device", O_DIRECTORY);
> >  
> > -       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(priv->fd.sysfs_bus >= 0);
> > -       }
> > +       priv->fd.sysfs_bus = openat(fd_sysfs_drm, "device/subsystem",
> > +                                   O_DIRECTORY);
> > +
> > +       close(fd_sysfs_drm);
> > +
> > +       igt_assert(priv->fd.sysfs_dev >= 0);
> > +       igt_assert(priv->fd.sysfs_bus >= 0);
> >  }
> 
> I find the lifecycle of hotunplug.fd.sysfs_* difficult to follow now...
> Would it be possible to keep the "prepare" step simpler and just open everything
> everytime? (perhaps closing and opening new ones when called multiple times?)

OK.

Thanks,
Janusz

> Or do we need to have drv separate from bus/dev?
> 
> -Michał
> 
> >  
> >  static const char *failure;
> > @@ -124,7 +136,7 @@ static void device_unplug(int fd_sysfs_dev)
> >  {
> >         failure = "Device unplug timeout!";
> >         igt_set_timeout(60, failure);
> > -       igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
> > +       igt_sysfs_set(fd_sysfs_dev, "remove", "1");
> >         igt_reset_timeout();
> >         failure = NULL;
> >  
> > @@ -185,7 +197,7 @@ static void unbind_rebind(void)
> >         struct hotunplug priv;
> >         char buf[PATH_MAX];
> >  
> > -       prepare(&priv, buf, sizeof(buf));
> > +       prepare_for_rebind(&priv, buf, sizeof(buf));
> >  
> >         local_debug("closing the device");
> >         close(priv.fd.drm);
> > @@ -203,7 +215,7 @@ static void unplug_rescan(void)
> >  {
> >         struct hotunplug priv;
> >  
> > -       prepare(&priv, NULL, 0);
> > +       prepare_for_rescan(&priv);
> >  
> >         local_debug("closing the device");
> >         close(priv.fd.drm);
> > @@ -222,7 +234,7 @@ static void hotunbind_lateclose(void)
> >         struct hotunplug priv;
> >         char buf[PATH_MAX];
> >  
> > -       prepare(&priv, buf, sizeof(buf));
> > +       prepare_for_rebind(&priv, buf, sizeof(buf));
> >  
> >         local_debug("hot unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> > @@ -240,7 +252,7 @@ static void hotunplug_lateclose(void)
> >  {
> >         struct hotunplug priv;
> >  
> > -       prepare(&priv, NULL, 0);
> > +       prepare_for_rescan(&priv);
> >  
> >         local_debug("hot unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> > -- 
> > 2.21.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [RFC PATCH i-g-t v2 2/8] tests/core_hotunplug: Use PCI device sysfs entry, not DRM

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 21:23 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:09)
> > Future subtests may want to access PCI attributes of the device after
> > driver unbind.  Refactor prepare() helper.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 68 +++++++++++++++++++++++++-----------------
> >  1 file changed, 40 insertions(+), 28 deletions(-)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 826645b1f..35eba9b8a 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -55,42 +55,54 @@ struct hotunplug {
> >         igt_kmsg(KMSG_DEBUG "%s: %s: %s\n", igt_test_name(), __func__, msg); \
> >  })
> >  
> > -static void prepare_for_unbind(struct hotunplug *priv, char *buf, int buflen)
> > +static inline int prepare_common(struct hotunplug *priv)
> >  {
> > -       int len;
> > +       int fd_sysfs_drm;
> > +
> > +       local_debug("opening device");
> > +       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> > +       igt_assert(priv->fd.drm >= 0);
> > +
> > +       fd_sysfs_drm = igt_sysfs_open(priv->fd.drm);
> > +       igt_assert(fd_sysfs_drm >= 0);
> > +
> > +       return fd_sysfs_drm;
> > +}
> > +
> > +static inline void prepare_for_rebind(struct hotunplug *priv,
> > +                                     char *buf, int buflen)
> > +{
> > +       int fd_sysfs_drm, len;
> >  
> >         igt_assert(buflen);
> >  
> > -       priv->fd.sysfs_drv = openat(priv->fd.sysfs_dev, "device/driver",
> > -                                   O_DIRECTORY);
> > -       igt_assert(priv->fd.sysfs_drv >= 0);
> > +       fd_sysfs_drm = prepare_common(priv);
> > +
> > +       priv->fd.sysfs_drv = openat(fd_sysfs_drm, "device/driver", O_DIRECTORY);
> >  
> > -       len = readlinkat(priv->fd.sysfs_dev, "device", buf, buflen - 1);
> > +       len = readlinkat(fd_sysfs_drm, "device", buf, buflen - 1);
> >         buf[len] = '\0';
> >         priv->dev_bus_addr = strrchr(buf, '/');
> > -       igt_assert(priv->dev_bus_addr++);
> >  
> > -       /* sysfs_dev no longer needed */
> > -       close(priv->fd.sysfs_dev);
> > +       close(fd_sysfs_drm);
> > +
> > +       igt_assert(priv->fd.sysfs_drv >= 0);
> > +       igt_assert(priv->dev_bus_addr++);
> >  }
> >  
> > -static inline void prepare(struct hotunplug *priv, char *buf, int buflen)
> > +static inline void prepare_for_rescan(struct hotunplug *priv)
> >  {
> > -       local_debug("opening device");
> > -       priv->fd.drm = __drm_open_driver(DRIVER_ANY);
> > -       igt_assert(priv->fd.drm >= 0);
> > +       int fd_sysfs_drm = prepare_common(priv);
> >  
> > -       priv->fd.sysfs_dev = igt_sysfs_open(priv->fd.drm);
> > -       igt_assert(priv->fd.sysfs_dev >= 0);
> > +       priv->fd.sysfs_dev = openat(fd_sysfs_drm, "device", O_DIRECTORY);
> >  
> > -       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(priv->fd.sysfs_bus >= 0);
> > -       }
> > +       priv->fd.sysfs_bus = openat(fd_sysfs_drm, "device/subsystem",
> > +                                   O_DIRECTORY);
> > +
> > +       close(fd_sysfs_drm);
> > +
> > +       igt_assert(priv->fd.sysfs_dev >= 0);
> > +       igt_assert(priv->fd.sysfs_bus >= 0);
> >  }
> 
> I find the lifecycle of hotunplug.fd.sysfs_* difficult to follow now...
> Would it be possible to keep the "prepare" step simpler and just open everything
> everytime? (perhaps closing and opening new ones when called multiple times?)

OK.

Thanks,
Janusz

> Or do we need to have drv separate from bus/dev?
> 
> -Michał
> 
> >  
> >  static const char *failure;
> > @@ -124,7 +136,7 @@ static void device_unplug(int fd_sysfs_dev)
> >  {
> >         failure = "Device unplug timeout!";
> >         igt_set_timeout(60, failure);
> > -       igt_sysfs_set(fd_sysfs_dev, "device/remove", "1");
> > +       igt_sysfs_set(fd_sysfs_dev, "remove", "1");
> >         igt_reset_timeout();
> >         failure = NULL;
> >  
> > @@ -185,7 +197,7 @@ static void unbind_rebind(void)
> >         struct hotunplug priv;
> >         char buf[PATH_MAX];
> >  
> > -       prepare(&priv, buf, sizeof(buf));
> > +       prepare_for_rebind(&priv, buf, sizeof(buf));
> >  
> >         local_debug("closing the device");
> >         close(priv.fd.drm);
> > @@ -203,7 +215,7 @@ static void unplug_rescan(void)
> >  {
> >         struct hotunplug priv;
> >  
> > -       prepare(&priv, NULL, 0);
> > +       prepare_for_rescan(&priv);
> >  
> >         local_debug("closing the device");
> >         close(priv.fd.drm);
> > @@ -222,7 +234,7 @@ static void hotunbind_lateclose(void)
> >         struct hotunplug priv;
> >         char buf[PATH_MAX];
> >  
> > -       prepare(&priv, buf, sizeof(buf));
> > +       prepare_for_rebind(&priv, buf, sizeof(buf));
> >  
> >         local_debug("hot unbinding the driver from the device");
> >         driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
> > @@ -240,7 +252,7 @@ static void hotunplug_lateclose(void)
> >  {
> >         struct hotunplug priv;
> >  
> > -       prepare(&priv, NULL, 0);
> > +       prepare_for_rescan(&priv);
> >  
> >         local_debug("hot unplugging the device");
> >         device_unplug(priv.fd.sysfs_dev);
> > -- 
> > 2.21.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Intel-gfx] [RFC PATCH i-g-t v2 3/8] tests/core_hotunplug: Add unbind-unplug-rescan variant

[Janusz Krzysztofik]

Check if this 3-step procedure exhibits any issues with device recover
after unplug.  Such issues may indicate insufficient device hardware
re-initialization performed by the device driver, or other kernel bugs
outside the driver code.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 35eba9b8a..a4809720b 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -211,6 +211,35 @@ static void unbind_rebind(void)
 	healthcheck();
 }
 
+static void unbind_unplug_rescan(void)
+{
+	struct hotunplug priv;
+	char buf[PATH_MAX];
+
+	/* prepare for unbind */
+	prepare_for_rebind(&priv, buf, sizeof(buf));
+
+	/* also prepare for unplug */
+	local_debug("closing the device");
+	close(priv.fd.drm);
+	prepare_for_rescan(&priv);
+
+	local_debug("closing the device");
+	close(priv.fd.drm);
+
+	local_debug("unbinding the driver from the device");
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	close(priv.fd.sysfs_drv);
+
+	local_debug("unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 static void unplug_rescan(void)
 {
 	struct hotunplug priv;
@@ -290,14 +319,21 @@ igt_main
 		close(fd_drm);
 	}
 
-	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
+	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed, then rebound");
 	igt_subtest("unbind-rebind")
 		unbind_rebind();
 
 	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_describe("Check if a device with the driver unbound from it can be cleanly recovered after being unplugged\n");
+	igt_subtest("unbind-unplug-rescan")
+		unbind_unplug_rescan();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device believed to be closed can be cleanly unplugged and recovered");
 	igt_subtest("unplug-rescan")
 		unplug_rescan();
 
-- 
2.21.1

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

[igt-dev] [RFC PATCH i-g-t v2 3/8] tests/core_hotunplug: Add unbind-unplug-rescan variant

[Janusz Krzysztofik]

Check if this 3-step procedure exhibits any issues with device recover
after unplug.  Such issues may indicate insufficient device hardware
re-initialization performed by the device driver, or other kernel bugs
outside the driver code.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 35eba9b8a..a4809720b 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -211,6 +211,35 @@ static void unbind_rebind(void)
 	healthcheck();
 }
 
+static void unbind_unplug_rescan(void)
+{
+	struct hotunplug priv;
+	char buf[PATH_MAX];
+
+	/* prepare for unbind */
+	prepare_for_rebind(&priv, buf, sizeof(buf));
+
+	/* also prepare for unplug */
+	local_debug("closing the device");
+	close(priv.fd.drm);
+	prepare_for_rescan(&priv);
+
+	local_debug("closing the device");
+	close(priv.fd.drm);
+
+	local_debug("unbinding the driver from the device");
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+	close(priv.fd.sysfs_drv);
+
+	local_debug("unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 static void unplug_rescan(void)
 {
 	struct hotunplug priv;
@@ -290,14 +319,21 @@ igt_main
 		close(fd_drm);
 	}
 
-	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed");
+	igt_describe("Check if the driver can be cleanly unbound from a device believed to be closed, then rebound");
 	igt_subtest("unbind-rebind")
 		unbind_rebind();
 
 	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_describe("Check if a device with the driver unbound from it can be cleanly recovered after being unplugged\n");
+	igt_subtest("unbind-unplug-rescan")
+		unbind_unplug_rescan();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device believed to be closed can be cleanly unplugged and recovered");
 	igt_subtest("unplug-rescan")
 		unplug_rescan();
 
-- 
2.21.1

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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 3/8] tests/core_hotunplug: Add unbind-unplug-rescan variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:10)
> Check if this 3-step procedure exhibits any issues with device recover
> after unplug.  Such issues may indicate insufficient device hardware
> re-initialization performed by the device driver, or other kernel bugs
> outside the driver code.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

After addressing comments from preceding patches:

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

-Michał

> ---
>  tests/core_hotunplug.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [RFC PATCH i-g-t v2 3/8] tests/core_hotunplug: Add unbind-unplug-rescan variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:10)
> Check if this 3-step procedure exhibits any issues with device recover
> after unplug.  Such issues may indicate insufficient device hardware
> re-initialization performed by the device driver, or other kernel bugs
> outside the driver code.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

After addressing comments from preceding patches:

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

-Michał

> ---
>  tests/core_hotunplug.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[Intel-gfx] [RFC PATCH i-g-t v2 4/8] tests/core_hotunplug: Add 'lateclose before recover' variants

[Janusz Krzysztofik]

If a GPU gets wedged during driver rebind or device re-plug for some
reason, current hotunbind/hotunplug test variants may time out before
lateclose phase, resulting in incomplete CI reports.  Let's rename
those variants to more adequate hotrebind/hotreplug-lateclose and add
new variants focused on exercising the lateclose phase regardless of
potential rebind/re-plug issues under old names.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index a4809720b..0892e1927 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -268,6 +268,43 @@ static void hotunbind_lateclose(void)
 	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
+	local_debug("late closing the unbound device instance");
+	close(priv.fd.drm);
+
+	local_debug("rebinding the driver to the device");
+	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+
+	healthcheck();
+}
+
+static void hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
+static void hotrebind_lateclose(void)
+{
+	struct hotunplug priv;
+	char buf[PATH_MAX];
+
+	prepare_for_rebind(&priv, buf, sizeof(buf));
+
+	local_debug("hot unbinding the driver from the device");
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+
 	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
@@ -277,7 +314,7 @@ static void hotunbind_lateclose(void)
 	healthcheck();
 }
 
-static void hotunplug_lateclose(void)
+static void hotreplug_lateclose(void)
 {
 	struct hotunplug priv;
 
@@ -340,17 +377,31 @@ igt_main
 	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_describe("Check if the driver can be cleanly unbound from a still open device, then released and rebound");
 	igt_subtest("hotunbind-lateclose")
 		hotunbind_lateclose();
 
 	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_describe("Check if a still open device can be cleanly unplugged, then released and recovered");
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose();
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if the driver can be cleanly unbound from an open device and rebound back, then released");
+	igt_subtest("hotrebind-lateclose")
+		hotrebind_lateclose();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a still open device can be cleanly unplugged and recovered, then released");
+	igt_subtest("hotreplug-lateclose")
+		hotreplug_lateclose();
+
+	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

[igt-dev] [RFC PATCH i-g-t v2 4/8] tests/core_hotunplug: Add 'lateclose before recover' variants

[Janusz Krzysztofik]

If a GPU gets wedged during driver rebind or device re-plug for some
reason, current hotunbind/hotunplug test variants may time out before
lateclose phase, resulting in incomplete CI reports.  Let's rename
those variants to more adequate hotrebind/hotreplug-lateclose and add
new variants focused on exercising the lateclose phase regardless of
potential rebind/re-plug issues under old names.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index a4809720b..0892e1927 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -268,6 +268,43 @@ static void hotunbind_lateclose(void)
 	local_debug("hot unbinding the driver from the device");
 	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
+	local_debug("late closing the unbound device instance");
+	close(priv.fd.drm);
+
+	local_debug("rebinding the driver to the device");
+	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+
+	healthcheck();
+}
+
+static void hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
+static void hotrebind_lateclose(void)
+{
+	struct hotunplug priv;
+	char buf[PATH_MAX];
+
+	prepare_for_rebind(&priv, buf, sizeof(buf));
+
+	local_debug("hot unbinding the driver from the device");
+	driver_unbind(priv.fd.sysfs_drv, priv.dev_bus_addr);
+
 	local_debug("rebinding the driver to the device");
 	driver_bind(priv.fd.sysfs_drv, priv.dev_bus_addr);
 
@@ -277,7 +314,7 @@ static void hotunbind_lateclose(void)
 	healthcheck();
 }
 
-static void hotunplug_lateclose(void)
+static void hotreplug_lateclose(void)
 {
 	struct hotunplug priv;
 
@@ -340,17 +377,31 @@ igt_main
 	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_describe("Check if the driver can be cleanly unbound from a still open device, then released and rebound");
 	igt_subtest("hotunbind-lateclose")
 		hotunbind_lateclose();
 
 	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_describe("Check if a still open device can be cleanly unplugged, then released and recovered");
 	igt_subtest("hotunplug-lateclose")
 		hotunplug_lateclose();
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if the driver can be cleanly unbound from an open device and rebound back, then released");
+	igt_subtest("hotrebind-lateclose")
+		hotrebind_lateclose();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a still open device can be cleanly unplugged and recovered, then released");
+	igt_subtest("hotreplug-lateclose")
+		hotreplug_lateclose();
+
+	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

Re: [Intel-gfx] [RFC PATCH i-g-t v2 4/8] tests/core_hotunplug: Add 'lateclose before recover' variants

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:11)
> If a GPU gets wedged during driver rebind or device re-plug for some
> reason, current hotunbind/hotunplug test variants may time out before
> lateclose phase, resulting in incomplete CI reports.  Let's rename
> those variants to more adequate hotrebind/hotreplug-lateclose and add
> new variants focused on exercising the lateclose phase regardless of
> potential rebind/re-plug issues under old names.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

After addressing comments from preceding patches:

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

-Michał

> ---
>  tests/core_hotunplug.c | 57 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 3 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 4/8] tests/core_hotunplug: Add 'lateclose before recover' variants

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:11)
> If a GPU gets wedged during driver rebind or device re-plug for some
> reason, current hotunbind/hotunplug test variants may time out before
> lateclose phase, resulting in incomplete CI reports.  Let's rename
> those variants to more adequate hotrebind/hotreplug-lateclose and add
> new variants focused on exercising the lateclose phase regardless of
> potential rebind/re-plug issues under old names.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

After addressing comments from preceding patches:

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

-Michał

> ---
>  tests/core_hotunplug.c | 57 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 3 deletions(-)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[Intel-gfx] [RFC PATCH i-g-t v2 5/8] tests/core_hotunplug: Add 'GEM address space' variant

[Janusz Krzysztofik]

Verify if an additional address space associated with an open device
file descriptor is cleaned up correctly on device hotunplug.

v2: rebase on upstream, update includes order

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 0892e1927..18a963564 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -30,6 +30,7 @@
 #include <unistd.h>
 
 #include "i915/gem.h"
+#include "i915/gem_vm.h"
 #include "igt.h"
 #include "igt_device_scan.h"
 #include "igt_kmod.h"
@@ -332,6 +333,29 @@ static void hotreplug_lateclose(void)
 	healthcheck();
 }
 
+static void vm_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	gem_require_vm(priv.fd.drm);
+
+	local_debug("creating additional GEM user address space");
+	igt_ignore_warn(gem_vm_create(priv.fd.drm));
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -404,4 +428,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a still open device with extra GEM address space can be cleanly unplugged, then released and recovered");
+	igt_subtest("vm-hotunplug-lateclose")
+		vm_hotunplug_lateclose();
+
+	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

[igt-dev] [RFC PATCH i-g-t v2 5/8] tests/core_hotunplug: Add 'GEM address space' variant

[Janusz Krzysztofik]

Verify if an additional address space associated with an open device
file descriptor is cleaned up correctly on device hotunplug.

v2: rebase on upstream, update includes order

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 0892e1927..18a963564 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -30,6 +30,7 @@
 #include <unistd.h>
 
 #include "i915/gem.h"
+#include "i915/gem_vm.h"
 #include "igt.h"
 #include "igt_device_scan.h"
 #include "igt_kmod.h"
@@ -332,6 +333,29 @@ static void hotreplug_lateclose(void)
 	healthcheck();
 }
 
+static void vm_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	gem_require_vm(priv.fd.drm);
+
+	local_debug("creating additional GEM user address space");
+	igt_ignore_warn(gem_vm_create(priv.fd.drm));
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -404,4 +428,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a still open device with extra GEM address space can be cleanly unplugged, then released and recovered");
+	igt_subtest("vm-hotunplug-lateclose")
+		vm_hotunplug_lateclose();
+
+	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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 5/8] tests/core_hotunplug: Add 'GEM address space' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:12)
> Verify if an additional address space associated with an open device
> file descriptor is cleaned up correctly on device hotunplug.
> 
> v2: rebase on upstream, update includes order
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 0892e1927..18a963564 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -30,6 +30,7 @@
>  #include <unistd.h>
>  
>  #include "i915/gem.h"
> +#include "i915/gem_vm.h"
>  #include "igt.h"
>  #include "igt_device_scan.h"
>  #include "igt_kmod.h"
> @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void vm_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +
> +       prepare_for_rescan(&priv);
> +
> +       gem_require_vm(priv.fd.drm);
> +
> +       local_debug("creating additional GEM user address space");
> +       igt_ignore_warn(gem_vm_create(priv.fd.drm));

Why the "ignore_warn"?
This deserves a comment. And perhaps a word of information about why we picked
this partucular operation in the commit message (vm_create).
This is a regression test, right?

LGTM otherwise (but again - see previous patches):

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

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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 5/8] tests/core_hotunplug: Add 'GEM address space' variant

[Janusz Krzysztofik]

Hi Michał,

thanks for review.

On Thu, 2020-06-25 at 21:42 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:12)
> > Verify if an additional address space associated with an open device
> > file descriptor is cleaned up correctly on device hotunplug.
> > 
> > v2: rebase on upstream, update includes order
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 0892e1927..18a963564 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -30,6 +30,7 @@
> >  #include <unistd.h>
> >  
> >  #include "i915/gem.h"
> > +#include "i915/gem_vm.h"
> >  #include "igt.h"
> >  #include "igt_device_scan.h"
> >  #include "igt_kmod.h"
> > @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void vm_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       gem_require_vm(priv.fd.drm);
> > +
> > +       local_debug("creating additional GEM user address space");
> > +       igt_ignore_warn(gem_vm_create(priv.fd.drm));
> 
> Why the "ignore_warn"?  This deserves a comment. 
> And perhaps a word of information about why we picked
> this partucular operation in the commit message (vm_create).
> This is a regression test, right?

Hmm, I didn't intend it to be a regression test.  The purpose was
generally the same as of other hotunplug-lateclose subtests - exercise
the driver behaviour on hotunplug and lateclose.  This subtest was
intended to perform still the same exercise, only under different
conditions, or different use case of the driver.  In particular,
hotunplug is performed here with an additional address space associated
with an open file descriptor.  We are not interested in exercising that
address space (that's why igt_ignore_warn), only in checking if it is
cleaned up on time so hotunplug-lateclose operations are still safe
from late dma_unmap issues.

Let me try to reword the commit description so it better reflects this
idea.

Thanks,
Janusz

> 
> LGTM otherwise (but again - see previous patches):
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał

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

Re: [igt-dev] [RFC PATCH i-g-t v2 5/8] tests/core_hotunplug: Add 'GEM address space' variant

[Janusz Krzysztofik]

Hi Michał,

thanks for review.

On Thu, 2020-06-25 at 21:42 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:12)
> > Verify if an additional address space associated with an open device
> > file descriptor is cleaned up correctly on device hotunplug.
> > 
> > v2: rebase on upstream, update includes order
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 0892e1927..18a963564 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -30,6 +30,7 @@
> >  #include <unistd.h>
> >  
> >  #include "i915/gem.h"
> > +#include "i915/gem_vm.h"
> >  #include "igt.h"
> >  #include "igt_device_scan.h"
> >  #include "igt_kmod.h"
> > @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void vm_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       gem_require_vm(priv.fd.drm);
> > +
> > +       local_debug("creating additional GEM user address space");
> > +       igt_ignore_warn(gem_vm_create(priv.fd.drm));
> 
> Why the "ignore_warn"?  This deserves a comment. 
> And perhaps a word of information about why we picked
> this partucular operation in the commit message (vm_create).
> This is a regression test, right?

Hmm, I didn't intend it to be a regression test.  The purpose was
generally the same as of other hotunplug-lateclose subtests - exercise
the driver behaviour on hotunplug and lateclose.  This subtest was
intended to perform still the same exercise, only under different
conditions, or different use case of the driver.  In particular,
hotunplug is performed here with an additional address space associated
with an open file descriptor.  We are not interested in exercising that
address space (that's why igt_ignore_warn), only in checking if it is
cleaned up on time so hotunplug-lateclose operations are still safe
from late dma_unmap issues.

Let me try to reword the commit description so it better reflects this
idea.

Thanks,
Janusz

> 
> LGTM otherwise (but again - see previous patches):
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał

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

[Intel-gfx] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Janusz Krzysztofik]

GEM objects belonging to user file descriptors still open on device
hotunplug may exhibit still more driver issues.  Add a subtest that
implements this scenario.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 18a963564..c30d98a69 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void gem_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating a GEM user object");
+	igt_ignore_warn(gem_create(priv.fd.drm, 4096));
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -435,4 +458,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open GEM object can be cleanly unplugged, then released and recovered");
+	igt_subtest("gem-hotunplug-lateclose")
+		gem_hotunplug_lateclose();
+
+	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

[igt-dev] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Janusz Krzysztofik]

GEM objects belonging to user file descriptors still open on device
hotunplug may exhibit still more driver issues.  Add a subtest that
implements this scenario.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 18a963564..c30d98a69 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void gem_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating a GEM user object");
+	igt_ignore_warn(gem_create(priv.fd.drm, 4096));
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -435,4 +458,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open GEM object can be cleanly unplugged, then released and recovered");
+	igt_subtest("gem-hotunplug-lateclose")
+		gem_hotunplug_lateclose();
+
+	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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> GEM objects belonging to user file descriptors still open on device
> hotunplug may exhibit still more driver issues.  Add a subtest that
> implements this scenario.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 18a963564..c30d98a69 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void gem_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating a GEM user object");
> +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));

Same as previous one.
(note - we could just check for proper error when we attempt to close this
handle after unplug, and the same thing applies to the previous one with the vm)

LGTM otherwise.

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

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

Re: [igt-dev] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> GEM objects belonging to user file descriptors still open on device
> hotunplug may exhibit still more driver issues.  Add a subtest that
> implements this scenario.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 18a963564..c30d98a69 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void gem_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating a GEM user object");
> +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));

Same as previous one.
(note - we could just check for proper error when we attempt to close this
handle after unplug, and the same thing applies to the previous one with the vm)

LGTM otherwise.

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

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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Janusz Krzysztofik]

Hi Michał,

Thanks for review.

On Thu, 2020-06-25 at 21:51 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> > GEM objects belonging to user file descriptors still open on device
> > hotunplug may exhibit still more driver issues.  Add a subtest that
> > implements this scenario.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 18a963564..c30d98a69 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void gem_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating a GEM user object");
> > +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));
> 
> Same as previous one.
> (note - we could just check for proper error when we attempt to close this
> handle after unplug, and the same thing applies to the previous one with the vm)

Oh, now I see what you meant in case of the address space variant.

I was thinking about that.  We may need another subtests, or a group of
subtests, for exercising the driver's safety from post-hotunplug
attempts to use the removed device, not only to close it.  I decided to
provide those variants later and call them 'hotunplug-lateuse*'.

However, now I see that we may perfectly exercise the driver's
resistance to late use of additional user resources while having those
resources already created.  Then, let me extend applicable members of
this patch series with those checks.

Thanks,
Janusz

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

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

Re: [igt-dev] [RFC PATCH i-g-t v2 6/8] tests/core_hotunplug: Add 'GEM object' variant

[Janusz Krzysztofik]

Hi Michał,

Thanks for review.

On Thu, 2020-06-25 at 21:51 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> > GEM objects belonging to user file descriptors still open on device
> > hotunplug may exhibit still more driver issues.  Add a subtest that
> > implements this scenario.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 18a963564..c30d98a69 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void gem_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating a GEM user object");
> > +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));
> 
> Same as previous one.
> (note - we could just check for proper error when we attempt to close this
> handle after unplug, and the same thing applies to the previous one with the vm)

Oh, now I see what you meant in case of the address space variant.

I was thinking about that.  We may need another subtests, or a group of
subtests, for exercising the driver's safety from post-hotunplug
attempts to use the removed device, not only to close it.  I decided to
provide those variants later and call them 'hotunplug-lateuse*'.

However, now I see that we may perfectly exercise the driver's
resistance to late use of additional user resources while having those
resources already created.  Then, let me extend applicable members of
this patch series with those checks.

Thanks,
Janusz

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

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

[Intel-gfx] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Janusz Krzysztofik]

Even if all device file descriptors are closed on device hotunplug,
PRIME exported objects may still exists, referenced by still open
dma-buf file handles.  Add a subtest that keeps such handle open on
device hotunplug.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index c30d98a69..7cb699cc2 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void prime_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+	uint32_t handle;
+	int dmabuf;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating and PRIME-exporting a GEM object");
+	handle = gem_create(priv.fd.drm, 4096);
+	dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
+
+	local_debug("closing the device");
+	close(priv.fd.drm);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the PRIME file handle");
+	close(dmabuf);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -465,4 +494,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
+	igt_subtest("prime-hotunplug-lateclose")
+		prime_hotunplug_lateclose();
+
+	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

[igt-dev] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Janusz Krzysztofik]

Even if all device file descriptors are closed on device hotunplug,
PRIME exported objects may still exists, referenced by still open
dma-buf file handles.  Add a subtest that keeps such handle open on
device hotunplug.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index c30d98a69..7cb699cc2 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void prime_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+	uint32_t handle;
+	int dmabuf;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating and PRIME-exporting a GEM object");
+	handle = gem_create(priv.fd.drm, 4096);
+	dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
+
+	local_debug("closing the device");
+	close(priv.fd.drm);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the PRIME file handle");
+	close(dmabuf);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -465,4 +494,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
+	igt_subtest("prime-hotunplug-lateclose")
+		prime_hotunplug_lateclose();
+
+	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

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> Even if all device file descriptors are closed on device hotunplug,
> PRIME exported objects may still exists, referenced by still open
> dma-buf file handles.  Add a subtest that keeps such handle open on
> device hotunplug.
> 
> v2: rebase on upstream

Would be interesting to see what happens when someone actually imports an
object from unplugged device (or the device is unplugged after it was imported).
But perhaps that's something for the future.

Also - the naming should probably be kept distinct from the other "lateclose"
tests, since here we're closing the device FD before the unplug.
Maybe just "prime-hotunplug"? But that's up to you - either way:

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

-Michał

> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index c30d98a69..7cb699cc2 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void prime_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       uint32_t handle;
> +       int dmabuf;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating and PRIME-exporting a GEM object");
> +       handle = gem_create(priv.fd.drm, 4096);
> +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> +
> +       local_debug("closing the device");
> +       close(priv.fd.drm);
> +
> +       local_debug("hot unplugging the device");
> +       device_unplug(priv.fd.sysfs_dev);
> +
> +       local_debug("late closing the PRIME file handle");
> +       close(dmabuf);
> +
> +       local_debug("recovering the device");
> +       bus_rescan(priv.fd.sysfs_bus);
> +
> +       healthcheck();
> +}
> +
>  /* Main */
>  
>  igt_main
> @@ -465,4 +494,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> +       igt_subtest("prime-hotunplug-lateclose")
> +               prime_hotunplug_lateclose();
> +
> +       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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> Even if all device file descriptors are closed on device hotunplug,
> PRIME exported objects may still exists, referenced by still open
> dma-buf file handles.  Add a subtest that keeps such handle open on
> device hotunplug.
> 
> v2: rebase on upstream

Would be interesting to see what happens when someone actually imports an
object from unplugged device (or the device is unplugged after it was imported).
But perhaps that's something for the future.

Also - the naming should probably be kept distinct from the other "lateclose"
tests, since here we're closing the device FD before the unplug.
Maybe just "prime-hotunplug"? But that's up to you - either way:

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

-Michał

> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index c30d98a69..7cb699cc2 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void prime_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       uint32_t handle;
> +       int dmabuf;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating and PRIME-exporting a GEM object");
> +       handle = gem_create(priv.fd.drm, 4096);
> +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> +
> +       local_debug("closing the device");
> +       close(priv.fd.drm);
> +
> +       local_debug("hot unplugging the device");
> +       device_unplug(priv.fd.sysfs_dev);
> +
> +       local_debug("late closing the PRIME file handle");
> +       close(dmabuf);
> +
> +       local_debug("recovering the device");
> +       bus_rescan(priv.fd.sysfs_bus);
> +
> +       healthcheck();
> +}
> +
>  /* Main */
>  
>  igt_main
> @@ -465,4 +494,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> +       igt_subtest("prime-hotunplug-lateclose")
> +               prime_hotunplug_lateclose();
> +
> +       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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 21:57 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> > Even if all device file descriptors are closed on device hotunplug,
> > PRIME exported objects may still exists, referenced by still open
> > dma-buf file handles.  Add a subtest that keeps such handle open on
> > device hotunplug.
> > 
> > v2: rebase on upstream
> 
> Would be interesting to see what happens when someone actually imports an
> object from unplugged device (or the device is unplugged after it was imported).
> But perhaps that's something for the future.

Yes, let's keep it relatively simple for now.  There seems to be quite
a few possible scenarios to cover.   However, I'm going to add a very
basic use-after-hotunplug check, similar to what we have (hopefully)
agreed for context and address space variants.

> 
> Also - the naming should probably be kept distinct from the other "lateclose"
> tests, since here we're closing the device FD before the unplug.
> Maybe just "prime-hotunplug"? 

Since we are still interested in exercising the driver behaviour on
late closing the prime handle (now this case also explodes inside
intel-iommu), let's keep that namig even if we close the device and
only keep the prime file open.

Thanks,
Janusz


> But that's up to you - either way:
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał
> 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index c30d98a69..7cb699cc2 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void prime_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       uint32_t handle;
> > +       int dmabuf;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating and PRIME-exporting a GEM object");
> > +       handle = gem_create(priv.fd.drm, 4096);
> > +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> > +
> > +       local_debug("closing the device");
> > +       close(priv.fd.drm);
> > +
> > +       local_debug("hot unplugging the device");
> > +       device_unplug(priv.fd.sysfs_dev);
> > +
> > +       local_debug("late closing the PRIME file handle");
> > +       close(dmabuf);
> > +
> > +       local_debug("recovering the device");
> > +       bus_rescan(priv.fd.sysfs_bus);
> > +
> > +       healthcheck();
> > +}
> > +
> >  /* Main */
> >  
> >  igt_main
> > @@ -465,4 +494,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("prime-hotunplug-lateclose")
> > +               prime_hotunplug_lateclose();
> > +
> > +       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

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

Re: [igt-dev] [RFC PATCH i-g-t v2 7/8] tests/core_hotunplug: Add 'PRIME handle' variant

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 21:57 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> > Even if all device file descriptors are closed on device hotunplug,
> > PRIME exported objects may still exists, referenced by still open
> > dma-buf file handles.  Add a subtest that keeps such handle open on
> > device hotunplug.
> > 
> > v2: rebase on upstream
> 
> Would be interesting to see what happens when someone actually imports an
> object from unplugged device (or the device is unplugged after it was imported).
> But perhaps that's something for the future.

Yes, let's keep it relatively simple for now.  There seems to be quite
a few possible scenarios to cover.   However, I'm going to add a very
basic use-after-hotunplug check, similar to what we have (hopefully)
agreed for context and address space variants.

> 
> Also - the naming should probably be kept distinct from the other "lateclose"
> tests, since here we're closing the device FD before the unplug.
> Maybe just "prime-hotunplug"? 

Since we are still interested in exercising the driver behaviour on
late closing the prime handle (now this case also explodes inside
intel-iommu), let's keep that namig even if we close the device and
only keep the prime file open.

Thanks,
Janusz


> But that's up to you - either way:
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał
> 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index c30d98a69..7cb699cc2 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void prime_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       uint32_t handle;
> > +       int dmabuf;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating and PRIME-exporting a GEM object");
> > +       handle = gem_create(priv.fd.drm, 4096);
> > +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> > +
> > +       local_debug("closing the device");
> > +       close(priv.fd.drm);
> > +
> > +       local_debug("hot unplugging the device");
> > +       device_unplug(priv.fd.sysfs_dev);
> > +
> > +       local_debug("late closing the PRIME file handle");
> > +       close(dmabuf);
> > +
> > +       local_debug("recovering the device");
> > +       bus_rescan(priv.fd.sysfs_bus);
> > +
> > +       healthcheck();
> > +}
> > +
> >  /* Main */
> >  
> >  igt_main
> > @@ -465,4 +494,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("prime-hotunplug-lateclose")
> > +               prime_hotunplug_lateclose();
> > +
> > +       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

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

[Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Janusz Krzysztofik]

Verify if a device with a GEM batch job still running on a GPU can be
hot-unplugged cleanly and released, then recovered.

v2: rebase on upstream

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

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7cb699cc2..672ff661d 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -33,6 +33,7 @@
 #include "i915/gem_vm.h"
 #include "igt.h"
 #include "igt_device_scan.h"
+#include "igt_dummyload.h"
 #include "igt_kmod.h"
 #include "igt_sysfs.h"
 
@@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void batch_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+	igt_spin_t *spin;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("running dummy load");
+	spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
+						    IGT_SPIN_NO_PREEMPTION);
+	igt_spin_busywait_until_started(spin);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -501,4 +528,11 @@ igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
+	igt_subtest("batch-hotunplug-lateclose")
+		batch_hotunplug_lateclose();
+
+	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

Re: [Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> Verify if a device with a GEM batch job still running on a GPU can be
> hot-unplugged cleanly and released, then recovered.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 7cb699cc2..672ff661d 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -33,6 +33,7 @@
>  #include "i915/gem_vm.h"
>  #include "igt.h"
>  #include "igt_device_scan.h"
> +#include "igt_dummyload.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
>  
> @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void batch_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       igt_spin_t *spin;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("running dummy load");
> +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> +                                                   IGT_SPIN_NO_PREEMPTION);

Do we need IGT_SPIN_NO_PREEMPTION here?
We're also leaking spin here... And I don't think we can just call igt_spin_free
after unplug, can we?

-Michał

> +       igt_spin_busywait_until_started(spin);
> +
> +       local_debug("hot unplugging the device");
> +       device_unplug(priv.fd.sysfs_dev);
> +
> +       local_debug("late closing the removed device instance");
> +       close(priv.fd.drm);
> +
> +       local_debug("recovering the device");
> +       bus_rescan(priv.fd.sysfs_bus);
> +
> +       healthcheck();
> +}
> +
>  /* Main */
>  
>  igt_main
> @@ -501,4 +528,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> +       igt_subtest("batch-hotunplug-lateclose")
> +               batch_hotunplug_lateclose();
> +
> +       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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Michał Winiarski]

Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> Verify if a device with a GEM batch job still running on a GPU can be
> hot-unplugged cleanly and released, then recovered.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 7cb699cc2..672ff661d 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -33,6 +33,7 @@
>  #include "i915/gem_vm.h"
>  #include "igt.h"
>  #include "igt_device_scan.h"
> +#include "igt_dummyload.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
>  
> @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void batch_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       igt_spin_t *spin;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("running dummy load");
> +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> +                                                   IGT_SPIN_NO_PREEMPTION);

Do we need IGT_SPIN_NO_PREEMPTION here?
We're also leaking spin here... And I don't think we can just call igt_spin_free
after unplug, can we?

-Michał

> +       igt_spin_busywait_until_started(spin);
> +
> +       local_debug("hot unplugging the device");
> +       device_unplug(priv.fd.sysfs_dev);
> +
> +       local_debug("late closing the removed device instance");
> +       close(priv.fd.drm);
> +
> +       local_debug("recovering the device");
> +       bus_rescan(priv.fd.sysfs_bus);
> +
> +       healthcheck();
> +}
> +
>  /* Main */
>  
>  igt_main
> @@ -501,4 +528,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> +       igt_subtest("batch-hotunplug-lateclose")
> +               batch_hotunplug_lateclose();
> +
> +       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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 22:02 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> > Verify if a device with a GEM batch job still running on a GPU can be
> > hot-unplugged cleanly and released, then recovered.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 7cb699cc2..672ff661d 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -33,6 +33,7 @@
> >  #include "i915/gem_vm.h"
> >  #include "igt.h"
> >  #include "igt_device_scan.h"
> > +#include "igt_dummyload.h"
> >  #include "igt_kmod.h"
> >  #include "igt_sysfs.h"
> >  
> > @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void batch_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       igt_spin_t *spin;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("running dummy load");
> > +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> > +                                                   IGT_SPIN_NO_PREEMPTION);
> 
> Do we need IGT_SPIN_NO_PREEMPTION here?

Assuming my understanding of IGT_SPIN_NO_PREEMPTION was correct, my
intention was to exercise the driver's ability to cancel persistent GPU
tasks on hotunplug and clean up their associated resources on time in
order to avoid late dma_unmap issues.  Please advise if you still think
this this flag not needed.

> We're also leaking spin here... And I don't think we can just call igt_spin_free
> after unplug, can we?

If you mean memory occupied by the spin structure, I know leaking it
looks bad but I think that shouldn't be a problem in a user space
process that is going to exit soon.  But ayway, let me try what happens
on late igt_spin_free attempt.

Thanks,
Janusz

> 
> -Michał
> 
> > +       igt_spin_busywait_until_started(spin);
> > +
> > +       local_debug("hot unplugging the device");
> > +       device_unplug(priv.fd.sysfs_dev);
> > +
> > +       local_debug("late closing the removed device instance");
> > +       close(priv.fd.drm);
> > +
> > +       local_debug("recovering the device");
> > +       bus_rescan(priv.fd.sysfs_bus);
> > +
> > +       healthcheck();
> > +}
> > +
> >  /* Main */
> >  
> >  igt_main
> > @@ -501,4 +528,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("batch-hotunplug-lateclose")
> > +               batch_hotunplug_lateclose();
> > +
> > +       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

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

Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Janusz Krzysztofik]

Hi Michał,

On Thu, 2020-06-25 at 22:02 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> > Verify if a device with a GEM batch job still running on a GPU can be
> > hot-unplugged cleanly and released, then recovered.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 7cb699cc2..672ff661d 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -33,6 +33,7 @@
> >  #include "i915/gem_vm.h"
> >  #include "igt.h"
> >  #include "igt_device_scan.h"
> > +#include "igt_dummyload.h"
> >  #include "igt_kmod.h"
> >  #include "igt_sysfs.h"
> >  
> > @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void batch_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       igt_spin_t *spin;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("running dummy load");
> > +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> > +                                                   IGT_SPIN_NO_PREEMPTION);
> 
> Do we need IGT_SPIN_NO_PREEMPTION here?

Assuming my understanding of IGT_SPIN_NO_PREEMPTION was correct, my
intention was to exercise the driver's ability to cancel persistent GPU
tasks on hotunplug and clean up their associated resources on time in
order to avoid late dma_unmap issues.  Please advise if you still think
this this flag not needed.

> We're also leaking spin here... And I don't think we can just call igt_spin_free
> after unplug, can we?

If you mean memory occupied by the spin structure, I know leaking it
looks bad but I think that shouldn't be a problem in a user space
process that is going to exit soon.  But ayway, let me try what happens
on late igt_spin_free attempt.

Thanks,
Janusz

> 
> -Michał
> 
> > +       igt_spin_busywait_until_started(spin);
> > +
> > +       local_debug("hot unplugging the device");
> > +       device_unplug(priv.fd.sysfs_dev);
> > +
> > +       local_debug("late closing the removed device instance");
> > +       close(priv.fd.drm);
> > +
> > +       local_debug("recovering the device");
> > +       bus_rescan(priv.fd.sysfs_bus);
> > +
> > +       healthcheck();
> > +}
> > +
> >  /* Main */
> >  
> >  igt_main
> > @@ -501,4 +528,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("batch-hotunplug-lateclose")
> > +               batch_hotunplug_lateclose();
> > +
> > +       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

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

[igt-dev] ✗ Fi.CI.IGT: failure for tests/core_hotunplug: New subtests and enhancements (rev2)

[Patchwork]

== Series Details ==

Series: tests/core_hotunplug: New subtests and enhancements (rev2)
URL   : https://patchwork.freedesktop.org/series/78698/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8651_full -> IGTPW_4687_full
====================================================

Summary
-------

  **FAILURE**

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_flip@2x-blocking-absolute-wf_vblank@ab-vga1-hdmi-a1:
    - shard-hsw:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw2/igt@kms_flip@2x-blocking-absolute-wf_vblank@ab-vga1-hdmi-a1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw2/igt@kms_flip@2x-blocking-absolute-wf_vblank@ab-vga1-hdmi-a1.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-hsw:          [SKIP][3] ([fdo#109271]) -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw2/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-snb:          [PASS][5] -> [TIMEOUT][6] ([i915#1958]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@gem_busy@close-race.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb5/igt@gem_busy@close-race.html

  * igt@gem_exec_flush@basic-wb-pro-default:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([i915#1635] / [i915#95]) +29 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl1/igt@gem_exec_flush@basic-wb-pro-default.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl1/igt@gem_exec_flush@basic-wb-pro-default.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_8651/shard-glk8/igt@gem_exec_reloc@basic-concurrent0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk2/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_8651/shard-glk9/igt@gem_exec_whisper@basic-queues-priority-all.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk2/igt@gem_exec_whisper@basic-queues-priority-all.html

  * igt@gem_shrink@reclaim:
    - shard-hsw:          [PASS][13] -> [SKIP][14] ([fdo#109271])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw4/igt@gem_shrink@reclaim.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw1/igt@gem_shrink@reclaim.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#1436] / [i915#716])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl3/igt@gen9_exec_parse@allowed-all.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl3/igt@gen9_exec_parse@allowed-all.html

  * igt@kms_atomic_transition@plane-toggle-modeset-transition:
    - shard-snb:          [PASS][17] -> [SKIP][18] ([fdo#109271])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@kms_atomic_transition@plane-toggle-modeset-transition.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb1/igt@kms_atomic_transition@plane-toggle-modeset-transition.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen:
    - shard-kbl:          [PASS][19] -> [DMESG-FAIL][20] ([i915#54] / [i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-64x21-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180] / [i915#93] / [i915#95])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-tglb:         [PASS][23] -> [FAIL][24] ([IGT#5])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-tglb2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-tglb6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
    - shard-glk:          [PASS][25] -> [FAIL][26] ([i915#79])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk6/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk1/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][27] -> [DMESG-WARN][28] ([i915#180]) +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@wf_vblank-ts-check@a-dp1:
    - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#1982])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl6/igt@kms_flip@wf_vblank-ts-check@a-dp1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl6/igt@kms_flip@wf_vblank-ts-check@a-dp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         [PASS][31] -> [INCOMPLETE][32] ([i915#123])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt:
    - shard-tglb:         [PASS][33] -> [DMESG-WARN][34] ([i915#402]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-pgflip-blt.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-hsw:          [PASS][35] -> [TIMEOUT][36] ([i915#1958]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-kbl:          [PASS][37] -> [DMESG-FAIL][38] ([fdo#108145] / [i915#95])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
    - shard-apl:          [PASS][39] -> [DMESG-FAIL][40] ([fdo#108145] / [i915#1635] / [i915#95])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_cursor@pipe-a-overlay-size-64:
    - shard-apl:          [PASS][41] -> [DMESG-FAIL][42] ([i915#1635] / [i915#95])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl2/igt@kms_plane_cursor@pipe-a-overlay-size-64.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl6/igt@kms_plane_cursor@pipe-a-overlay-size-64.html

  * igt@kms_plane_cursor@pipe-c-overlay-size-64:
    - shard-iclb:         [PASS][43] -> [TIMEOUT][44] ([i915#1958]) +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb4/igt@kms_plane_cursor@pipe-c-overlay-size-64.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb3/igt@kms_plane_cursor@pipe-c-overlay-size-64.html
    - shard-glk:          [PASS][45] -> [TIMEOUT][46] ([i915#1958]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk6/igt@kms_plane_cursor@pipe-c-overlay-size-64.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk6/igt@kms_plane_cursor@pipe-c-overlay-size-64.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][47] -> [SKIP][48] ([fdo#109642] / [fdo#111068])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb6/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][49] -> [SKIP][50] ([fdo#109441]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vblank@pipe-a-query-forked-hang:
    - shard-kbl:          [PASS][51] -> [DMESG-WARN][52] ([i915#93] / [i915#95]) +52 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl1/igt@kms_vblank@pipe-a-query-forked-hang.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl3/igt@kms_vblank@pipe-a-query-forked-hang.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-apl:          [PASS][53] -> [INCOMPLETE][54] ([i915#1635])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl2/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl7/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@kms_vblank@pipe-b-wait-busy:
    - shard-apl:          [PASS][55] -> [DMESG-WARN][56] ([i915#1982]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl8/igt@kms_vblank@pipe-b-wait-busy.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl6/igt@kms_vblank@pipe-b-wait-busy.html

  
#### Possible fixes ####

  * igt@drm_read@invalid-buffer:
    - shard-tglb:         [DMESG-WARN][57] ([i915#402]) -> [PASS][58] +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-tglb3/igt@drm_read@invalid-buffer.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-tglb7/igt@drm_read@invalid-buffer.html

  * igt@gem_exec_whisper@basic-fds-priority:
    - shard-glk:          [DMESG-WARN][59] ([i915#118] / [i915#95]) -> [PASS][60] +3 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk6/igt@gem_exec_whisper@basic-fds-priority.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk7/igt@gem_exec_whisper@basic-fds-priority.html

  * igt@gem_mmap_wc@read-write-distinct:
    - shard-hsw:          [TIMEOUT][61] ([i915#1958]) -> [PASS][62] +3 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw4/igt@gem_mmap_wc@read-write-distinct.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw6/igt@gem_mmap_wc@read-write-distinct.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][63] ([i915#454]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb1/igt@i915_pm_dc@dc6-psr.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb1/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_big_fb@linear-64bpp-rotate-180:
    - shard-apl:          [DMESG-WARN][65] ([i915#1982]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl7/igt@kms_big_fb@linear-64bpp-rotate-180.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl1/igt@kms_big_fb@linear-64bpp-rotate-180.html
    - shard-glk:          [DMESG-FAIL][67] ([i915#118] / [i915#95]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-180.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk7/igt@kms_big_fb@linear-64bpp-rotate-180.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x128-random:
    - shard-kbl:          [DMESG-FAIL][69] ([i915#54] / [i915#95]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-128x128-random.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge:
    - shard-glk:          [DMESG-WARN][71] ([i915#1982]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk2/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk8/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled:
    - shard-apl:          [DMESG-FAIL][73] ([i915#1635] / [i915#54] / [i915#95]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl7/igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-untiled.html

  * igt@kms_flip@2x-nonexisting-fb-interruptible@ab-vga1-hdmi-a1:
    - shard-hsw:          [DMESG-WARN][75] ([i915#1982]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-hsw6/igt@kms_flip@2x-nonexisting-fb-interruptible@ab-vga1-hdmi-a1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-hsw4/igt@kms_flip@2x-nonexisting-fb-interruptible@ab-vga1-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-snb:          [SKIP][77] ([fdo#109271]) -> [PASS][78] +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          [FAIL][79] ([i915#247]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl7/igt@kms_plane@plane-position-covered-pipe-c-planes.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl4/igt@kms_plane@plane-position-covered-pipe-c-planes.html
    - shard-kbl:          [FAIL][81] ([i915#247]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl6/igt@kms_plane@plane-position-covered-pipe-c-planes.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl7/igt@kms_plane@plane-position-covered-pipe-c-planes.html

  * igt@kms_plane_cursor@pipe-a-viewport-size-64:
    - shard-kbl:          [DMESG-FAIL][83] ([i915#95]) -> [PASS][84] +2 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl7/igt@kms_plane_cursor@pipe-a-viewport-size-64.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl1/igt@kms_plane_cursor@pipe-a-viewport-size-64.html
    - shard-apl:          [DMESG-FAIL][85] ([i915#1635] / [i915#95]) -> [PASS][86] +2 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl4/igt@kms_plane_cursor@pipe-a-viewport-size-64.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl4/igt@kms_plane_cursor@pipe-a-viewport-size-64.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][87] ([fdo#109642] / [fdo#111068]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb4/igt@kms_psr2_su@frontbuffer.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][89] ([fdo#109441]) -> [PASS][90] +2 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb8/igt@kms_psr@psr2_no_drrs.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_vblank@pipe-a-query-forked:
    - shard-kbl:          [DMESG-WARN][91] ([i915#93] / [i915#95]) -> [PASS][92] +47 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl1/igt@kms_vblank@pipe-a-query-forked.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl7/igt@kms_vblank@pipe-a-query-forked.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][93] ([i915#180]) -> [PASS][94] +6 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf@invalid-oa-metric-set-id:
    - shard-apl:          [DMESG-WARN][95] ([i915#1635] / [i915#95]) -> [PASS][96] +31 similar issues
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl3/igt@perf@invalid-oa-metric-set-id.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl8/igt@perf@invalid-oa-metric-set-id.html

  * igt@syncobj_wait@invalid-multi-wait-all-unsubmitted-signaled:
    - shard-snb:          [TIMEOUT][97] ([i915#1958]) -> [PASS][98] +2 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@syncobj_wait@invalid-multi-wait-all-unsubmitted-signaled.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb1/igt@syncobj_wait@invalid-multi-wait-all-unsubmitted-signaled.html

  
#### Warnings ####

  * igt@gem_exec_reloc@basic-concurrent16:
    - shard-iclb:         [INCOMPLETE][99] ([i915#1958]) -> [TIMEOUT][100] ([i915#1958])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb8/igt@gem_exec_reloc@basic-concurrent16.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb3/igt@gem_exec_reloc@basic-concurrent16.html
    - shard-glk:          [INCOMPLETE][101] ([i915#1958] / [i915#58] / [k.org#198133]) -> [TIMEOUT][102] ([i915#1958])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk7/igt@gem_exec_reloc@basic-concurrent16.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk6/igt@gem_exec_reloc@basic-concurrent16.html

  * igt@gem_userptr_blits@process-exit-mmap@gtt:
    - shard-apl:          [SKIP][103] ([fdo#109271] / [i915#1635] / [i915#1699]) -> [SKIP][104] ([fdo#109271] / [i915#1699])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl6/igt@gem_userptr_blits@process-exit-mmap@gtt.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl3/igt@gem_userptr_blits@process-exit-mmap@gtt.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][105] ([i915#588]) -> [SKIP][106] ([i915#658])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-iclb3/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [SKIP][107] ([i915#468]) -> [FAIL][108] ([i915#454])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-tglb7/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
    - shard-apl:          [SKIP][109] ([fdo#109271] / [i915#1937]) -> [SKIP][110] ([fdo#109271] / [i915#1635] / [i915#1937])
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl8/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html

  * igt@kms_chamelium@dp-edid-read:
    - shard-apl:          [SKIP][111] ([fdo#109271] / [fdo#111827] / [i915#1635]) -> [SKIP][112] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl3/igt@kms_chamelium@dp-edid-read.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl2/igt@kms_chamelium@dp-edid-read.html

  * igt@kms_color_chamelium@pipe-c-gamma:
    - shard-apl:          [SKIP][113] ([fdo#109271] / [fdo#111827]) -> [SKIP][114] ([fdo#109271] / [fdo#111827] / [i915#1635])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl2/igt@kms_color_chamelium@pipe-c-gamma.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl6/igt@kms_color_chamelium@pipe-c-gamma.html

  * igt@kms_cursor_edge_walk@pipe-d-128x128-left-edge:
    - shard-apl:          [SKIP][115] ([fdo#109271]) -> [SKIP][116] ([fdo#109271] / [i915#1635]) +23 similar issues
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl3/igt@kms_cursor_edge_walk@pipe-d-128x128-left-edge.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl1/igt@kms_cursor_edge_walk@pipe-d-128x128-left-edge.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-snb:          [SKIP][117] ([fdo#109271]) -> [TIMEOUT][118] ([i915#1958]) +1 similar issue
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
    - shard-glk:          [SKIP][119] ([fdo#109271]) -> [INCOMPLETE][120] ([i915#58] / [k.org#198133])
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-glk1/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-glk6/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-indfb-plflip-blt:
    - shard-snb:          [SKIP][121] ([fdo#109271]) -> [INCOMPLETE][122] ([i915#82])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-indfb-plflip-blt.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb5/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-indfb-plflip-blt.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-apl:          [DMESG-FAIL][123] ([i915#1635] / [i915#95]) -> [FAIL][124] ([i915#265])
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl3/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb:
    - shard-kbl:          [DMESG-FAIL][125] ([i915#95]) -> [FAIL][126] ([i915#265])
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-kbl1/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-kbl4/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html

  * igt@kms_plane_cursor@pipe-c-viewport-size-256:
    - shard-snb:          [TIMEOUT][127] ([i915#1958]) -> [SKIP][128] ([fdo#109271])
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-snb6/igt@kms_plane_cursor@pipe-c-viewport-size-256.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-snb2/igt@kms_plane_cursor@pipe-c-viewport-size-256.html

  * igt@prime_nv_pcopy@test2:
    - shard-apl:          [SKIP][129] ([fdo#109271] / [i915#1635]) -> [SKIP][130] ([fdo#109271]) +12 similar issues
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8651/shard-apl6/igt@prime_nv_pcopy@test2.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/shard-apl8/igt@prime_nv_pcopy@test2.html

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

  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1699]: https://gitlab.freedesktop.org/drm/intel/issues/1699
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [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#247]: https://gitlab.freedesktop.org/drm/intel/issues/247
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [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#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (11 -> 8)
------------------------------

  Missing    (3): pig-skl-6260u pig-glk-j5005 pig-icl-1065g7 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5715 -> IGTPW_4687
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_8651: f6210d1dd268f9e09e10d3704c768d7679a44f48 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_4687: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4687/index.html
  IGT_5715: 3b6975c0f9e429c0c1f48c61a3417be9d68300cf @ 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_4687/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ GitLab.Pipeline: warning for tests/core_hotunplug: New subtests and enhancements (rev2)

[Patchwork]

== Series Details ==

Series: tests/core_hotunplug: New subtests and enhancements (rev2)
URL   : https://patchwork.freedesktop.org/series/78698/
State : warning

== Summary ==

Did not get list of undocumented tests for this run, something is wrong!

Other than that, pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/165472 for the overview.

build-containers:build-debian-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/3230552):
  section_start:1592844401:restore_cache
  Restoring cache
  section_end:1592844404:restore_cache
  section_start:1592844404:download_artifacts
  Downloading artifacts
  section_end:1592844409:download_artifacts
  section_start:1592844409:build_script
  Running before_script and script
  Authenticating with credentials from job payload (GitLab Registry)
  $ podman login -u gitlab-ci-token -p $CI_JOB_TOKEN $CI_REGISTRY
  Inconsistency detected by ld.so: ../sysdeps/x86_64/dl-machine.h: 538: elf_machine_rela_relative: Assertion `ELFW(R_TYPE) (reloc->r_info) == R_X86_64_RELATIVE' failed!
  section_end:1592844420:build_script
  section_start:1592844420:after_script
  Running after_script
  section_end:1592844421:after_script
  section_start:1592844421:upload_artifacts_on_failure
  Uploading artifacts for failed job
  section_end:1592844424:upload_artifacts_on_failure
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/165472
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev