| Message ID | 20180501090712.31892-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/05/2018 10:07, Chris Wilson wrote: > Our parent is RT, we are not. In theory, we should wait until our parent > has gone to sleep before we are allowed to proceed (we should both be > bound to the same cpu). Double down on this by sleeping in the child > until our parent has written a byte along a pipe(). > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/drv_missed_irq.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/tests/drv_missed_irq.c b/tests/drv_missed_irq.c > index 9326a5a62..791ee51fb 100644 > --- a/tests/drv_missed_irq.c > +++ b/tests/drv_missed_irq.c > @@ -34,20 +34,29 @@ IGT_TEST_DESCRIPTION("Inject missed interrupts and make sure they are caught"); > static void trigger_missed_interrupt(int fd, unsigned ring) > { > igt_spin_t *spin = __igt_spin_batch_new(fd, 0, ring, 0); > + uint32_t go; > + int link[2]; > + > + igt_assert(pipe(link) == 0); > > igt_fork(child, 1) { > - /* We are now a low priority child on the *same* CPU as the > + /* > + * We are now a low priority child on the *same* CPU as the > * parent. We will have to wait for our parent to sleep > * (gem_sync -> i915_wait_request) before we run. > */ > + read(link[0], &go, sizeof(go)); > igt_assert(gem_bo_busy(fd, spin->handle)); > igt_spin_batch_end(spin); > } > > + write(link[1], &go, sizeof(go)); > gem_sync(fd, spin->handle); > igt_waitchildren(); > > igt_spin_batch_free(fd, spin); > + close(link[1]); > + close(link[0]); > } > > static void bind_to_cpu(int cpu) > Does the parent need to be RT at all now? Would it work to use a short(er) timed wait and signal back to the child to terminate the spinner only then? Fake missing interrupt only needs some wait, right? Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-05-01 12:10:22) > > On 01/05/2018 10:07, Chris Wilson wrote: > > Our parent is RT, we are not. In theory, we should wait until our parent > > has gone to sleep before we are allowed to proceed (we should both be > > bound to the same cpu). Double down on this by sleeping in the child > > until our parent has written a byte along a pipe(). > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > tests/drv_missed_irq.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/tests/drv_missed_irq.c b/tests/drv_missed_irq.c > > index 9326a5a62..791ee51fb 100644 > > --- a/tests/drv_missed_irq.c > > +++ b/tests/drv_missed_irq.c > > @@ -34,20 +34,29 @@ IGT_TEST_DESCRIPTION("Inject missed interrupts and make sure they are caught"); > > static void trigger_missed_interrupt(int fd, unsigned ring) > > { > > igt_spin_t *spin = __igt_spin_batch_new(fd, 0, ring, 0); > > + uint32_t go; > > + int link[2]; > > + > > + igt_assert(pipe(link) == 0); > > > > igt_fork(child, 1) { > > - /* We are now a low priority child on the *same* CPU as the > > + /* > > + * We are now a low priority child on the *same* CPU as the > > * parent. We will have to wait for our parent to sleep > > * (gem_sync -> i915_wait_request) before we run. > > */ > > + read(link[0], &go, sizeof(go)); > > igt_assert(gem_bo_busy(fd, spin->handle)); > > igt_spin_batch_end(spin); > > } > > > > + write(link[1], &go, sizeof(go)); > > gem_sync(fd, spin->handle); > > igt_waitchildren(); > > > > igt_spin_batch_free(fd, spin); > > + close(link[1]); > > + close(link[0]); > > } > > > > static void bind_to_cpu(int cpu) > > > > Does the parent need to be RT at all now? Would it work to use a > short(er) timed wait and signal back to the child to terminate the > spinner only then? Fake missing interrupt only needs some wait, right? No, we need to wait inside the gem_sync in order to turn on the irq. We do need to get passed that first busy-spin loop. The fun part about this setup was that it didn't require any more knowledge than knowing when it scheduled it was waiting on the interrupt; and thus we didn't need arbitrary timeouts and could be quite rapid. -Chris
On 01/05/2018 12:21, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-05-01 12:10:22) >> >> On 01/05/2018 10:07, Chris Wilson wrote: >>> Our parent is RT, we are not. In theory, we should wait until our parent >>> has gone to sleep before we are allowed to proceed (we should both be >>> bound to the same cpu). Double down on this by sleeping in the child >>> until our parent has written a byte along a pipe(). >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> --- >>> tests/drv_missed_irq.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/tests/drv_missed_irq.c b/tests/drv_missed_irq.c >>> index 9326a5a62..791ee51fb 100644 >>> --- a/tests/drv_missed_irq.c >>> +++ b/tests/drv_missed_irq.c >>> @@ -34,20 +34,29 @@ IGT_TEST_DESCRIPTION("Inject missed interrupts and make sure they are caught"); >>> static void trigger_missed_interrupt(int fd, unsigned ring) >>> { >>> igt_spin_t *spin = __igt_spin_batch_new(fd, 0, ring, 0); >>> + uint32_t go; >>> + int link[2]; >>> + >>> + igt_assert(pipe(link) == 0); >>> >>> igt_fork(child, 1) { >>> - /* We are now a low priority child on the *same* CPU as the >>> + /* >>> + * We are now a low priority child on the *same* CPU as the >>> * parent. We will have to wait for our parent to sleep >>> * (gem_sync -> i915_wait_request) before we run. >>> */ >>> + read(link[0], &go, sizeof(go)); >>> igt_assert(gem_bo_busy(fd, spin->handle)); >>> igt_spin_batch_end(spin); >>> } >>> >>> + write(link[1], &go, sizeof(go)); >>> gem_sync(fd, spin->handle); >>> igt_waitchildren(); >>> >>> igt_spin_batch_free(fd, spin); >>> + close(link[1]); >>> + close(link[0]); >>> } >>> >>> static void bind_to_cpu(int cpu) >>> >> >> Does the parent need to be RT at all now? Would it work to use a >> short(er) timed wait and signal back to the child to terminate the >> spinner only then? Fake missing interrupt only needs some wait, right? > > No, we need to wait inside the gem_sync in order to turn on the irq. We > do need to get passed that first busy-spin loop. The fun part about this > setup was that it didn't require any more knowledge than knowing when it > scheduled it was waiting on the interrupt; and thus we didn't need > arbitrary timeouts and could be quite rapid. I was thinking, as much as the pipe comms improve the odds child will not terminate the spin until the parent is in wait, if we could get rid of the RT requirement by being even more explicit in synchronization. But true, it would need a timed wait, so a slightly slower test, after which parent would signal the child to terminate. Anyway, don't see any issues with this version: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/tests/drv_missed_irq.c b/tests/drv_missed_irq.c index 9326a5a62..791ee51fb 100644 --- a/tests/drv_missed_irq.c +++ b/tests/drv_missed_irq.c @@ -34,20 +34,29 @@ IGT_TEST_DESCRIPTION("Inject missed interrupts and make sure they are caught"); static void trigger_missed_interrupt(int fd, unsigned ring) { igt_spin_t *spin = __igt_spin_batch_new(fd, 0, ring, 0); + uint32_t go; + int link[2]; + + igt_assert(pipe(link) == 0); igt_fork(child, 1) { - /* We are now a low priority child on the *same* CPU as the + /* + * We are now a low priority child on the *same* CPU as the * parent. We will have to wait for our parent to sleep * (gem_sync -> i915_wait_request) before we run. */ + read(link[0], &go, sizeof(go)); igt_assert(gem_bo_busy(fd, spin->handle)); igt_spin_batch_end(spin); } + write(link[1], &go, sizeof(go)); gem_sync(fd, spin->handle); igt_waitchildren(); igt_spin_batch_free(fd, spin); + close(link[1]); + close(link[0]); } static void bind_to_cpu(int cpu)
Our parent is RT, we are not. In theory, we should wait until our parent has gone to sleep before we are allowed to proceed (we should both be bound to the same cpu). Double down on this by sleeping in the child until our parent has written a byte along a pipe(). Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- tests/drv_missed_irq.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)