* [Intel-gfx] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 17:42 ` Chris Wilson
0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2018-06-06 17:42 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
2018-06-06 17:42 ` [Intel-gfx] " Chris Wilson
@ 2018-06-06 18:00 ` Imre Deak
-1 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2018-06-06 18:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 18:00 ` Imre Deak
0 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2018-06-06 18:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
2018-06-06 18:00 ` [igt-dev] " Imre Deak
@ 2018-06-06 18:04 ` Chris Wilson
-1 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2018-06-06 18:04 UTC (permalink / raw)
To: imre.deak; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 18:04 ` Chris Wilson
0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2018-06-06 18:04 UTC (permalink / raw)
To: imre.deak, Imre Deak; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
2018-06-06 18:04 ` [Intel-gfx] " Chris Wilson
@ 2018-06-06 18:15 ` Imre Deak
-1 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2018-06-06 18:15 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 18:15 ` Imre Deak
0 siblings, 0 replies; 19+ messages in thread
From: Imre Deak @ 2018-06-06 18:15 UTC (permalink / raw)
To: Chris Wilson; +Cc: igt-dev, intel-gfx
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
2018-06-06 17:42 ` [Intel-gfx] " Chris Wilson
@ 2018-06-06 20:48 ` Antonio Argenziano
-1 siblings, 0 replies; 19+ messages in thread
From: Antonio Argenziano @ 2018-06-06 20:48 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 20:48 ` Antonio Argenziano
0 siblings, 0 replies; 19+ messages in thread
From: Antonio Argenziano @ 2018-06-06 20:48 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: igt-dev
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
2018-06-06 20:48 ` Antonio Argenziano
@ 2018-06-06 20:54 ` Chris Wilson
-1 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2018-06-06 20:54 UTC (permalink / raw)
To: Antonio Argenziano, intel-gfx; +Cc: igt-dev
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection
@ 2018-06-06 20:54 ` Chris Wilson
0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2018-06-06 20:54 UTC (permalink / raw)
To: Antonio Argenziano, intel-gfx; +Cc: igt-dev
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
^ permalink raw reply [flat|nested] 19+ messages in thread