* [CI 1/3] drm/i915: Always mark the writer as also a read for busy ioctl
@ 2016-08-09 16:47 Chris Wilson
2016-08-09 16:47 ` [CI 2/3] drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Chris Wilson @ 2016-08-09 16:47 UTC (permalink / raw)
To: intel-gfx
One of the few guarantees we want the busy ioctl to provide is that the
reported busy writer is included in the set of busy read engines. This
should be provided by the ordering of setting and retiring the active
trackers, but we can do better by explicitly setting the busy read
engine flag for the last writer.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d71fa9a93afa..0478e4610da4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3748,7 +3748,7 @@ static __always_inline unsigned int __busy_read_flag(unsigned int id)
static __always_inline unsigned int __busy_write_id(unsigned int id)
{
- return id;
+ return id | __busy_read_flag(id);
}
static __always_inline unsigned int
@@ -3857,9 +3857,11 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
args->busy |= busy_check_reader(&obj->last_read[idx]);
/* For ABI sanity, we only care that the write engine is in
- * the set of read engines. This is ensured by the ordering
- * of setting last_read/last_write in i915_vma_move_to_active,
- * and then in reverse in retire.
+ * the set of read engines. This should be ensured by the
+ * ordering of setting last_read/last_write in
+ * i915_vma_move_to_active(), and then in reverse in retire.
+ * However, for good measure, we always report the last_write
+ * request as a busy read as well as being a busy write.
*
* We don't care that the set of active read/write engines
* may change during construction of the result, as it is
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [CI 2/3] drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs
2016-08-09 16:47 [CI 1/3] drm/i915: Always mark the writer as also a read for busy ioctl Chris Wilson
@ 2016-08-09 16:47 ` Chris Wilson
2016-08-09 16:47 ` [CI 3/3] drm/i915: Use RCU to annotate and enforce protection for breadcrumb's bh Chris Wilson
2016-08-10 8:43 ` ✗ Ro.CI.BAT: failure for series starting with [CI,1/3] drm/i915: Always mark the writer as also a read for busy ioctl Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2016-08-09 16:47 UTC (permalink / raw)
To: intel-gfx
In commit 2529d57050af ("drm/i915: Drop racy markup of missed-irqs from
idle-worker") the racy detection of missed interrupts was removed when
we went idle. This however opened up the issue that the stuck waiters
were not being reported, causing a test case failure. If we move the
stuck waiter detection out of hangcheck and into the breadcrumb
mechanims (i.e. the waiter) itself, we can avoid this issue entirely.
This leaves hangcheck looking for a stuck GPU (inspecting for request
advancement and HEAD motion), and breadcrumbs looking for a stuck
waiter - hopefully make both easier to understand by their segregation.
v2: Reduce the error message as we now run independently of hangcheck,
and the hanging batch used by igt also counts as a stuck waiter causing
extra warnings in dmesg.
v3: Move the breadcrumb's hangcheck kickstart to the first missed wait.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97104
Fixes: 2529d57050af (waiter"drm/i915: Drop racy markup of missed-irqs...")
Testcase: igt/drv_missed_irq
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 ++---
drivers/gpu/drm/i915/i915_gem.c | 10 -----
drivers/gpu/drm/i915/i915_irq.c | 26 +-----------
drivers/gpu/drm/i915/intel_breadcrumbs.c | 69 ++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_engine_cs.c | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 6 +--
6 files changed, 56 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f62285c1ed7f..96bfc745a820 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -787,8 +787,6 @@ static void i915_ring_seqno_info(struct seq_file *m,
seq_printf(m, "Current sequence (%s): %x\n",
engine->name, intel_engine_get_seqno(engine));
- seq_printf(m, "Current user interrupts (%s): %lx\n",
- engine->name, READ_ONCE(engine->breadcrumbs.irq_wakeups));
spin_lock(&b->lock);
for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
@@ -1434,11 +1432,10 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
engine->hangcheck.seqno,
seqno[id],
engine->last_submitted_seqno);
- seq_printf(m, "\twaiters? %d\n",
- intel_engine_has_waiter(engine));
- seq_printf(m, "\tuser interrupts = %lx [current %lx]\n",
- engine->hangcheck.user_interrupts,
- READ_ONCE(engine->breadcrumbs.irq_wakeups));
+ seq_printf(m, "\twaiters? %s, fake irq active? %s\n",
+ yesno(intel_engine_has_waiter(engine)),
+ yesno(test_bit(engine->id,
+ &dev_priv->gpu_error.missed_irq_rings)));
seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
(long long)engine->hangcheck.acthd,
(long long)acthd[id]);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0478e4610da4..c3652e09d272 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2524,7 +2524,6 @@ i915_gem_idle_work_handler(struct work_struct *work)
container_of(work, typeof(*dev_priv), gt.idle_work.work);
struct drm_device *dev = &dev_priv->drm;
struct intel_engine_cs *engine;
- unsigned int stuck_engines;
bool rearm_hangcheck;
if (!READ_ONCE(dev_priv->gt.awake))
@@ -2554,15 +2553,6 @@ i915_gem_idle_work_handler(struct work_struct *work)
dev_priv->gt.awake = false;
rearm_hangcheck = false;
- /* As we have disabled hangcheck, we need to unstick any waiters still
- * hanging around. However, as we may be racing against the interrupt
- * handler or the waiters themselves, we skip enabling the fake-irq.
- */
- stuck_engines = intel_kick_waiters(dev_priv);
- if (unlikely(stuck_engines))
- DRM_DEBUG_DRIVER("kicked stuck waiters (%x)...missed irq?\n",
- stuck_engines);
-
if (INTEL_GEN(dev_priv) >= 6)
gen6_rps_idle(dev_priv);
intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 591f452ece68..ebb83d5a448b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -972,10 +972,8 @@ static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
static void notify_ring(struct intel_engine_cs *engine)
{
smp_store_mb(engine->breadcrumbs.irq_posted, true);
- if (intel_engine_wakeup(engine)) {
+ if (intel_engine_wakeup(engine))
trace_i915_gem_request_notify(engine);
- engine->breadcrumbs.irq_wakeups++;
- }
}
static void vlv_c0_read(struct drm_i915_private *dev_priv,
@@ -3044,22 +3042,6 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd)
return HANGCHECK_HUNG;
}
-static unsigned long kick_waiters(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *i915 = engine->i915;
- unsigned long irq_count = READ_ONCE(engine->breadcrumbs.irq_wakeups);
-
- if (engine->hangcheck.user_interrupts == irq_count &&
- !test_and_set_bit(engine->id, &i915->gpu_error.missed_irq_rings)) {
- if (!test_bit(engine->id, &i915->gpu_error.test_irq_rings))
- DRM_ERROR("Hangcheck timer elapsed... %s idle\n",
- engine->name);
-
- intel_engine_enable_fake_irq(engine);
- }
-
- return irq_count;
-}
/*
* This is called when the chip hasn't reported back with completed
* batchbuffers in a long time. We keep track per ring seqno progress and
@@ -3097,7 +3079,6 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
bool busy = intel_engine_has_waiter(engine);
u64 acthd;
u32 seqno;
- unsigned user_interrupts;
semaphore_clear_deadlocks(dev_priv);
@@ -3114,15 +3095,11 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
acthd = intel_engine_get_active_head(engine);
seqno = intel_engine_get_seqno(engine);
- /* Reset stuck interrupts between batch advances */
- user_interrupts = 0;
-
if (engine->hangcheck.seqno == seqno) {
if (!intel_engine_is_active(engine)) {
engine->hangcheck.action = HANGCHECK_IDLE;
if (busy) {
/* Safeguard against driver failure */
- user_interrupts = kick_waiters(engine);
engine->hangcheck.score += BUSY;
}
} else {
@@ -3185,7 +3162,6 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
engine->hangcheck.seqno = seqno;
engine->hangcheck.acthd = acthd;
- engine->hangcheck.user_interrupts = user_interrupts;
busy_count += busy;
}
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 90867446f1a5..7be9af1d5424 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -26,6 +26,40 @@
#include "i915_drv.h"
+static void intel_breadcrumbs_hangcheck(unsigned long data)
+{
+ struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+ struct intel_breadcrumbs *b = &engine->breadcrumbs;
+
+ if (!b->irq_enabled)
+ return;
+
+ if (time_before(jiffies, b->timeout)) {
+ mod_timer(&b->hangcheck, b->timeout);
+ return;
+ }
+
+ DRM_DEBUG("Hangcheck timer elapsed... %s idle\n", engine->name);
+ set_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
+ mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
+
+ /* Ensure that even if the GPU hangs, we get woken up.
+ *
+ * However, note that if no one is waiting, we never notice
+ * a gpu hang. Eventually, we will have to wait for a resource
+ * held by the GPU and so trigger a hangcheck. In the most
+ * pathological case, this will be upon memory starvation! To
+ * prevent this, we also queue the hangcheck from the retire
+ * worker.
+ */
+ i915_queue_hangcheck(engine->i915);
+}
+
+static unsigned long wait_timeout(void)
+{
+ return round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES);
+}
+
static void intel_breadcrumbs_fake_irq(unsigned long data)
{
struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
@@ -51,13 +85,6 @@ static void irq_enable(struct intel_engine_cs *engine)
*/
engine->breadcrumbs.irq_posted = true;
- /* Make sure the current hangcheck doesn't falsely accuse a just
- * started irq handler from missing an interrupt (because the
- * interrupt count still matches the stale value from when
- * the irq handler was disabled, many hangchecks ago).
- */
- engine->breadcrumbs.irq_wakeups++;
-
spin_lock_irq(&engine->i915->irq_lock);
engine->irq_enable(engine);
spin_unlock_irq(&engine->i915->irq_lock);
@@ -98,17 +125,13 @@ static void __intel_breadcrumbs_enable_irq(struct intel_breadcrumbs *b)
}
if (!b->irq_enabled ||
- test_bit(engine->id, &i915->gpu_error.missed_irq_rings))
+ test_bit(engine->id, &i915->gpu_error.missed_irq_rings)) {
mod_timer(&b->fake_irq, jiffies + 1);
-
- /* Ensure that even if the GPU hangs, we get woken up.
- *
- * However, note that if no one is waiting, we never notice
- * a gpu hang. Eventually, we will have to wait for a resource
- * held by the GPU and so trigger a hangcheck. In the most
- * pathological case, this will be upon memory starvation!
- */
- i915_queue_hangcheck(i915);
+ } else {
+ /* Ensure we never sleep indefinitely */
+ GEM_BUG_ON(!time_after(b->timeout, jiffies));
+ mod_timer(&b->hangcheck, b->timeout);
+ }
}
static void __intel_breadcrumbs_disable_irq(struct intel_breadcrumbs *b)
@@ -219,6 +242,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
GEM_BUG_ON(!next && !first);
if (next && next != &wait->node) {
GEM_BUG_ON(first);
+ b->timeout = wait_timeout();
b->first_wait = to_wait(next);
smp_store_mb(b->irq_seqno_bh, b->first_wait->tsk);
/* As there is a delay between reading the current
@@ -245,6 +269,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
if (first) {
GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
+ b->timeout = wait_timeout();
b->first_wait = wait;
smp_store_mb(b->irq_seqno_bh, wait->tsk);
/* After assigning ourselves as the new bottom-half, we must
@@ -277,11 +302,6 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine,
return first;
}
-void intel_engine_enable_fake_irq(struct intel_engine_cs *engine)
-{
- mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
-}
-
static inline bool chain_wakeup(struct rb_node *rb, int priority)
{
return rb && to_wait(rb)->tsk->prio <= priority;
@@ -359,6 +379,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
* the interrupt, or if we have to handle an
* exception rather than a seqno completion.
*/
+ b->timeout = wait_timeout();
b->first_wait = to_wait(next);
smp_store_mb(b->irq_seqno_bh, b->first_wait->tsk);
if (b->first_wait->seqno != wait->seqno)
@@ -536,6 +557,9 @@ int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
setup_timer(&b->fake_irq,
intel_breadcrumbs_fake_irq,
(unsigned long)engine);
+ setup_timer(&b->hangcheck,
+ intel_breadcrumbs_hangcheck,
+ (unsigned long)engine);
/* Spawn a thread to provide a common bottom-half for all signals.
* As this is an asynchronous interface we cannot steal the current
@@ -560,6 +584,7 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
if (!IS_ERR_OR_NULL(b->signaler))
kthread_stop(b->signaler);
+ del_timer_sync(&b->hangcheck);
del_timer_sync(&b->fake_irq);
}
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index e9b301ae2d0c..0dd3d1de18aa 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -164,6 +164,7 @@ cleanup:
void intel_engine_init_hangcheck(struct intel_engine_cs *engine)
{
memset(&engine->hangcheck, 0, sizeof(engine->hangcheck));
+ clear_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings);
}
static void intel_engine_init_requests(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index bf9a6e56e719..c7a6db310f9f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -75,7 +75,6 @@ enum intel_engine_hangcheck_action {
struct intel_engine_hangcheck {
u64 acthd;
- unsigned long user_interrupts;
u32 seqno;
int score;
enum intel_engine_hangcheck_action action;
@@ -173,7 +172,6 @@ struct intel_engine_cs {
*/
struct intel_breadcrumbs {
struct task_struct *irq_seqno_bh; /* bh for user interrupts */
- unsigned long irq_wakeups;
bool irq_posted;
spinlock_t lock; /* protects the lists of requests */
@@ -183,6 +181,9 @@ struct intel_engine_cs {
struct task_struct *signaler; /* used for fence signalling */
struct drm_i915_gem_request *first_signal;
struct timer_list fake_irq; /* used after a missed interrupt */
+ struct timer_list hangcheck; /* detect missed interrupts */
+
+ unsigned long timeout;
bool irq_enabled : 1;
bool rpm_wakelock : 1;
@@ -560,7 +561,6 @@ static inline bool intel_engine_wakeup(struct intel_engine_cs *engine)
return wakeup;
}
-void intel_engine_enable_fake_irq(struct intel_engine_cs *engine);
void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
unsigned int intel_kick_waiters(struct drm_i915_private *i915);
unsigned int intel_kick_signalers(struct drm_i915_private *i915);
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [CI 3/3] drm/i915: Use RCU to annotate and enforce protection for breadcrumb's bh
2016-08-09 16:47 [CI 1/3] drm/i915: Always mark the writer as also a read for busy ioctl Chris Wilson
2016-08-09 16:47 ` [CI 2/3] drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs Chris Wilson
@ 2016-08-09 16:47 ` Chris Wilson
2016-08-10 8:43 ` ✗ Ro.CI.BAT: failure for series starting with [CI,1/3] drm/i915: Always mark the writer as also a read for busy ioctl Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2016-08-09 16:47 UTC (permalink / raw)
To: intel-gfx
The bottom-half we use for processing the breadcrumb interrupt is a
task, which is an RCU protected struct. When accessing this struct, we
need to be holding the RCU read lock to prevent it disappearing beneath
us. We can use the RCU annotation to mark our irq_seqno_bh pointer as
being under RCU guard and then use the RCU accessors to both provide
correct ordering of access through the pointer.
Most notably, this fixes the access from hard irq context to use the RCU
read lock, which both Daniel and Tvrtko complained about.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++--
drivers/gpu/drm/i915/intel_breadcrumbs.c | 22 +++++++++-------------
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 --
drivers/gpu/drm/i915/intel_ringbuffer.h | 25 ++++++++++++++++---------
4 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 54f789c75aa1..2cb91ffbbfa2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3849,7 +3849,7 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req)
* is woken.
*/
if (engine->irq_seqno_barrier &&
- READ_ONCE(engine->breadcrumbs.irq_seqno_bh) == current &&
+ rcu_access_pointer(engine->breadcrumbs.irq_seqno_bh) == current &&
cmpxchg_relaxed(&engine->breadcrumbs.irq_posted, 1, 0)) {
struct task_struct *tsk;
@@ -3874,7 +3874,7 @@ static inline bool __i915_request_irq_complete(struct drm_i915_gem_request *req)
* irq_posted == false but we are still running).
*/
rcu_read_lock();
- tsk = READ_ONCE(engine->breadcrumbs.irq_seqno_bh);
+ tsk = rcu_dereference(engine->breadcrumbs.irq_seqno_bh);
if (tsk && tsk != current)
/* Note that if the bottom-half is changed as we
* are sending the wake-up, the new bottom-half will
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 7be9af1d5424..2491e4c1eaf0 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -71,10 +71,8 @@ static void intel_breadcrumbs_fake_irq(unsigned long data)
* every jiffie in order to kick the oldest waiter to do the
* coherent seqno check.
*/
- rcu_read_lock();
if (intel_engine_wakeup(engine))
mod_timer(&engine->breadcrumbs.fake_irq, jiffies + 1);
- rcu_read_unlock();
}
static void irq_enable(struct intel_engine_cs *engine)
@@ -234,7 +232,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
}
rb_link_node(&wait->node, parent, p);
rb_insert_color(&wait->node, &b->waiters);
- GEM_BUG_ON(!first && !b->irq_seqno_bh);
+ GEM_BUG_ON(!first && !rcu_access_pointer(b->irq_seqno_bh));
if (completed) {
struct rb_node *next = rb_next(completed);
@@ -244,7 +242,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
GEM_BUG_ON(first);
b->timeout = wait_timeout();
b->first_wait = to_wait(next);
- smp_store_mb(b->irq_seqno_bh, b->first_wait->tsk);
+ rcu_assign_pointer(b->irq_seqno_bh, b->first_wait->tsk);
/* As there is a delay between reading the current
* seqno, processing the completed tasks and selecting
* the next waiter, we may have missed the interrupt
@@ -271,7 +269,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
GEM_BUG_ON(rb_first(&b->waiters) != &wait->node);
b->timeout = wait_timeout();
b->first_wait = wait;
- smp_store_mb(b->irq_seqno_bh, wait->tsk);
+ rcu_assign_pointer(b->irq_seqno_bh, wait->tsk);
/* After assigning ourselves as the new bottom-half, we must
* perform a cursory check to prevent a missed interrupt.
* Either we miss the interrupt whilst programming the hardware,
@@ -282,7 +280,7 @@ static bool __intel_engine_add_wait(struct intel_engine_cs *engine,
*/
__intel_breadcrumbs_enable_irq(b);
}
- GEM_BUG_ON(!b->irq_seqno_bh);
+ GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh));
GEM_BUG_ON(!b->first_wait);
GEM_BUG_ON(rb_first(&b->waiters) != &b->first_wait->node);
@@ -337,7 +335,7 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
const int priority = wakeup_priority(b, wait->tsk);
struct rb_node *next;
- GEM_BUG_ON(b->irq_seqno_bh != wait->tsk);
+ GEM_BUG_ON(rcu_access_pointer(b->irq_seqno_bh) != wait->tsk);
/* We are the current bottom-half. Find the next candidate,
* the first waiter in the queue on the remaining oldest
@@ -381,13 +379,13 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
*/
b->timeout = wait_timeout();
b->first_wait = to_wait(next);
- smp_store_mb(b->irq_seqno_bh, b->first_wait->tsk);
+ rcu_assign_pointer(b->irq_seqno_bh, b->first_wait->tsk);
if (b->first_wait->seqno != wait->seqno)
__intel_breadcrumbs_enable_irq(b);
- wake_up_process(b->irq_seqno_bh);
+ wake_up_process(b->first_wait->tsk);
} else {
b->first_wait = NULL;
- WRITE_ONCE(b->irq_seqno_bh, NULL);
+ rcu_assign_pointer(b->irq_seqno_bh, NULL);
__intel_breadcrumbs_disable_irq(b);
}
} else {
@@ -401,7 +399,7 @@ out_unlock:
GEM_BUG_ON(b->first_wait == wait);
GEM_BUG_ON(rb_first(&b->waiters) !=
(b->first_wait ? &b->first_wait->node : NULL));
- GEM_BUG_ON(!b->irq_seqno_bh ^ RB_EMPTY_ROOT(&b->waiters));
+ GEM_BUG_ON(!rcu_access_pointer(b->irq_seqno_bh) ^ RB_EMPTY_ROOT(&b->waiters));
spin_unlock(&b->lock);
}
@@ -598,11 +596,9 @@ unsigned int intel_kick_waiters(struct drm_i915_private *i915)
* RCU lock, i.e. as we call wake_up_process() we must be holding the
* rcu_read_lock().
*/
- rcu_read_lock();
for_each_engine(engine, i915)
if (unlikely(intel_engine_wakeup(engine)))
mask |= intel_engine_flag(engine);
- rcu_read_unlock();
return mask;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e08a1e1b04e4..16b726fe33eb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2410,9 +2410,7 @@ void intel_engine_init_seqno(struct intel_engine_cs *engine, u32 seqno)
/* After manually advancing the seqno, fake the interrupt in case
* there are any waiters for that seqno.
*/
- rcu_read_lock();
intel_engine_wakeup(engine);
- rcu_read_unlock();
}
static void gen6_bsd_submit_request(struct drm_i915_gem_request *request)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c7a6db310f9f..ea2735144b2a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -171,7 +171,7 @@ struct intel_engine_cs {
* the overhead of waking that client is much preferred.
*/
struct intel_breadcrumbs {
- struct task_struct *irq_seqno_bh; /* bh for user interrupts */
+ struct task_struct __rcu *irq_seqno_bh; /* bh for interrupts */
bool irq_posted;
spinlock_t lock; /* protects the lists of requests */
@@ -539,25 +539,32 @@ void intel_engine_remove_wait(struct intel_engine_cs *engine,
struct intel_wait *wait);
void intel_engine_enable_signaling(struct drm_i915_gem_request *request);
-static inline bool intel_engine_has_waiter(struct intel_engine_cs *engine)
+static inline bool intel_engine_has_waiter(const struct intel_engine_cs *engine)
{
- return READ_ONCE(engine->breadcrumbs.irq_seqno_bh);
+ return rcu_access_pointer(engine->breadcrumbs.irq_seqno_bh);
}
-static inline bool intel_engine_wakeup(struct intel_engine_cs *engine)
+static inline bool intel_engine_wakeup(const struct intel_engine_cs *engine)
{
bool wakeup = false;
- struct task_struct *tsk = READ_ONCE(engine->breadcrumbs.irq_seqno_bh);
+
/* Note that for this not to dangerously chase a dangling pointer,
- * the caller is responsible for ensure that the task remain valid for
- * wake_up_process() i.e. that the RCU grace period cannot expire.
+ * we must hold the rcu_read_lock here.
*
* Also note that tsk is likely to be in !TASK_RUNNING state so an
* early test for tsk->state != TASK_RUNNING before wake_up_process()
* is unlikely to be beneficial.
*/
- if (tsk)
- wakeup = wake_up_process(tsk);
+ if (intel_engine_has_waiter(engine)) {
+ struct task_struct *tsk;
+
+ rcu_read_lock();
+ tsk = rcu_dereference(engine->breadcrumbs.irq_seqno_bh);
+ if (tsk)
+ wakeup = wake_up_process(tsk);
+ rcu_read_unlock();
+ }
+
return wakeup;
}
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [CI,1/3] drm/i915: Always mark the writer as also a read for busy ioctl
2016-08-09 16:47 [CI 1/3] drm/i915: Always mark the writer as also a read for busy ioctl Chris Wilson
2016-08-09 16:47 ` [CI 2/3] drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs Chris Wilson
2016-08-09 16:47 ` [CI 3/3] drm/i915: Use RCU to annotate and enforce protection for breadcrumb's bh Chris Wilson
@ 2016-08-10 8:43 ` Patchwork
2 siblings, 0 replies; 4+ messages in thread
From: Patchwork @ 2016-08-10 8:43 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/3] drm/i915: Always mark the writer as also a read for busy ioctl
URL : https://patchwork.freedesktop.org/series/10862/
State : failure
== Summary ==
Series 10862v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/10862/revisions/1/mbox
Test kms_cursor_legacy:
Subgroup basic-flip-vs-cursor-legacy:
pass -> FAIL (ro-byt-n2820)
pass -> FAIL (ro-skl3-i5-6260u)
Subgroup basic-flip-vs-cursor-varying-size:
pass -> FAIL (ro-hsw-i7-4770r)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass -> FAIL (ro-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c-frame-sequence:
fail -> PASS (ro-ivb-i7-3770)
Subgroup suspend-read-crc-pipe-a:
incomplete -> PASS (fi-hsw-i7-4770k)
skip -> DMESG-WARN (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-b:
skip -> DMESG-WARN (ro-bdw-i5-5250u)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> SKIP (ro-bdw-i5-5250u)
fi-hsw-i7-4770k total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-qkkr total:244 pass:186 dwarn:28 dfail:0 fail:3 skip:27
fi-skl-i5-6260u total:244 pass:224 dwarn:4 dfail:0 fail:2 skip:14
fi-skl-i7-6700k total:107 pass:84 dwarn:0 dfail:0 fail:0 skip:22
fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
ro-bdw-i5-5250u total:240 pass:220 dwarn:3 dfail:0 fail:0 skip:17
ro-bdw-i7-5600u total:240 pass:206 dwarn:1 dfail:0 fail:1 skip:32
ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:3 skip:43
ro-byt-n2820 total:240 pass:196 dwarn:0 dfail:0 fail:4 skip:40
ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26
ro-hsw-i7-4770r total:240 pass:213 dwarn:0 dfail:0 fail:1 skip:26
ro-ilk1-i5-650 total:235 pass:173 dwarn:0 dfail:0 fail:2 skip:60
ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35
ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31
ro-skl3-i5-6260u total:240 pass:223 dwarn:0 dfail:0 fail:3 skip:14
ro-bdw-i7-5557U failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1801/
d0e3a4b drm-intel-nightly: 2016y-08m-09d-20h-18m-38s UTC integration manifest
5cd44fa drm/i915: Use RCU to annotate and enforce protection for breadcrumb's bh
7f85419 drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs
039377c drm/i915: Always mark the writer as also a read for busy ioctl
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-10 8:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 16:47 [CI 1/3] drm/i915: Always mark the writer as also a read for busy ioctl Chris Wilson
2016-08-09 16:47 ` [CI 2/3] drm/i915: Move missed interrupt detection from hangcheck to breadcrumbs Chris Wilson
2016-08-09 16:47 ` [CI 3/3] drm/i915: Use RCU to annotate and enforce protection for breadcrumb's bh Chris Wilson
2016-08-10 8:43 ` ✗ Ro.CI.BAT: failure for series starting with [CI,1/3] drm/i915: Always mark the writer as also a read for busy ioctl Patchwork
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.