[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
---
 tests/drv_module_reload.c | 48 +++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 092982960..e6bc354b6 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,41 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir, err;
+
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	err = 1;
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &err);
+	close(dir);
+
+	igt_kmod_unload(module_name, 0);
+
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -323,12 +358,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) < 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.17.1

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

Re: [PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Michał Winiarski]

On Wed, Jun 06, 2018 at 02:09:16PM +0100, Chris Wilson wrote:
> The current method of checking for a failed module load is flawed, as we
> only report the error on probing it is not being reported back by
> modprobe. So we have to dig inside the module_parameters while the
> module is still loaded to discover the error.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>

That also works.

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

Should we push now? Or delay until we fix i915?

-Michał

> ---
>  tests/drv_module_reload.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Michał Winiarski (2018-06-06 15:18:14)
> On Wed, Jun 06, 2018 at 02:09:16PM +0100, Chris Wilson wrote:
> > The current method of checking for a failed module load is flawed, as we
> > only report the error on probing it is not being reported back by
> > modprobe. So we have to dig inside the module_parameters while the
> > module is still loaded to discover the error.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> 
> That also works.
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> Should we push now? Or delay until we fix i915?

We wait a bit to fix i915. But the corresponding kernel patch should be
clean to go (well hopefully v2!).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 092982960..e18aaea5e 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,38 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_kmod_unload(module_name, 0);
+
+	return fault;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -323,12 +355,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.17.1

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

[Intel-gfx] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 092982960..e18aaea5e 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,38 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_kmod_unload(module_name, 0);
+
+	return fault;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -323,12 +355,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.17.1

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

Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Imre Deak]

On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> The current method of checking for a failed module load is flawed, as we
> only report the error on probing it is not being reported back by
> modprobe. So we have to dig inside the module_parameters while the
> module is still loaded to discover the error.
> 
> v2: Expect i915.inject_load_failure to be zero on success
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 092982960..e18aaea5e 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -234,6 +234,38 @@ reload(const char *opts_i915)
>  	return err;
>  }
>  
> +static int open_parameters(const char *module_name)
> +{
> +	char path[256];
> +
> +	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> +	return open(path, O_RDONLY);
> +}
> +
> +static int
> +inject_fault(const char *module_name, const char *opt, int fault)
> +{
> +	char buf[1024];
> +	int dir;
> +
> +	igt_assert(fault > 0);
> +	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> +
> +	if (igt_kmod_load(module_name, buf)) {
> +		igt_warn("Failed to load module '%s' with options '%s'\n",
> +			 module_name, buf);
> +		return 1;
> +	}
> +
> +	dir = open_parameters(module_name);
> +	igt_sysfs_scanf(dir, opt, "%d", &fault);

Just a note for later: in case we switch to async probing we'll have to
wait somehow until the probe completed. One way would be to disable
async probing for this test, not sure if that could hide some problem
though.

> +	close(dir);
> +
> +	igt_kmod_unload(module_name, 0);
> +
> +	return fault;
> +}
> +
>  static void
>  gem_sanitycheck(void)
>  {
> @@ -323,12 +355,15 @@ igt_main
>  		igt_assert_eq(reload("disable_display=1"), 0);
>  
>  	igt_subtest("basic-reload-inject") {
> -		char buf[64];
>  		int i = 0;
> -		do {
> -			snprintf(buf, sizeof(buf),
> -				 "inject_load_failure=%d", ++i);
> -		} while (reload(buf));
> +
> +		igt_i915_driver_unload();
> +
> +		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> +			;
> +
> +		/* We expect to hit at least one fault! */
> +		igt_assert(i > 1);
>  	}
>  
>  	igt_fixture {
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Imre Deak]

On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> The current method of checking for a failed module load is flawed, as we
> only report the error on probing it is not being reported back by
> modprobe. So we have to dig inside the module_parameters while the
> module is still loaded to discover the error.
> 
> v2: Expect i915.inject_load_failure to be zero on success
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 092982960..e18aaea5e 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -234,6 +234,38 @@ reload(const char *opts_i915)
>  	return err;
>  }
>  
> +static int open_parameters(const char *module_name)
> +{
> +	char path[256];
> +
> +	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> +	return open(path, O_RDONLY);
> +}
> +
> +static int
> +inject_fault(const char *module_name, const char *opt, int fault)
> +{
> +	char buf[1024];
> +	int dir;
> +
> +	igt_assert(fault > 0);
> +	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> +
> +	if (igt_kmod_load(module_name, buf)) {
> +		igt_warn("Failed to load module '%s' with options '%s'\n",
> +			 module_name, buf);
> +		return 1;
> +	}
> +
> +	dir = open_parameters(module_name);
> +	igt_sysfs_scanf(dir, opt, "%d", &fault);

Just a note for later: in case we switch to async probing we'll have to
wait somehow until the probe completed. One way would be to disable
async probing for this test, not sure if that could hide some problem
though.

> +	close(dir);
> +
> +	igt_kmod_unload(module_name, 0);
> +
> +	return fault;
> +}
> +
>  static void
>  gem_sanitycheck(void)
>  {
> @@ -323,12 +355,15 @@ igt_main
>  		igt_assert_eq(reload("disable_display=1"), 0);
>  
>  	igt_subtest("basic-reload-inject") {
> -		char buf[64];
>  		int i = 0;
> -		do {
> -			snprintf(buf, sizeof(buf),
> -				 "inject_load_failure=%d", ++i);
> -		} while (reload(buf));
> +
> +		igt_i915_driver_unload();
> +
> +		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> +			;
> +
> +		/* We expect to hit at least one fault! */
> +		igt_assert(i > 1);
>  	}
>  
>  	igt_fixture {
> -- 
> 2.17.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Imre Deak (2018-06-06 19:00:52)
> On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> > The current method of checking for a failed module load is flawed, as we
> > only report the error on probing it is not being reported back by
> > modprobe. So we have to dig inside the module_parameters while the
> > module is still loaded to discover the error.
> > 
> > v2: Expect i915.inject_load_failure to be zero on success
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > index 092982960..e18aaea5e 100644
> > --- a/tests/drv_module_reload.c
> > +++ b/tests/drv_module_reload.c
> > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> >       return err;
> >  }
> >  
> > +static int open_parameters(const char *module_name)
> > +{
> > +     char path[256];
> > +
> > +     snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> > +     return open(path, O_RDONLY);
> > +}
> > +
> > +static int
> > +inject_fault(const char *module_name, const char *opt, int fault)
> > +{
> > +     char buf[1024];
> > +     int dir;
> > +
> > +     igt_assert(fault > 0);
> > +     snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> > +
> > +     if (igt_kmod_load(module_name, buf)) {
> > +             igt_warn("Failed to load module '%s' with options '%s'\n",
> > +                      module_name, buf);
> > +             return 1;
> > +     }
> > +
> > +     dir = open_parameters(module_name);
> > +     igt_sysfs_scanf(dir, opt, "%d", &fault);
> 
> Just a note for later: in case we switch to async probing we'll have to
> wait somehow until the probe completed. One way would be to disable
> async probing for this test, not sure if that could hide some problem
> though.

modprobe already waits for async probes (via async_synchronize_full()
before it returns to userspace). Afaict, that async flag only has any
relevance for builtins. If you want to parallel module loading, you need
to fork.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Imre Deak (2018-06-06 19:00:52)
> On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> > The current method of checking for a failed module load is flawed, as we
> > only report the error on probing it is not being reported back by
> > modprobe. So we have to dig inside the module_parameters while the
> > module is still loaded to discover the error.
> > 
> > v2: Expect i915.inject_load_failure to be zero on success
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > index 092982960..e18aaea5e 100644
> > --- a/tests/drv_module_reload.c
> > +++ b/tests/drv_module_reload.c
> > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> >       return err;
> >  }
> >  
> > +static int open_parameters(const char *module_name)
> > +{
> > +     char path[256];
> > +
> > +     snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> > +     return open(path, O_RDONLY);
> > +}
> > +
> > +static int
> > +inject_fault(const char *module_name, const char *opt, int fault)
> > +{
> > +     char buf[1024];
> > +     int dir;
> > +
> > +     igt_assert(fault > 0);
> > +     snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> > +
> > +     if (igt_kmod_load(module_name, buf)) {
> > +             igt_warn("Failed to load module '%s' with options '%s'\n",
> > +                      module_name, buf);
> > +             return 1;
> > +     }
> > +
> > +     dir = open_parameters(module_name);
> > +     igt_sysfs_scanf(dir, opt, "%d", &fault);
> 
> Just a note for later: in case we switch to async probing we'll have to
> wait somehow until the probe completed. One way would be to disable
> async probing for this test, not sure if that could hide some problem
> though.

modprobe already waits for async probes (via async_synchronize_full()
before it returns to userspace). Afaict, that async flag only has any
relevance for builtins. If you want to parallel module loading, you need
to fork.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Imre Deak]

On Wed, Jun 06, 2018 at 07:04:47PM +0100, Chris Wilson wrote:
> Quoting Imre Deak (2018-06-06 19:00:52)
> > On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> > > The current method of checking for a failed module load is flawed, as we
> > > only report the error on probing it is not being reported back by
> > > modprobe. So we have to dig inside the module_parameters while the
> > > module is still loaded to discover the error.
> > > 
> > > v2: Expect i915.inject_load_failure to be zero on success
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > > ---
> > >  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 40 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > > index 092982960..e18aaea5e 100644
> > > --- a/tests/drv_module_reload.c
> > > +++ b/tests/drv_module_reload.c
> > > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> > >       return err;
> > >  }
> > >  
> > > +static int open_parameters(const char *module_name)
> > > +{
> > > +     char path[256];
> > > +
> > > +     snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> > > +     return open(path, O_RDONLY);
> > > +}
> > > +
> > > +static int
> > > +inject_fault(const char *module_name, const char *opt, int fault)
> > > +{
> > > +     char buf[1024];
> > > +     int dir;
> > > +
> > > +     igt_assert(fault > 0);
> > > +     snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> > > +
> > > +     if (igt_kmod_load(module_name, buf)) {
> > > +             igt_warn("Failed to load module '%s' with options '%s'\n",
> > > +                      module_name, buf);
> > > +             return 1;
> > > +     }
> > > +
> > > +     dir = open_parameters(module_name);
> > > +     igt_sysfs_scanf(dir, opt, "%d", &fault);
> > 
> > Just a note for later: in case we switch to async probing we'll have to
> > wait somehow until the probe completed. One way would be to disable
> > async probing for this test, not sure if that could hide some problem
> > though.
> 
> modprobe already waits for async probes (via async_synchronize_full()
> before it returns to userspace). Afaict, that async flag only has any
> relevance for builtins. If you want to parallel module loading, you need
> to fork.

Hm, right. Looking at that now, there's also the async_probe module
param, but we only need to care about the driver flag. So looks ok then.

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

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Imre Deak]

On Wed, Jun 06, 2018 at 07:04:47PM +0100, Chris Wilson wrote:
> Quoting Imre Deak (2018-06-06 19:00:52)
> > On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> > > The current method of checking for a failed module load is flawed, as we
> > > only report the error on probing it is not being reported back by
> > > modprobe. So we have to dig inside the module_parameters while the
> > > module is still loaded to discover the error.
> > > 
> > > v2: Expect i915.inject_load_failure to be zero on success
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > > ---
> > >  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 40 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > > index 092982960..e18aaea5e 100644
> > > --- a/tests/drv_module_reload.c
> > > +++ b/tests/drv_module_reload.c
> > > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> > >       return err;
> > >  }
> > >  
> > > +static int open_parameters(const char *module_name)
> > > +{
> > > +     char path[256];
> > > +
> > > +     snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> > > +     return open(path, O_RDONLY);
> > > +}
> > > +
> > > +static int
> > > +inject_fault(const char *module_name, const char *opt, int fault)
> > > +{
> > > +     char buf[1024];
> > > +     int dir;
> > > +
> > > +     igt_assert(fault > 0);
> > > +     snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> > > +
> > > +     if (igt_kmod_load(module_name, buf)) {
> > > +             igt_warn("Failed to load module '%s' with options '%s'\n",
> > > +                      module_name, buf);
> > > +             return 1;
> > > +     }
> > > +
> > > +     dir = open_parameters(module_name);
> > > +     igt_sysfs_scanf(dir, opt, "%d", &fault);
> > 
> > Just a note for later: in case we switch to async probing we'll have to
> > wait somehow until the probe completed. One way would be to disable
> > async probing for this test, not sure if that could hide some problem
> > though.
> 
> modprobe already waits for async probes (via async_synchronize_full()
> before it returns to userspace). Afaict, that async flag only has any
> relevance for builtins. If you want to parallel module loading, you need
> to fork.

Hm, right. Looking at that now, there's also the async_probe module
param, but we only need to care about the driver flag. So looks ok then.

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

[igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Revamp fault-injection

[Patchwork]

== Series Details ==

Series: igt/drv_module_reload: Revamp fault-injection
URL   : https://patchwork.freedesktop.org/series/44363/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4282 -> IGTPW_1420 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44363/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload-inject:
      fi-skl-6260u:       PASS -> FAIL
      fi-snb-2600:        PASS -> FAIL
      fi-byt-j1900:       PASS -> FAIL
      fi-kbl-7560u:       PASS -> FAIL
      fi-cnl-y3:          NOTRUN -> FAIL
      fi-kbl-guc:         PASS -> FAIL
      fi-hsw-4200u:       PASS -> FAIL
      {fi-cfl-u2}:        PASS -> FAIL
      fi-skl-6770hq:      PASS -> FAIL
      fi-byt-n2820:       PASS -> FAIL
      fi-bxt-j4205:       PASS -> FAIL
      fi-bwr-2160:        PASS -> FAIL
      fi-kbl-r:           PASS -> FAIL
      fi-cfl-s3:          PASS -> FAIL
      fi-snb-2520m:       PASS -> FAIL
      fi-gdg-551:         PASS -> FAIL
      fi-ilk-650:         PASS -> FAIL
      fi-bsw-n3050:       PASS -> FAIL
      fi-kbl-7567u:       PASS -> FAIL
      fi-glk-j4005:       PASS -> FAIL
      fi-bxt-dsi:         NOTRUN -> FAIL
      fi-ivb-3770:        PASS -> FAIL
      fi-kbl-7500u:       PASS -> FAIL
      fi-ivb-3520m:       PASS -> FAIL
      fi-hsw-4770:        PASS -> FAIL
      fi-bdw-5557u:       PASS -> FAIL
      fi-cfl-8700k:       PASS -> FAIL
      fi-hsw-peppy:       NOTRUN -> FAIL
      fi-skl-6600u:       PASS -> FAIL
      fi-pnv-d510:        PASS -> FAIL
      fi-hsw-4770r:       PASS -> FAIL
      fi-skl-guc:         PASS -> FAIL
      fi-cfl-guc:         PASS -> FAIL
      fi-blb-e6850:       PASS -> FAIL
      fi-skl-6700k2:      PASS -> FAIL
      fi-elk-e7500:       PASS -> FAIL

    
    ==== Warnings ====

    igt@drv_module_reload@basic-reload-inject:
      fi-cnl-psr:         DMESG-WARN (fdo#105395) -> DMESG-FAIL

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_reloc@basic-cpu:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

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

    
    ==== Possible fixes ====

    igt@gem_sync@basic-each:
      fi-cnl-y3:          INCOMPLETE (fdo#105086) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

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

  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000


== Participating hosts (41 -> 38) ==

  Additional (1): fi-hsw-peppy 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4508 -> IGTPW_1420

  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1420: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1420/
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Antonio Argenziano]

On 06/06/18 10:42, Chris Wilson wrote:
> The current method of checking for a failed module load is flawed, as we
> only report the error on probing it is not being reported back by
> modprobe. So we have to dig inside the module_parameters while the
> module is still loaded to discover the error.
> 
> v2: Expect i915.inject_load_failure to be zero on success
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>   tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
>   1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 092982960..e18aaea5e 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -234,6 +234,38 @@ reload(const char *opts_i915)
>   	return err;
>   }
>   

>   static void
>   gem_sanitycheck(void)
>   {
> @@ -323,12 +355,15 @@ igt_main
>   		igt_assert_eq(reload("disable_display=1"), 0);
>   
>   	igt_subtest("basic-reload-inject") {
> -		char buf[64];
>   		int i = 0;
> -		do {
> -			snprintf(buf, sizeof(buf),
> -				 "inject_load_failure=%d", ++i);
> -		} while (reload(buf));
> +
> +		igt_i915_driver_unload();
> +
> +		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> +			;
> +
> +		/* We expect to hit at least one fault! */
> +		igt_assert(i > 1);

I think Michal's patch adds the number of available checkpoints in a 
debugfs, should we trust the driver and assert on: amount of checkpoints 
hit != available checkpoints? Or maybe just spew out a warning.

Thanks,
Antonio

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

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Antonio Argenziano]

On 06/06/18 10:42, Chris Wilson wrote:
> The current method of checking for a failed module load is flawed, as we
> only report the error on probing it is not being reported back by
> modprobe. So we have to dig inside the module_parameters while the
> module is still loaded to discover the error.
> 
> v2: Expect i915.inject_load_failure to be zero on success
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>   tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
>   1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> index 092982960..e18aaea5e 100644
> --- a/tests/drv_module_reload.c
> +++ b/tests/drv_module_reload.c
> @@ -234,6 +234,38 @@ reload(const char *opts_i915)
>   	return err;
>   }
>   

>   static void
>   gem_sanitycheck(void)
>   {
> @@ -323,12 +355,15 @@ igt_main
>   		igt_assert_eq(reload("disable_display=1"), 0);
>   
>   	igt_subtest("basic-reload-inject") {
> -		char buf[64];
>   		int i = 0;
> -		do {
> -			snprintf(buf, sizeof(buf),
> -				 "inject_load_failure=%d", ++i);
> -		} while (reload(buf));
> +
> +		igt_i915_driver_unload();
> +
> +		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> +			;
> +
> +		/* We expect to hit at least one fault! */
> +		igt_assert(i > 1);

I think Michal's patch adds the number of available checkpoints in a 
debugfs, should we trust the driver and assert on: amount of checkpoints 
hit != available checkpoints? Or maybe just spew out a warning.

Thanks,
Antonio

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

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Antonio Argenziano (2018-06-06 21:48:22)
> 
> 
> On 06/06/18 10:42, Chris Wilson wrote:
> > The current method of checking for a failed module load is flawed, as we
> > only report the error on probing it is not being reported back by
> > modprobe. So we have to dig inside the module_parameters while the
> > module is still loaded to discover the error.
> > 
> > v2: Expect i915.inject_load_failure to be zero on success
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >   tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > index 092982960..e18aaea5e 100644
> > --- a/tests/drv_module_reload.c
> > +++ b/tests/drv_module_reload.c
> > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> >       return err;
> >   }
> >   
> 
> >   static void
> >   gem_sanitycheck(void)
> >   {
> > @@ -323,12 +355,15 @@ igt_main
> >               igt_assert_eq(reload("disable_display=1"), 0);
> >   
> >       igt_subtest("basic-reload-inject") {
> > -             char buf[64];
> >               int i = 0;
> > -             do {
> > -                     snprintf(buf, sizeof(buf),
> > -                              "inject_load_failure=%d", ++i);
> > -             } while (reload(buf));
> > +
> > +             igt_i915_driver_unload();
> > +
> > +             while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> > +                     ;
> > +
> > +             /* We expect to hit at least one fault! */
> > +             igt_assert(i > 1);
> 
> I think Michal's patch adds the number of available checkpoints in a 
> debugfs, should we trust the driver and assert on: amount of checkpoints 
> hit != available checkpoints? Or maybe just spew out a warning.

This loop hits all the fault points you can hit. There is nothing more
the driver nor igt can do. The only assertion we have there is to 
basically catch the case where the protocol fails, or there are no fault
points built into the driver.

That is trusting the driver less than expecting it to report the exact
number of reachable fault points.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Antonio Argenziano (2018-06-06 21:48:22)
> 
> 
> On 06/06/18 10:42, Chris Wilson wrote:
> > The current method of checking for a failed module load is flawed, as we
> > only report the error on probing it is not being reported back by
> > modprobe. So we have to dig inside the module_parameters while the
> > module is still loaded to discover the error.
> > 
> > v2: Expect i915.inject_load_failure to be zero on success
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >   tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > index 092982960..e18aaea5e 100644
> > --- a/tests/drv_module_reload.c
> > +++ b/tests/drv_module_reload.c
> > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> >       return err;
> >   }
> >   
> 
> >   static void
> >   gem_sanitycheck(void)
> >   {
> > @@ -323,12 +355,15 @@ igt_main
> >               igt_assert_eq(reload("disable_display=1"), 0);
> >   
> >       igt_subtest("basic-reload-inject") {
> > -             char buf[64];
> >               int i = 0;
> > -             do {
> > -                     snprintf(buf, sizeof(buf),
> > -                              "inject_load_failure=%d", ++i);
> > -             } while (reload(buf));
> > +
> > +             igt_i915_driver_unload();
> > +
> > +             while (inject_fault("i915", "inject_load_failure", ++i) == 0)
> > +                     ;
> > +
> > +             /* We expect to hit at least one fault! */
> > +             igt_assert(i > 1);
> 
> I think Michal's patch adds the number of available checkpoints in a 
> debugfs, should we trust the driver and assert on: amount of checkpoints 
> hit != available checkpoints? Or maybe just spew out a warning.

This loop hits all the fault points you can hit. There is nothing more
the driver nor igt can do. The only assertion we have there is to 
basically catch the case where the protocol fails, or there are no fault
points built into the driver.

That is trusting the driver less than expecting it to report the exact
number of reachable fault points.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Revamp fault-injection

[Patchwork]

== Series Details ==

Series: igt/drv_module_reload: Revamp fault-injection
URL   : https://patchwork.freedesktop.org/series/44363/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4282 -> IGTPW_1423 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44363/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload-inject:
      fi-skl-6260u:       PASS -> FAIL
      fi-snb-2600:        PASS -> FAIL
      fi-byt-j1900:       PASS -> FAIL
      fi-kbl-7560u:       PASS -> FAIL
      fi-kbl-7500u:       PASS -> FAIL
      fi-ivb-3520m:       PASS -> FAIL
      fi-hsw-4770:        PASS -> FAIL
      fi-kbl-guc:         PASS -> FAIL
      fi-hsw-4200u:       PASS -> FAIL
      fi-cfl-u2:          PASS -> FAIL
      fi-skl-6770hq:      PASS -> FAIL
      fi-byt-n2820:       PASS -> FAIL
      fi-bxt-j4205:       PASS -> FAIL
      fi-bwr-2160:        PASS -> FAIL
      fi-kbl-r:           PASS -> FAIL
      fi-bdw-5557u:       PASS -> FAIL
      fi-cfl-s3:          PASS -> FAIL
      fi-snb-2520m:       PASS -> FAIL
      fi-cfl-8700k:       PASS -> FAIL
      fi-gdg-551:         PASS -> FAIL
      fi-skl-6600u:       PASS -> FAIL
      fi-pnv-d510:        PASS -> FAIL
      fi-hsw-4770r:       PASS -> FAIL
      fi-ilk-650:         PASS -> FAIL
      fi-skl-guc:         PASS -> FAIL
      fi-bsw-n3050:       PASS -> FAIL
      fi-cfl-guc:         PASS -> FAIL
      fi-blb-e6850:       PASS -> FAIL
      fi-kbl-7567u:       PASS -> FAIL
      fi-skl-6700k2:      PASS -> FAIL
      fi-elk-e7500:       PASS -> FAIL
      fi-glk-j4005:       PASS -> FAIL
      fi-ivb-3770:        PASS -> FAIL

    
    ==== Warnings ====

    igt@drv_module_reload@basic-reload-inject:
      fi-cnl-psr:         DMESG-WARN (fdo#105395) -> DMESG-FAIL

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128)

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@gem_sync@basic-many-each:
      fi-cnl-y3:          NOTRUN -> INCOMPLETE (fdo#105086)

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

    igt@prime_vgem@basic-fence-flip:
      fi-kbl-7500u:       PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@gem_sync@basic-each:
      fi-cnl-y3:          INCOMPLETE (fdo#105086) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000


== Participating hosts (41 -> 37) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4508 -> IGTPW_1423

  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1423: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1423/
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] ✗ Fi.CI.BAT: failure for igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

Quoting Patchwork (2018-06-07 11:49:52)
> == Series Details ==
> 
> Series: igt/drv_module_reload: Revamp fault-injection
> URL   : https://patchwork.freedesktop.org/series/44363/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4282 -> IGTPW_1423 =
> 
> == Summary - FAILURE ==
> 
>   Serious unknown changes coming with IGTPW_1423 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in IGTPW_1423, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/44363/revisions/1/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in IGTPW_1423:
> 
>   === IGT changes ===
> 
>     ==== Possible regressions ====
> 
>     igt@drv_module_reload@basic-reload-inject:
>       fi-skl-6260u:       PASS -> FAIL
>       fi-snb-2600:        PASS -> FAIL
>       fi-byt-j1900:       PASS -> FAIL
>       fi-kbl-7560u:       PASS -> FAIL
>       fi-kbl-7500u:       PASS -> FAIL
>       fi-ivb-3520m:       PASS -> FAIL
>       fi-hsw-4770:        PASS -> FAIL
>       fi-kbl-guc:         PASS -> FAIL
>       fi-hsw-4200u:       PASS -> FAIL
>       fi-cfl-u2:          PASS -> FAIL
>       fi-skl-6770hq:      PASS -> FAIL
>       fi-byt-n2820:       PASS -> FAIL
>       fi-bxt-j4205:       PASS -> FAIL
>       fi-bwr-2160:        PASS -> FAIL
>       fi-kbl-r:           PASS -> FAIL
>       fi-bdw-5557u:       PASS -> FAIL
>       fi-cfl-s3:          PASS -> FAIL
>       fi-snb-2520m:       PASS -> FAIL
>       fi-cfl-8700k:       PASS -> FAIL
>       fi-gdg-551:         PASS -> FAIL
>       fi-skl-6600u:       PASS -> FAIL
>       fi-pnv-d510:        PASS -> FAIL
>       fi-hsw-4770r:       PASS -> FAIL
>       fi-ilk-650:         PASS -> FAIL
>       fi-skl-guc:         PASS -> FAIL
>       fi-bsw-n3050:       PASS -> FAIL
>       fi-cfl-guc:         PASS -> FAIL
>       fi-blb-e6850:       PASS -> FAIL
>       fi-kbl-7567u:       PASS -> FAIL
>       fi-skl-6700k2:      PASS -> FAIL
>       fi-elk-e7500:       PASS -> FAIL
>       fi-glk-j4005:       PASS -> FAIL
>       fi-ivb-3770:        PASS -> FAIL

Huh? When in doubt resend...
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for igt/drv_module_reload: Revamp fault-injection

[Patchwork]

== Series Details ==

Series: igt/drv_module_reload: Revamp fault-injection
URL   : https://patchwork.freedesktop.org/series/44363/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4508_full -> IGTPW_1423_full =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/44363/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-reload-inject:
      shard-glk:          PASS -> FAIL
      shard-snb:          PASS -> FAIL
      shard-hsw:          PASS -> FAIL
      shard-kbl:          PASS -> FAIL
      shard-apl:          PASS -> FAIL

    
    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_cursor_legacy@2x-flip-vs-cursor-legacy:
      shard-glk:          PASS -> FAIL (fdo#104873) +1

    igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
      shard-hsw:          PASS -> FAIL (fdo#104873)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +2

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

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

    igt@perf_pmu@busy-rcs0:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411) +1

    igt@pm_rps@reset:
      shard-kbl:          PASS -> FAIL (fdo#102250)

    
    ==== Possible fixes ====

    igt@gem_eio@suspend:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_exec_basic@gtt-vebox:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack:
      shard-glk:          FAIL (fdo#103167, fdo#104724) -> PASS

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

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

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4508 -> IGTPW_1423
    * Linux: CI_DRM_4280 -> CI_DRM_4282

  CI_DRM_4280: 967aa2f22752af3adc629b50e7d2ed2b7e061e44 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4282: c1064b9be065603680d060184da1a93d404dcf0c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1423: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1423/
  IGT_4508: 78a68c905066beeefd24b3a4518d817a811e8798 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success
v3: Do a full i915 unload to ensure fbdev is unbound in cases where it
managed to bind itself before failure injection.
v4: Disable the display (temporary) to minimally fix up the test case
before more regressions slip by.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> #v3
---
 tests/drv_module_reload.c | 52 +++++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 3046d8227..bb26368e7 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,45 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+#define XXX_NO_DISPLAY "disable_display=1 "
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), XXX_NO_DISPLAY "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_debug("Loaded '%s %s', result=%d\n", module_name, buf, fault);
+
+	if (strcmp(module_name, "i915")) /* XXX better ideas! */
+		igt_kmod_unload(module_name, 0);
+	else
+		igt_i915_driver_unload();
+
+	return fault;
+#undef XXX_NO_DISPLAY
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -320,12 +359,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.18.0

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

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success
v3: Do a full i915 unload to ensure fbdev is unbound in cases where it
managed to bind itself before failure injection.
v4: Disable the display (temporary) to minimally fix up the test case
before more regressions slip by.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> #v3
---
 tests/drv_module_reload.c | 52 +++++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 3046d8227..652bb3272 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,45 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+#define XXX_NO_DISPLAY "disable_display=1"
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), XXX_NO_DISPLAY "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_debug("Loaded '%s %s', result=%d\n", module_name, buf, fault);
+
+	if (strcmp(module_name, "i915")) /* XXX better ideas! */
+		igt_kmod_unload(module_name, 0);
+	else
+		igt_i915_driver_unload();
+
+	return fault;
+#undef XXX_NO_DISPLAY
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -320,12 +359,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.18.0

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

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 tests/drv_module_reload.c | 47 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 3046d8227..57e5b50ec 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,40 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_debug("Loaded '%s %s', result=%d\n", module_name, buf, fault);
+
+	igt_kmod_unload(module_name, 0);
+
+	return fault;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -320,12 +354,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.18.0

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

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 tests/drv_module_reload.c | 47 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 3046d8227..57e5b50ec 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,40 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_debug("Loaded '%s %s', result=%d\n", module_name, buf, fault);
+
+	igt_kmod_unload(module_name, 0);
+
+	return fault;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -320,12 +354,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.17.1

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

[PATCH i-g-t] igt/drv_module_reload: Revamp fault-injection

[Chris Wilson]

The current method of checking for a failed module load is flawed, as we
only report the error on probing it is not being reported back by
modprobe. So we have to dig inside the module_parameters while the
module is still loaded to discover the error.

v2: Expect i915.inject_load_failure to be zero on success

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
---
 tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
index 3046d8227..5d6680b0b 100644
--- a/tests/drv_module_reload.c
+++ b/tests/drv_module_reload.c
@@ -234,6 +234,38 @@ reload(const char *opts_i915)
 	return err;
 }
 
+static int open_parameters(const char *module_name)
+{
+	char path[256];
+
+	snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
+	return open(path, O_RDONLY);
+}
+
+static int
+inject_fault(const char *module_name, const char *opt, int fault)
+{
+	char buf[1024];
+	int dir;
+
+	igt_assert(fault > 0);
+	snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
+
+	if (igt_kmod_load(module_name, buf)) {
+		igt_warn("Failed to load module '%s' with options '%s'\n",
+			 module_name, buf);
+		return 1;
+	}
+
+	dir = open_parameters(module_name);
+	igt_sysfs_scanf(dir, opt, "%d", &fault);
+	close(dir);
+
+	igt_kmod_unload(module_name, 0);
+
+	return fault;
+}
+
 static void
 gem_sanitycheck(void)
 {
@@ -320,12 +352,15 @@ igt_main
 		igt_assert_eq(reload("disable_display=1"), 0);
 
 	igt_subtest("basic-reload-inject") {
-		char buf[64];
 		int i = 0;
-		do {
-			snprintf(buf, sizeof(buf),
-				 "inject_load_failure=%d", ++i);
-		} while (reload(buf));
+
+		igt_i915_driver_unload();
+
+		while (inject_fault("i915", "inject_load_failure", ++i) == 0)
+			;
+
+		/* We expect to hit at least one fault! */
+		igt_assert(i > 1);
 	}
 
 	igt_fixture {
-- 
2.17.1

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