drm/i915: Only report a wakeup if the waiter was truly asleep
If we attempt to wake up a waiter, who is currently checking the seqno it will be in the TASK_INTERRUPTIBLE state and ttwu will report success. However, it is actually awake and functioning -- so delay reporting the actual wake up until it sleeps. This fixes some spurious claims of missed_breadcrumbs when running under heavy load; i.e. sufficient load to preempt away the newly woken waiter before they complete their checks. However, it does so at the cost of a rare false negative; where the waiter changes between the check and ttwu -- the only way to fix that would be to extend the reporting from ttwu where the check could be done atomically. v2: Defend against !CONFIG_SMP v3: Don't filter out calls to wake_up_process v4: Drop risky microoptimisation to skip wakeups Testcase: igt/drv_missed_irq # sanity check we do detect missed_breadcrumb() Testcase: igt/gem_concurrent_blit # for generating false positives References: https://bugs.freedesktop.org/show_bug.cgi?id=100007 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20171209124710.1606-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
This commit is contained in:
parent
776bc27fd8
commit
b92326a040
@ -27,6 +27,12 @@
|
||||
|
||||
#include "i915_drv.h"
|
||||
|
||||
#ifdef CONFIG_SMP
|
||||
#define task_asleep(tsk) ((tsk)->state & TASK_NORMAL && !(tsk)->on_cpu)
|
||||
#else
|
||||
#define task_asleep(tsk) ((tsk)->state & TASK_NORMAL)
|
||||
#endif
|
||||
|
||||
static unsigned int __intel_breadcrumbs_wakeup(struct intel_breadcrumbs *b)
|
||||
{
|
||||
struct intel_wait *wait;
|
||||
@ -36,8 +42,20 @@ static unsigned int __intel_breadcrumbs_wakeup(struct intel_breadcrumbs *b)
|
||||
|
||||
wait = b->irq_wait;
|
||||
if (wait) {
|
||||
/*
|
||||
* N.B. Since task_asleep() and ttwu are not atomic, the
|
||||
* waiter may actually go to sleep after the check, causing
|
||||
* us to suppress a valid wakeup. We prefer to reduce the
|
||||
* number of false positive missed_breadcrumb() warnings
|
||||
* at the expense of a few false negatives, as it it easy
|
||||
* to trigger a false positive under heavy load. Enough
|
||||
* signal should remain from genuine missed_breadcrumb()
|
||||
* for us to detect in CI.
|
||||
*/
|
||||
bool was_asleep = task_asleep(wait->tsk);
|
||||
|
||||
result = ENGINE_WAKEUP_WAITER;
|
||||
if (wake_up_process(wait->tsk))
|
||||
if (wake_up_process(wait->tsk) && was_asleep)
|
||||
result |= ENGINE_WAKEUP_ASLEEP;
|
||||
}
|
||||
|
||||
@ -77,8 +95,8 @@ static noinline void missed_breadcrumb(struct intel_engine_cs *engine)
|
||||
|
||||
static void intel_breadcrumbs_hangcheck(struct timer_list *t)
|
||||
{
|
||||
struct intel_engine_cs *engine = from_timer(engine, t,
|
||||
breadcrumbs.hangcheck);
|
||||
struct intel_engine_cs *engine =
|
||||
from_timer(engine, t, breadcrumbs.hangcheck);
|
||||
struct intel_breadcrumbs *b = &engine->breadcrumbs;
|
||||
|
||||
if (!b->irq_armed)
|
||||
@ -104,7 +122,7 @@ static void intel_breadcrumbs_hangcheck(struct timer_list *t)
|
||||
*/
|
||||
if (intel_engine_wakeup(engine) & ENGINE_WAKEUP_ASLEEP) {
|
||||
missed_breadcrumb(engine);
|
||||
mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
|
||||
mod_timer(&b->fake_irq, jiffies + 1);
|
||||
} else {
|
||||
mod_timer(&b->hangcheck, wait_timeout());
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user