* [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex
@ 2017-08-09 12:15 Chris Wilson
2017-08-09 12:15 ` [PATCH igt 2/2] lib/core: Don't leak dummyloads between subtests Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-08-09 12:15 UTC (permalink / raw)
To: intel-gfx
Give the list a mutex, for we try to iterate over it from many a random
context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_dummyload.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 5ad386a5..5d654825 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -50,6 +50,7 @@
static const int BATCH_SIZE = 4096;
static IGT_LIST(spin_list);
+static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
static void
fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
@@ -162,7 +163,9 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
emit_recursive_batch(spin, fd, ctx, engine, dep);
igt_assert(gem_bo_busy(fd, spin->handle));
+ pthread_mutex_lock(&list_lock);
igt_list_add(&spin->link, &spin_list);
+ pthread_mutex_unlock(&list_lock);
return spin;
}
@@ -261,7 +264,9 @@ void igt_spin_batch_free(int fd, igt_spin_t *spin)
if (!spin)
return;
+ pthread_mutex_lock(&list_lock);
igt_list_del(&spin->link);
+ pthread_mutex_unlock(&list_lock);
if (spin->timer)
timer_delete(spin->timer);
@@ -277,6 +282,8 @@ void igt_terminate_spin_batches(void)
{
struct igt_spin *iter;
+ pthread_mutex_lock(&list_lock);
igt_list_for_each(iter, &spin_list, link)
igt_spin_batch_end(iter);
+ pthread_mutex_unlock(&list_lock);
}
--
2.13.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH igt 2/2] lib/core: Don't leak dummyloads between subtests
2017-08-09 12:15 [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex Chris Wilson
@ 2017-08-09 12:15 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-08-09 12:15 UTC (permalink / raw)
To: intel-gfx
If a test fails or skips early, it may not clean up after itself. In
lieu of having a framework for test deconstructors, hook
igt_terminate_spin_batches() into exit_subtest() itself so that we don't
allow a recursive batch from an earlier test to leak into the next and
cause an unexpected GPU hang.
Similarly, we also want to terminate the dummyload as the first step in
our atexit handlers (currently it is at the start of the last step) as
some atexit handlers may be unwittingly exposed to dummyloads and so
cause another wait on GPU hang.
We trust that the core already distinguishes correctly between the
principal test process and its children.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index e1cab46f..77402267 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -63,6 +63,7 @@
#include "intel_chipset.h"
#include "intel_io.h"
#include "igt_debugfs.h"
+#include "igt_dummyload.h"
#include "igt_ftrace.h"
#include "igt_kcov.h"
#include "version.h"
@@ -1068,6 +1069,8 @@ static void exit_subtest(const char *result)
}
}
+ igt_terminate_spin_batches();
+
in_subtest = NULL;
siglongjmp(igt_subtest_jmpbuf, 1);
}
@@ -1872,6 +1875,8 @@ static void call_exit_handlers(int sig)
{
int i;
+ igt_terminate_spin_batches();
+
if (!exit_handler_count) {
return;
}
--
2.13.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex
2017-12-12 15:35 ` Chris Wilson
@ 2017-12-12 15:57 ` Tvrtko Ursulin
0 siblings, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2017-12-12 15:57 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 12/12/2017 15:35, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-12-12 15:19:03)
>>
>> On 12/12/2017 12:21, Chris Wilson wrote:
>>> Give the list a mutex, for we try to iterate over it from many a random
>>> context.
>>
>> Isn't it only tests and their exit handlers, so already serialized?
>
> No, it can be used from threads as well.
>
>> Should spin batches instead install an exit handler?
>
> Problem being ordering. Since other exit handlers may depend upon GPU
> activity, cancelling that GPU activity has to be done first. If we
> install an exit handler for an early spin batch, that will be run last,
> causing a GPU hang in the other handlers.
>
> Furthermore, the exit handlers are not run after subtests; there is no
> setup/teardown procedure other than trying to fit such into
> igt_subtest_group + igt_fixture, which isn't always feasible.
Okay, I am convinced. :)
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex
2017-12-12 15:19 ` Tvrtko Ursulin
@ 2017-12-12 15:35 ` Chris Wilson
2017-12-12 15:57 ` Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-12-12 15:35 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2017-12-12 15:19:03)
>
> On 12/12/2017 12:21, Chris Wilson wrote:
> > Give the list a mutex, for we try to iterate over it from many a random
> > context.
>
> Isn't it only tests and their exit handlers, so already serialized?
No, it can be used from threads as well.
> Should spin batches instead install an exit handler?
Problem being ordering. Since other exit handlers may depend upon GPU
activity, cancelling that GPU activity has to be done first. If we
install an exit handler for an early spin batch, that will be run last,
causing a GPU hang in the other handlers.
Furthermore, the exit handlers are not run after subtests; there is no
setup/teardown procedure other than trying to fit such into
igt_subtest_group + igt_fixture, which isn't always feasible.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex
2017-12-12 12:21 [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex Chris Wilson
@ 2017-12-12 15:19 ` Tvrtko Ursulin
2017-12-12 15:35 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2017-12-12 15:19 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 12/12/2017 12:21, Chris Wilson wrote:
> Give the list a mutex, for we try to iterate over it from many a random
> context.
Isn't it only tests and their exit handlers, so already serialized?
Should spin batches instead install an exit handler?
Regards,
Tvrtko
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> lib/igt_dummyload.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index bb2be557a..d19b4e5ea 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -57,6 +57,7 @@
>
> static const int BATCH_SIZE = 4096;
> static IGT_LIST(spin_list);
> +static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
>
> static void
> fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
> @@ -182,7 +183,9 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
> emit_recursive_batch(spin, fd, ctx, engine, dep);
> igt_assert(gem_bo_busy(fd, spin->handle));
>
> + pthread_mutex_lock(&list_lock);
> igt_list_add(&spin->link, &spin_list);
> + pthread_mutex_unlock(&list_lock);
>
> return spin;
> }
> @@ -281,7 +284,9 @@ void igt_spin_batch_free(int fd, igt_spin_t *spin)
> if (!spin)
> return;
>
> + pthread_mutex_lock(&list_lock);
> igt_list_del(&spin->link);
> + pthread_mutex_unlock(&list_lock);
>
> if (spin->timer)
> timer_delete(spin->timer);
> @@ -297,6 +302,8 @@ void igt_terminate_spin_batches(void)
> {
> struct igt_spin *iter;
>
> + pthread_mutex_lock(&list_lock);
> igt_list_for_each(iter, &spin_list, link)
> igt_spin_batch_end(iter);
> + pthread_mutex_unlock(&list_lock);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex
@ 2017-12-12 12:21 Chris Wilson
2017-12-12 15:19 ` Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-12-12 12:21 UTC (permalink / raw)
To: intel-gfx
Give the list a mutex, for we try to iterate over it from many a random
context.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_dummyload.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index bb2be557a..d19b4e5ea 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -57,6 +57,7 @@
static const int BATCH_SIZE = 4096;
static IGT_LIST(spin_list);
+static pthread_mutex_t list_lock = PTHREAD_MUTEX_INITIALIZER;
static void
fill_reloc(struct drm_i915_gem_relocation_entry *reloc,
@@ -182,7 +183,9 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
emit_recursive_batch(spin, fd, ctx, engine, dep);
igt_assert(gem_bo_busy(fd, spin->handle));
+ pthread_mutex_lock(&list_lock);
igt_list_add(&spin->link, &spin_list);
+ pthread_mutex_unlock(&list_lock);
return spin;
}
@@ -281,7 +284,9 @@ void igt_spin_batch_free(int fd, igt_spin_t *spin)
if (!spin)
return;
+ pthread_mutex_lock(&list_lock);
igt_list_del(&spin->link);
+ pthread_mutex_unlock(&list_lock);
if (spin->timer)
timer_delete(spin->timer);
@@ -297,6 +302,8 @@ void igt_terminate_spin_batches(void)
{
struct igt_spin *iter;
+ pthread_mutex_lock(&list_lock);
igt_list_for_each(iter, &spin_list, link)
igt_spin_batch_end(iter);
+ pthread_mutex_unlock(&list_lock);
}
--
2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-12 15:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 12:15 [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex Chris Wilson
2017-08-09 12:15 ` [PATCH igt 2/2] lib/core: Don't leak dummyloads between subtests Chris Wilson
2017-12-12 12:21 [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex Chris Wilson
2017-12-12 15:19 ` Tvrtko Ursulin
2017-12-12 15:35 ` Chris Wilson
2017-12-12 15:57 ` Tvrtko Ursulin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.