diff options
| author | Chris Wilson <chris@chris-wilson.co.uk> | 2017-03-15 22:22:59 +0000 |
|---|---|---|
| committer | Chris Wilson <chris@chris-wilson.co.uk> | 2017-03-16 08:49:28 +0000 |
| commit | a6b0a141289db58d4934345fa2617602bf859a44 (patch) | |
| tree | a5cac9cabf4286a3926bad5b1b383b66cba3520b /drivers/gpu/drm | |
| parent | db93991bf515916e181aa3ddb5a6cbc8b86bdc42 (diff) | |
drm/i915/breadcrumbs: Tweak commentary
Tvrtko spotted a stale reference to b->lock (now b->rb_lock) so review
the comments and try to improve them in passing.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170315222259.1469-1-chris@chris-wilson.co.uk
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Diffstat (limited to 'drivers/gpu/drm')
| -rw-r--r-- | drivers/gpu/drm/i915/intel_breadcrumbs.c | 25 |
1 files changed, 17 insertions, 8 deletions
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index cb6985acc542..ba986edee312 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -85,7 +85,7 @@ static void intel_breadcrumbs_hangcheck(unsigned long data) return; } - /* We keep the hangcheck time alive until we disarm the irq, even + /* We keep the hangcheck timer alive until we disarm the irq, even * if there are no waiters at present. * * If the waiter was currently running, assume it hasn't had a chance @@ -110,12 +110,11 @@ static void intel_breadcrumbs_fake_irq(unsigned long data) struct intel_engine_cs *engine = (struct intel_engine_cs *)data; struct intel_breadcrumbs *b = &engine->breadcrumbs; - /* - * The timer persists in case we cannot enable interrupts, + /* The timer persists in case we cannot enable interrupts, * or if we have previously seen seqno/interrupt incoherency - * ("missed interrupt" syndrome). Here the worker will wake up - * every jiffie in order to kick the oldest waiter to do the - * coherent seqno check. + * ("missed interrupt" syndrome, better known as a "missed breadcrumb"). + * Here the worker will wake up every jiffie in order to kick the + * oldest waiter to do the coherent seqno check. */ spin_lock_irq(&b->irq_lock); @@ -290,7 +289,12 @@ static inline void __intel_breadcrumbs_finish(struct intel_breadcrumbs *b, GEM_BUG_ON(b->irq_wait == wait); /* This request is completed, so remove it from the tree, mark it as - * complete, and *then* wake up the associated task. + * complete, and *then* wake up the associated task. N.B. when the + * task wakes up, it will find the empty rb_node, discern that it + * has already been removed from the tree and skip the serialisation + * of the b->rb_lock and b->irq_lock. This means that the destruction + * of the intel_wait is not serialised with the interrupt handler + * by the waiter - it must instead be serialised by the caller. */ rb_erase(&wait->node, &b->waiters); RB_CLEAR_NODE(&wait->node); @@ -397,6 +401,11 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine, } if (completed) { + /* Advance the bottom-half (b->irq_wait) before we wake up + * the waiters who may scribble over their intel_wait + * just as the interrupt handler is dereferencing it via + * b->irq_wait. + */ if (!first) { struct rb_node *next = rb_next(completed); GEM_BUG_ON(next == &wait->node); @@ -653,7 +662,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request) /* Note that we may be called from an interrupt handler on another * device (e.g. nouveau signaling a fence completion causing us * to submit a request, and so enable signaling). As such, - * we need to make sure that all other users of b->lock protect + * we need to make sure that all other users of b->rb_lock protect * against interrupts, i.e. use spin_lock_irqsave. */ |