All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	igt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t] i915/gem_exec_balancer: Randomise bonded submission
Date: Wed, 27 May 2020 17:00:55 +0100	[thread overview]
Message-ID: <159059525586.30979.1718850848806311032@build.alporthouse.com> (raw)
In-Reply-To: <e3ceb72a-156b-a4cf-96c7-c339ea67ffb1@linux.intel.com>

Quoting Tvrtko Ursulin (2020-05-27 16:51:50)
> 
> On 27/05/2020 14:14, Chris Wilson wrote:
> > Randomly submit a paired spinner and its cancellation as a bonded
> > (submit fence) pair. Apply congestion to the engine with more bonded
> > pairs to see if the execution order fails. If we prevent a cancellation
> > from running, then the spinner will remain spinning forever.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   tests/i915/gem_exec_balancer.c | 108 +++++++++++++++++++++++++++++++++
> >   1 file changed, 108 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > index 80ae82416..98715d726 100644
> > --- a/tests/i915/gem_exec_balancer.c
> > +++ b/tests/i915/gem_exec_balancer.c
> > @@ -1154,6 +1154,111 @@ static void bonded_semaphore(int i915)
> >       gem_context_destroy(i915, ctx);
> >   }
> >   
> > +static void __bonded_dual(int i915,
> > +                       const struct i915_engine_class_instance *siblings,
> > +                       unsigned int count)
> > +{
> > +     struct drm_i915_gem_exec_object2 batch = {};
> > +     struct drm_i915_gem_execbuffer2 execbuf = {
> > +             .buffers_ptr = to_user_pointer(&batch),
> > +             .buffer_count = 1,
> > +     };
> > +     unsigned long cycles = 0;
> > +     uint32_t A, B;
> > +
> > +     A = gem_context_create(i915);
> > +     set_load_balancer(i915, A, siblings, count, NULL);
> > +
> > +     B = gem_context_create(i915);
> > +     set_load_balancer(i915, B, siblings, count, NULL);
> > +
> > +     igt_until_timeout(5) {
> > +             unsigned int master = rand() % count + 1;
> > +             int timeline, fence;
> > +             igt_spin_t *a, *b;
> > +
> > +             timeline = sw_sync_timeline_create();
> > +             fence = sw_sync_timeline_create_fence(timeline, 1);
> > +
> > +             a = igt_spin_new(i915, A,
> > +                              .engine = master,
> > +                              .fence = fence,
> > +                              .flags = (IGT_SPIN_FENCE_IN |
> > +                                        IGT_SPIN_POLL_RUN |
> > +                                        IGT_SPIN_NO_PREEMPTION |
> > +                                        IGT_SPIN_FENCE_OUT));
> > +             b = igt_spin_new(i915, B,
> > +                              .engine = master,
> > +                              .fence = fence,
> > +                              .flags = (IGT_SPIN_FENCE_IN |
> > +                                        IGT_SPIN_POLL_RUN |
> > +                                        IGT_SPIN_NO_PREEMPTION |
> > +                                        IGT_SPIN_FENCE_OUT));
> > +
> > +             close(fence);
> 
> Does this or close(timeline) releases the submissions?

The timeline is the release.
 
> I suggest a variant without the fence anyway for a slightly different 
> profile.

It didn't make any difference. But yes I contemplated the variant.
 
> > +
> > +             if (rand() % 1)
> > +                     igt_swap(a, b);
> > +
> > +             batch.handle = create_semaphore_to_spinner(i915, a);
> 
> These will be preemptible right? More so they schedule out on semaphore 
> interrupt straight away? So I don't see how slaves can be prevented from 
> running because they always are on a different physical engine.

Right, as I understood your description the masters could only be on one
engine with the bonded requests on the other.

And I don't know how to wait from the GPU other than with the
preemptible semaphore spin. A non preemptible wait would be another
spinner, but that would not coordinate across the bond. To reproduce the
non-preemptible HW might require the actual instruction flow.

> We would need a test flavour where slave spins non-preemptively until 
> either the master or CPU signals to stop. Coming up with the exact 
> scheme to achieve this might be challenging. I can think about this more 
> tomorrow.
> 
> And also I don't know why this would fail with my patch! I'll want to 
> have a experiment with that tomorrow as well.

That was indeed mysterious :) I don't have an explanation either yet.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

WARNING: multiple messages have this Message-ID (diff)
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	igt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_exec_balancer: Randomise bonded submission
Date: Wed, 27 May 2020 17:00:55 +0100	[thread overview]
Message-ID: <159059525586.30979.1718850848806311032@build.alporthouse.com> (raw)
In-Reply-To: <e3ceb72a-156b-a4cf-96c7-c339ea67ffb1@linux.intel.com>

Quoting Tvrtko Ursulin (2020-05-27 16:51:50)
> 
> On 27/05/2020 14:14, Chris Wilson wrote:
> > Randomly submit a paired spinner and its cancellation as a bonded
> > (submit fence) pair. Apply congestion to the engine with more bonded
> > pairs to see if the execution order fails. If we prevent a cancellation
> > from running, then the spinner will remain spinning forever.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   tests/i915/gem_exec_balancer.c | 108 +++++++++++++++++++++++++++++++++
> >   1 file changed, 108 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> > index 80ae82416..98715d726 100644
> > --- a/tests/i915/gem_exec_balancer.c
> > +++ b/tests/i915/gem_exec_balancer.c
> > @@ -1154,6 +1154,111 @@ static void bonded_semaphore(int i915)
> >       gem_context_destroy(i915, ctx);
> >   }
> >   
> > +static void __bonded_dual(int i915,
> > +                       const struct i915_engine_class_instance *siblings,
> > +                       unsigned int count)
> > +{
> > +     struct drm_i915_gem_exec_object2 batch = {};
> > +     struct drm_i915_gem_execbuffer2 execbuf = {
> > +             .buffers_ptr = to_user_pointer(&batch),
> > +             .buffer_count = 1,
> > +     };
> > +     unsigned long cycles = 0;
> > +     uint32_t A, B;
> > +
> > +     A = gem_context_create(i915);
> > +     set_load_balancer(i915, A, siblings, count, NULL);
> > +
> > +     B = gem_context_create(i915);
> > +     set_load_balancer(i915, B, siblings, count, NULL);
> > +
> > +     igt_until_timeout(5) {
> > +             unsigned int master = rand() % count + 1;
> > +             int timeline, fence;
> > +             igt_spin_t *a, *b;
> > +
> > +             timeline = sw_sync_timeline_create();
> > +             fence = sw_sync_timeline_create_fence(timeline, 1);
> > +
> > +             a = igt_spin_new(i915, A,
> > +                              .engine = master,
> > +                              .fence = fence,
> > +                              .flags = (IGT_SPIN_FENCE_IN |
> > +                                        IGT_SPIN_POLL_RUN |
> > +                                        IGT_SPIN_NO_PREEMPTION |
> > +                                        IGT_SPIN_FENCE_OUT));
> > +             b = igt_spin_new(i915, B,
> > +                              .engine = master,
> > +                              .fence = fence,
> > +                              .flags = (IGT_SPIN_FENCE_IN |
> > +                                        IGT_SPIN_POLL_RUN |
> > +                                        IGT_SPIN_NO_PREEMPTION |
> > +                                        IGT_SPIN_FENCE_OUT));
> > +
> > +             close(fence);
> 
> Does this or close(timeline) releases the submissions?

The timeline is the release.
 
> I suggest a variant without the fence anyway for a slightly different 
> profile.

It didn't make any difference. But yes I contemplated the variant.
 
> > +
> > +             if (rand() % 1)
> > +                     igt_swap(a, b);
> > +
> > +             batch.handle = create_semaphore_to_spinner(i915, a);
> 
> These will be preemptible right? More so they schedule out on semaphore 
> interrupt straight away? So I don't see how slaves can be prevented from 
> running because they always are on a different physical engine.

Right, as I understood your description the masters could only be on one
engine with the bonded requests on the other.

And I don't know how to wait from the GPU other than with the
preemptible semaphore spin. A non preemptible wait would be another
spinner, but that would not coordinate across the bond. To reproduce the
non-preemptible HW might require the actual instruction flow.

> We would need a test flavour where slave spins non-preemptively until 
> either the master or CPU signals to stop. Coming up with the exact 
> scheme to achieve this might be challenging. I can think about this more 
> tomorrow.
> 
> And also I don't know why this would fail with my patch! I'll want to 
> have a experiment with that tomorrow as well.

That was indeed mysterious :) I don't have an explanation either yet.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2020-05-27 16:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 13:14 [Intel-gfx] [PATCH i-g-t] i915/gem_exec_balancer: Randomise bonded submission Chris Wilson
2020-05-27 13:14 ` [igt-dev] " Chris Wilson
2020-05-27 13:56 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-05-27 15:51 ` [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin
2020-05-27 15:51   ` [igt-dev] " Tvrtko Ursulin
2020-05-27 16:00   ` Chris Wilson [this message]
2020-05-27 16:00     ` Chris Wilson
2020-05-27 16:43     ` Chris Wilson
2020-05-27 16:43       ` [igt-dev] " Chris Wilson
  -- strict thread matches above, loose matches on Subject: below --
2020-05-27 12:11 Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=159059525586.30979.1718850848806311032@build.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.