On Mon, Nov 02, 2015 at 02:57:47PM +0000, Gong, Zhipeng wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] > > Sent: Monday, November 02, 2015 10:16 PM > > To: Gong, Zhipeng > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Rogozhkin, Dmitry V > > Subject: Re: [PATCH] RFC drm/i915: Slaughter the thundering > > i915_wait_request herd > > > > On Mon, Nov 02, 2015 at 02:00:47PM +0000, Gong, Zhipeng wrote: > > > Attach the perf data for BDW async1 and async5 with or without patch. > > > > Hmm, I can see it is the i915_spin_request() consuming the time, but I was > > hoping to get the callgraph so I could see where the call to i915_wait_request > > was originating from. Could you regenerate the perf report with -g (or maybe > > you need a slightly newer perf)? > > -Chris > > > Chris, I dump the it to txt, but it is hard to read since the call chain is too long. > If you need the data in other format, please let me know. Just get a wider monitor :) Ok, that does confirm that it is the frequent inter-ring sync keeping the waitqueue short. If you try, diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 21017daecb90..008b43be791f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -70,6 +70,8 @@ static void intel_engine_irq_wakeup(struct work_struct *work) container_of(work, struct intel_engine_cs, irq_work); const bool noirq = !__irq_enable(engine); DEFINE_WAIT(wait); +#define KEEPALIVE 10 + int keepalive = KEEPALIVE; for (;;) { struct timer_list timer; @@ -115,8 +117,11 @@ static void intel_engine_irq_wakeup(struct work_struct *work) } engine->irq_first = request; spin_unlock(&engine->irq_lock); - if (request == NULL) - break; + if (request == NULL) { + if (--keepalive == 0) + break; + } else + keepalive = KEEPALIVE; /* Make sure the hangcheck timer is armed in case the GPU * hangs and the request never completes. @@ -124,7 +129,7 @@ static void intel_engine_irq_wakeup(struct work_struct *work) i915_queue_hangcheck(engine->i915); timer.function = NULL; - if (noirq || missed_irq(engine)) { + if (request == NULL || noirq || missed_irq(engine)) { setup_timer_on_stack(&timer, (void (*)(unsigned long))wake_up_process, (unsigned long)current); @@ -165,6 +170,8 @@ void intel_engine_add_wakeup(struct drm_i915_gem_request *request) if (RB_EMPTY_ROOT(&engine->irq_requests)) schedule_work(&engine->irq_work); + else + wake_up_all(&engine->irq_queue); init_waitqueue_head(&request->wait); That should keep the worker alive for a further 10 jiffies, hopefully long enough for the next wait to occur. The cost is that it keeps the interrupt asserted (and to avoid that requires a little rearrangment and probably a dedicated kthread for each ring). -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
