diff mbox series

[i-g-t,3/3] igt/gem_sync: Exercising waiting while keeping the GPU busy

Message ID 20180810110149.10771-3-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [i-g-t,1/3] igt/gem_sync: Exercise sync after context switch | expand

Commit Message

Chris Wilson Aug. 10, 2018, 11:01 a.m. UTC
Normally we wait on the last request, but that overlooks any
difficulties in waiting on a request while the next is being qeued.
Check those.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/gem_sync.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

Antonio Argenziano Aug. 10, 2018, 5:41 p.m. UTC | #1
On 10/08/18 04:01, Chris Wilson wrote:
> Normally we wait on the last request, but that overlooks any
> difficulties in waiting on a request while the next is being qeued.

/s/qeued/queued

> Check those.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   tests/gem_sync.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
> 
> diff --git a/tests/gem_sync.c b/tests/gem_sync.c
> index c697220ad..fb209977d 100644
> --- a/tests/gem_sync.c
> +++ b/tests/gem_sync.c
> @@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
>   	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
>   }
>   

> +	intel_detect_and_clear_missed_interrupts(fd);
> +	igt_fork(child, num_engines) {
> +		double start, end, elapsed;
> +		unsigned long cycles;
> +		igt_spin_t *spin[2];
> +		uint32_t cmd;
> +
> +		spin[0] = __igt_spin_batch_new(fd,
> +					       .engine = ring,
> +					       .flags = IGT_SPIN_FAST);
> +		cmd = *spin[0]->batch;
> +
> +		spin[1] = __igt_spin_batch_new(fd,
> +					       .engine = ring,
> +					       .flags = IGT_SPIN_FAST);
> +		igt_assert(*spin[1]->batch == cmd);
> +
> +		start = gettime();
> +		end = start + timeout;
> +		cycles = 0;
> +		do {
> +			for (int loop = 0; loop < 1024; loop++) {
> +				igt_spin_t *s = spin[loop & 1];
> +
> +				igt_spin_batch_end(s);
> +				gem_sync(fd, s->handle);

How does the test fail if the sync goes wrong? Hang detector on the 
queued batch?

Antonio

> +
> +				*s->batch = cmd;
> +				gem_execbuf(fd, &s->execbuf);
> +			}
> +			cycles += 1024;
> +		} while ((elapsed = gettime()) < end);
> +		igt_spin_batch_free(fd, spin[1]);
> +		igt_spin_batch_free(fd, spin[0]);
> +
> +		igt_info("%s%sompleted %ld cycles: %.3f us\n",
> +			 names[child % num_engines] ?: "",
> +			 names[child % num_engines] ? " c" : "C",
> +			 cycles, (elapsed - start)*1e6/cycles);
> +	}
> +	igt_waitchildren_timeout(2*timeout, NULL);
> +	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> +}
> +
Chris Wilson Aug. 10, 2018, 5:51 p.m. UTC | #2
Quoting Antonio Argenziano (2018-08-10 18:41:22)
> 
> 
> On 10/08/18 04:01, Chris Wilson wrote:
> > Normally we wait on the last request, but that overlooks any
> > difficulties in waiting on a request while the next is being qeued.
> 
> /s/qeued/queued
> 
> > Check those.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   tests/gem_sync.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 72 insertions(+)
> > 
> > diff --git a/tests/gem_sync.c b/tests/gem_sync.c
> > index c697220ad..fb209977d 100644
> > --- a/tests/gem_sync.c
> > +++ b/tests/gem_sync.c
> > @@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
> >       igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
> >   }
> >   
> 
> > +     intel_detect_and_clear_missed_interrupts(fd);
> > +     igt_fork(child, num_engines) {
> > +             double start, end, elapsed;
> > +             unsigned long cycles;
> > +             igt_spin_t *spin[2];
> > +             uint32_t cmd;
> > +
> > +             spin[0] = __igt_spin_batch_new(fd,
> > +                                            .engine = ring,
> > +                                            .flags = IGT_SPIN_FAST);
> > +             cmd = *spin[0]->batch;
> > +
> > +             spin[1] = __igt_spin_batch_new(fd,
> > +                                            .engine = ring,
> > +                                            .flags = IGT_SPIN_FAST);
> > +             igt_assert(*spin[1]->batch == cmd);
> > +
> > +             start = gettime();
> > +             end = start + timeout;
> > +             cycles = 0;
> > +             do {
> > +                     for (int loop = 0; loop < 1024; loop++) {
> > +                             igt_spin_t *s = spin[loop & 1];
> > +
> > +                             igt_spin_batch_end(s);
> > +                             gem_sync(fd, s->handle);
> 
> How does the test fail if the sync goes wrong? Hang detector on the 
> queued batch?

We have a hang detector for both missed wakeups and GPU hangs. As tests
goes it's fairly tame, but in essence this entire file is about trying
to trick the HW+driver into not sending an interrupt back to userspace.
Just a very narrow stress test, over and over again from slightly
different angles.
-Chris
Antonio Argenziano Aug. 10, 2018, 6:11 p.m. UTC | #3
On 10/08/18 10:51, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-08-10 18:41:22)
>>
>>
>> On 10/08/18 04:01, Chris Wilson wrote:
>>> Normally we wait on the last request, but that overlooks any
>>> difficulties in waiting on a request while the next is being qeued.
>>
>> /s/qeued/queued
>>
>>> Check those.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    tests/gem_sync.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 72 insertions(+)
>>>
>>> diff --git a/tests/gem_sync.c b/tests/gem_sync.c
>>> index c697220ad..fb209977d 100644
>>> --- a/tests/gem_sync.c
>>> +++ b/tests/gem_sync.c
>>> @@ -294,6 +294,74 @@ wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
>>>        igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
>>>    }
>>>    
>>
>>> +     intel_detect_and_clear_missed_interrupts(fd);
>>> +     igt_fork(child, num_engines) {
>>> +             double start, end, elapsed;
>>> +             unsigned long cycles;
>>> +             igt_spin_t *spin[2];
>>> +             uint32_t cmd;
>>> +
>>> +             spin[0] = __igt_spin_batch_new(fd,
>>> +                                            .engine = ring,
>>> +                                            .flags = IGT_SPIN_FAST);
>>> +             cmd = *spin[0]->batch;
>>> +
>>> +             spin[1] = __igt_spin_batch_new(fd,
>>> +                                            .engine = ring,
>>> +                                            .flags = IGT_SPIN_FAST);
>>> +             igt_assert(*spin[1]->batch == cmd);
>>> +
>>> +             start = gettime();
>>> +             end = start + timeout;
>>> +             cycles = 0;
>>> +             do {
>>> +                     for (int loop = 0; loop < 1024; loop++) {
>>> +                             igt_spin_t *s = spin[loop & 1];
>>> +
>>> +                             igt_spin_batch_end(s);
>>> +                             gem_sync(fd, s->handle);
>>
>> How does the test fail if the sync goes wrong? Hang detector on the
>> queued batch?
> 
> We have a hang detector for both missed wakeups and GPU hangs. As tests
> goes it's fairly tame, but in essence this entire file is about trying
> to trick the HW+driver into not sending an interrupt back to userspace.
> Just a very narrow stress test, over and over again from slightly
> different angles.

I see.

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> -Chris
>
Chris Wilson Aug. 14, 2018, 6:27 p.m. UTC | #4
Quoting Antonio Argenziano (2018-08-10 19:11:02)
> 
> 
> On 10/08/18 10:51, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-08-10 18:41:22)
> >> How does the test fail if the sync goes wrong? Hang detector on the
> >> queued batch?
> > 
> > We have a hang detector for both missed wakeups and GPU hangs. As tests
> > goes it's fairly tame, but in essence this entire file is about trying
> > to trick the HW+driver into not sending an interrupt back to userspace.
> > Just a very narrow stress test, over and over again from slightly
> > different angles.
> 
> I see.
> 
> Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

Was that a general r-b for the very similar series or just this last
patch?
-Chris
Antonio Argenziano Aug. 14, 2018, 6:31 p.m. UTC | #5
On 14/08/18 11:27, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-08-10 19:11:02)
>>
>>
>> On 10/08/18 10:51, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-08-10 18:41:22)
>>>> How does the test fail if the sync goes wrong? Hang detector on the
>>>> queued batch?
>>>
>>> We have a hang detector for both missed wakeups and GPU hangs. As tests
>>> goes it's fairly tame, but in essence this entire file is about trying
>>> to trick the HW+driver into not sending an interrupt back to userspace.
>>> Just a very narrow stress test, over and over again from slightly
>>> different angles.
>>
>> I see.
>>
>> Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>
> 
> Was that a general r-b for the very similar series or just this last
> patch?

I've only read this last patch, I'll have a look at the rest.

Antonio

> -Chris
>
diff mbox series

Patch

diff --git a/tests/gem_sync.c b/tests/gem_sync.c
index c697220ad..fb209977d 100644
--- a/tests/gem_sync.c
+++ b/tests/gem_sync.c
@@ -294,6 +294,74 @@  wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
 }
 
+static void active_ring(int fd, unsigned ring, int timeout)
+{
+	unsigned engines[16];
+	const char *names[16];
+	int num_engines = 0;
+
+	if (ring == ALL_ENGINES) {
+		for_each_physical_engine(fd, ring) {
+			if (!gem_can_store_dword(fd, ring))
+				continue;
+
+			names[num_engines] = e__->name;
+			engines[num_engines++] = ring;
+			if (num_engines == ARRAY_SIZE(engines))
+				break;
+		}
+		igt_require(num_engines);
+	} else {
+		gem_require_ring(fd, ring);
+		igt_require(gem_can_store_dword(fd, ring));
+		names[num_engines] = NULL;
+		engines[num_engines++] = ring;
+	}
+
+	intel_detect_and_clear_missed_interrupts(fd);
+	igt_fork(child, num_engines) {
+		double start, end, elapsed;
+		unsigned long cycles;
+		igt_spin_t *spin[2];
+		uint32_t cmd;
+
+		spin[0] = __igt_spin_batch_new(fd,
+					       .engine = ring,
+					       .flags = IGT_SPIN_FAST);
+		cmd = *spin[0]->batch;
+
+		spin[1] = __igt_spin_batch_new(fd,
+					       .engine = ring,
+					       .flags = IGT_SPIN_FAST);
+		igt_assert(*spin[1]->batch == cmd);
+
+		start = gettime();
+		end = start + timeout;
+		cycles = 0;
+		do {
+			for (int loop = 0; loop < 1024; loop++) {
+				igt_spin_t *s = spin[loop & 1];
+
+				igt_spin_batch_end(s);
+				gem_sync(fd, s->handle);
+
+				*s->batch = cmd;
+				gem_execbuf(fd, &s->execbuf);
+			}
+			cycles += 1024;
+		} while ((elapsed = gettime()) < end);
+		igt_spin_batch_free(fd, spin[1]);
+		igt_spin_batch_free(fd, spin[0]);
+
+		igt_info("%s%sompleted %ld cycles: %.3f us\n",
+			 names[child % num_engines] ?: "",
+			 names[child % num_engines] ? " c" : "C",
+			 cycles, (elapsed - start)*1e6/cycles);
+	}
+	igt_waitchildren_timeout(2*timeout, NULL);
+	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
 static void
 active_wakeup_ring(int fd, unsigned ring, int timeout, int wlen)
 {
@@ -1154,6 +1222,8 @@  igt_main
 			sync_ring(fd, e->exec_id | e->flags, 1, 150);
 		igt_subtest_f("idle-%s", e->name)
 			idle_ring(fd, e->exec_id | e->flags, 150);
+		igt_subtest_f("active-%s", e->name)
+			active_ring(fd, e->exec_id | e->flags, 150);
 		igt_subtest_f("wakeup-%s", e->name)
 			wakeup_ring(fd, e->exec_id | e->flags, 150, 1);
 		igt_subtest_f("active-wakeup-%s", e->name)
@@ -1188,6 +1258,8 @@  igt_main
 		sync_ring(fd, ALL_ENGINES, ncpus, 150);
 	igt_subtest("forked-store-each")
 		store_ring(fd, ALL_ENGINES, ncpus, 150);
+	igt_subtest("active-each")
+		active_ring(fd, ALL_ENGINES, 150);
 	igt_subtest("wakeup-each")
 		wakeup_ring(fd, ALL_ENGINES, 150, 1);
 	igt_subtest("active-wakeup-each")