[PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[Chris Wilson]

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

[PATCH igt 2/2] lib/core: Don't leak dummyloads between subtests

[Chris Wilson]

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>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 777687b5f..558a538d3 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 "version.h"
 #include "config.h"
 
@@ -994,6 +995,8 @@ static void exit_subtest(const char *result)
 		 (!__igt_plain_output) ? "\x1b[0m" : "");
 	fflush(stdout);
 
+	igt_terminate_spin_batches();
+
 	in_subtest = NULL;
 	siglongjmp(igt_subtest_jmpbuf, 1);
 }
@@ -1817,6 +1820,8 @@ static void call_exit_handlers(int sig)
 {
 	int i;
 
+	igt_terminate_spin_batches();
+
 	if (!exit_handler_count) {
 		return;
 	}
-- 
2.15.1

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

Re: [PATCH igt 2/2] lib/core: Don't leak dummyloads between subtests

[Tvrtko Ursulin]

On 12/12/2017 12:21, Chris Wilson wrote:
> 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>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   lib/igt_core.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 777687b5f..558a538d3 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 "version.h"
>   #include "config.h"
>   
> @@ -994,6 +995,8 @@ static void exit_subtest(const char *result)
>   		 (!__igt_plain_output) ? "\x1b[0m" : "");
>   	fflush(stdout);
>   
> +	igt_terminate_spin_batches();
> +
>   	in_subtest = NULL;
>   	siglongjmp(igt_subtest_jmpbuf, 1);
>   }
> @@ -1817,6 +1820,8 @@ static void call_exit_handlers(int sig)
>   {
>   	int i;
>   
> +	igt_terminate_spin_batches();
> +
>   	if (!exit_handler_count) {
>   		return;
>   	}
> 

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

✓ Fi.CI.BAT: success for series starting with [1/2] lib/dummyload: Wrap global list inside its own mutex

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib/dummyload: Wrap global list inside its own mutex
URL   : https://patchwork.freedesktop.org/series/35225/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
74407418720ff7a9de7caabec05d4c3afe9a5c51 igt/kms_flip: Allow very large bo to fail pageflips with E2BIG

with latest DRM-Tip kernel build CI_DRM_3501
f49a92217adc drm-tip: 2017y-12m-12d-12h-12m-26s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-glk-1) fdo#103326
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103326 https://bugs.freedesktop.org/show_bug.cgi?id=103326
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:440s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:387s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:518s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:507s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:511s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:488s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:271s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:360s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:259s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:394s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:478s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:524s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:530s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:596s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:514s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:493s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:452s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:424s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:601s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:624s

== Logs ==

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

Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[Tvrtko Ursulin]

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

Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[Chris Wilson]

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

Re: [PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[Tvrtko Ursulin]

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

✗ Fi.CI.IGT: warning for series starting with [1/2] lib/dummyload: Wrap global list inside its own mutex

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib/dummyload: Wrap global list inside its own mutex
URL   : https://patchwork.freedesktop.org/series/35225/
State : warning

== Summary ==

Warning: bzip IGTPW_654/shard-hsw8/results12.json.bz2 wasn't in correct JSON format
Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-snb) fdo#104009
Test drv_suspend:
        Subgroup debugfs-reader-hibernate:
                fail       -> SKIP       (shard-hsw) fdo#103375 +1
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623
Test gem_mmap_wc:
        Subgroup set-cache-level:
                pass       -> SKIP       (shard-snb)

fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2636 pass:1493 dwarn:1   dfail:0   fail:9   skip:1133 time:9282s
shard-snb        total:2712 pass:1307 dwarn:1   dfail:0   fail:12  skip:1392 time:8074s
Blacklisted hosts:
shard-apl        total:2614 pass:1616 dwarn:1   dfail:0   fail:24  skip:971 time:13102s
shard-kbl        total:2635 pass:1757 dwarn:1   dfail:0   fail:21  skip:856 time:10826s

== Logs ==

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

[PATCH igt 1/2] lib/dummyload: Wrap global list inside its own mutex

[Chris Wilson]

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