All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Ruhl, Michael J" <michael.j.ruhl@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_close_race: Mix in a contexts and a small delay to closure
Date: Wed, 01 Jul 2020 14:09:31 +0100	[thread overview]
Message-ID: <159360897195.17178.15908745323615959086@build.alporthouse.com> (raw)
In-Reply-To: <14063C7AD467DE4B82DEDB5C278E8663011E8636D8@fmsmsx107.amr.corp.intel.com>

Quoting Ruhl, Michael J (2020-07-01 13:39:22)
> >               do {
> >-                      if (drmIoctl(crashme.fd, DRM_IOCTL_GEM_OPEN,
> >&name))
> >+                      uint32_t ctx = 0;
> >+
> >+                      if (drmIoctl(crashme.fd,
> >+                                   DRM_IOCTL_GEM_OPEN,
> >+                                   &name))
> >                               break;
> >
> >-                      selfcopy(crashme.fd, name.handle, 100);
> >-                      drmIoctl(crashme.fd, DRM_IOCTL_GEM_CLOSE,
> >&name.handle);
> >+                      if (flags & CONTEXTS)
> >+                              __gem_context_create(crashme.fd, &ctx);
> >+
> >+                      selfcopy(crashme.fd, ctx, name.handle, 1);
> >+
> >+                      ctx = history[n % N_HISTORY];
> 
> Ahh this 'ctx' isn't really a context, it an unclosed handle.

Welcome to my world of laziness.
 
> So the difference is that you keep around N_HISTORY open handles and
> the associated contexts (if requested) until the test is done.
> 
> And 256 is enough history?  Any concerns with faster CPU/GPUs?

It's a balancing between trying to keep the original test where we are
closing handles as go along and keeping some around. On the slow atom
with debug enabled, it would complete a few hundred cycles in the 100ms
loop. So I picked 256 so that it would start evicting some old handles
on some passes.

For the purpose of hitting the bookmark, we just need to hit one case
with more than one element. And I manually verified that the test case
was seeing contention at that point, i.e. we released the spinlock so
that another close_object was seeing the other bookmarks in its
obj->lut_list walk. So I'm confident this will hit the path in question
in CI, I'm not happy that it can't prove it did :|

[At the extreme, we could look at the fairness of close_object!]

> Looks like a good way to stress things.
> 
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Ta,
-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: "Ruhl, Michael J" <michael.j.ruhl@intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] i915/gem_close_race: Mix in a contexts and a small delay to closure
Date: Wed, 01 Jul 2020 14:09:31 +0100	[thread overview]
Message-ID: <159360897195.17178.15908745323615959086@build.alporthouse.com> (raw)
In-Reply-To: <14063C7AD467DE4B82DEDB5C278E8663011E8636D8@fmsmsx107.amr.corp.intel.com>

Quoting Ruhl, Michael J (2020-07-01 13:39:22)
> >               do {
> >-                      if (drmIoctl(crashme.fd, DRM_IOCTL_GEM_OPEN,
> >&name))
> >+                      uint32_t ctx = 0;
> >+
> >+                      if (drmIoctl(crashme.fd,
> >+                                   DRM_IOCTL_GEM_OPEN,
> >+                                   &name))
> >                               break;
> >
> >-                      selfcopy(crashme.fd, name.handle, 100);
> >-                      drmIoctl(crashme.fd, DRM_IOCTL_GEM_CLOSE,
> >&name.handle);
> >+                      if (flags & CONTEXTS)
> >+                              __gem_context_create(crashme.fd, &ctx);
> >+
> >+                      selfcopy(crashme.fd, ctx, name.handle, 1);
> >+
> >+                      ctx = history[n % N_HISTORY];
> 
> Ahh this 'ctx' isn't really a context, it an unclosed handle.

Welcome to my world of laziness.
 
> So the difference is that you keep around N_HISTORY open handles and
> the associated contexts (if requested) until the test is done.
> 
> And 256 is enough history?  Any concerns with faster CPU/GPUs?

It's a balancing between trying to keep the original test where we are
closing handles as go along and keeping some around. On the slow atom
with debug enabled, it would complete a few hundred cycles in the 100ms
loop. So I picked 256 so that it would start evicting some old handles
on some passes.

For the purpose of hitting the bookmark, we just need to hit one case
with more than one element. And I manually verified that the test case
was seeing contention at that point, i.e. we released the spinlock so
that another close_object was seeing the other bookmarks in its
obj->lut_list walk. So I'm confident this will hit the path in question
in CI, I'm not happy that it can't prove it did :|

[At the extreme, we could look at the fairness of close_object!]

> Looks like a good way to stress things.
> 
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Ta,
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  reply	other threads:[~2020-07-01 13:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 21:25 [Intel-gfx] [PATCH i-g-t] i915/gem_close_race: Mix in a contexts and a small delay to closure Chris Wilson
2020-06-30 21:25 ` [igt-dev] " Chris Wilson
2020-07-01  3:17 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-07-01  5:43 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2020-07-01 12:39 ` [Intel-gfx] [PATCH i-g-t] " Ruhl, Michael J
2020-07-01 12:39   ` [igt-dev] " Ruhl, Michael J
2020-07-01 13:09   ` Chris Wilson [this message]
2020-07-01 13:09     ` 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=159360897195.17178.15908745323615959086@build.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=michael.j.ruhl@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.