From: Jeff McGee <jeff.mcgee@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere
Date: Mon, 28 Aug 2017 13:18:44 -0700 [thread overview]
Message-ID: <20170828201844.GC11255@jeffdesk> (raw)
In-Reply-To: <150394948806.25384.1744811151800023186@mail.alporthouse.com>
On Mon, Aug 28, 2017 at 08:44:48PM +0100, Chris Wilson wrote:
> Quoting jeff.mcgee@intel.com (2017-08-28 20:25:30)
> > From: Jeff McGee <jeff.mcgee@intel.com>
> >
> > If someone else is resetting the engine we should clear our own bit as
> > part of skipping that engine. Otherwise we will later believe that it
> > has not been reset successfully and then trigger full gpu reset. If the
> > other guy's reset actually fails, he will trigger the full gpu reset.
>
> The reason we did continue on to the global reset was to serialise
> i915_handle_error() with the other thread. Not a huge issue, but a
> reasonable property to keep -- and we definitely want a to explain why
> only one reset at a time is important.
>
> bool intel_engine_lock_reset() {
> if (!test_and_set_bit(I915_RESET_ENGINE + engine->id,
> &engine->i915->gpu_error.flags))
> return true;
>
> intel_engine_wait_for_reset(engine);
The current code doesn't wait for the other thread to finish the reset, but
this would add that wait. Did you intend that as an additional change to
the current code? I don't think it is necessary. Each thread wants to
reset some subset of engines, so it seems the thread can safely exit as soon
as it knows each of those engines has been reset or is being reset as part
of another thread that got the lock first. If any of the threads fail to
reset an engine they "own", then full gpu reset is assured.
-Jeff
> return false; /* somebody else beat us to the reset */
> }
>
> void intel_engine_wait_for_reset() {
> while (test_and_set_bit(I915_RESET_ENGINE + engine->id,
> &engine->i915->gpu_error.flags))
> wait_on_bit(&engine->i915->gpu_error.flags, I915_RESET_ENGINE + engine->id,
> TASK_UNINTERRUPTIBLE);
> }
>
> It can also be used by selftests/intel_hangcheck.c, so let's refactor
> before we have 3 copies.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-08-28 20:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-28 19:25 [PATCH] drm/i915: Clear local engine-needs-reset bit if in progress elsewhere jeff.mcgee
2017-08-28 19:41 ` Michel Thierry
2017-08-28 19:46 ` Jeff McGee
2017-08-29 15:22 ` Chris Wilson
2017-08-28 19:44 ` Chris Wilson
2017-08-28 20:18 ` Jeff McGee [this message]
2017-08-29 9:07 ` Chris Wilson
2017-08-29 15:04 ` Jeff McGee
2017-08-29 15:17 ` Chris Wilson
2017-08-29 17:01 ` Jeff McGee
2017-09-06 21:57 ` Chris Wilson
2017-09-13 14:23 ` Jeff McGee
2017-08-28 19:48 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-28 20:59 ` ✗ Fi.CI.IGT: warning " Patchwork
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=20170828201844.GC11255@jeffdesk \
--to=jeff.mcgee@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).