All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Jens Axboe <axboe@kernel.dk>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Eero Tamminen <eero.t.tamminen@intel.com>,
	stable@kernel.vger.org, "Rantala,
	Valtteri" <valtteri.rantala@intel.com>
Subject: [PATCH 01/15] drm/i915: Break busywaiting for requests on pending signals
Date: Sun, 29 Nov 2015 08:47:59 +0000	[thread overview]
Message-ID: <1448786893-2522-2-git-send-email-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <1448786893-2522-1-git-send-email-chris@chris-wilson.co.uk>

The busywait in __i915_spin_request() does not respect pending signals
and so may consume the entire timeslice for the task instead of
returning to userspace to handle the signal.

In the worst case this could cause a delay in signal processing of 20ms,
which would be a noticeable jitter in cursor tracking. If a higher
resolution signal was being used, for example to provide fairness of a
server timeslices between clients, we could expect to detect some
unfairness between clients (i.e. some windows not updating as fast as
others). This issue was noticed when inspecting a report of poor
interactivity resulting from excessively high __i915_spin_request usage.

Fixes regression from
commit 2def4ad99befa25775dd2f714fdd4d92faec6e34 [v4.2]
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Apr 7 16:20:41 2015 +0100

     drm/i915: Optimistically spin for the request completion

v2: Try to assess the impact of the bug

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jens Axboe <axboe@kernel.dk>
Cc; "Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Cc: "Rantala, Valtteri" <valtteri.rantala@intel.com>
Cc: stable@kernel.vger.org
---
 drivers/gpu/drm/i915/i915_gem.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 33adc8f8ab20..87fc34f5899f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1146,7 +1146,7 @@ static bool missed_irq(struct drm_i915_private *dev_priv,
 	return test_bit(ring->id, &dev_priv->gpu_error.missed_irq_rings);
 }
 
-static int __i915_spin_request(struct drm_i915_gem_request *req)
+static int __i915_spin_request(struct drm_i915_gem_request *req, int state)
 {
 	unsigned long timeout;
 
@@ -1158,6 +1158,9 @@ static int __i915_spin_request(struct drm_i915_gem_request *req)
 		if (i915_gem_request_completed(req, true))
 			return 0;
 
+		if (signal_pending_state(state, current))
+			break;
+
 		if (time_after_eq(jiffies, timeout))
 			break;
 
@@ -1197,6 +1200,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	const bool irq_test_in_progress =
 		ACCESS_ONCE(dev_priv->gpu_error.test_irq_rings) & intel_ring_flag(ring);
+	int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
 	DEFINE_WAIT(wait);
 	unsigned long timeout_expire;
 	s64 before, now;
@@ -1221,7 +1225,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	before = ktime_get_raw_ns();
 
 	/* Optimistic spin for the next jiffie before touching IRQs */
-	ret = __i915_spin_request(req);
+	ret = __i915_spin_request(req, state);
 	if (ret == 0)
 		goto out;
 
@@ -1233,8 +1237,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	for (;;) {
 		struct timer_list timer;
 
-		prepare_to_wait(&ring->irq_queue, &wait,
-				interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE);
+		prepare_to_wait(&ring->irq_queue, &wait, state);
 
 		/* We need to check whether any gpu reset happened in between
 		 * the caller grabbing the seqno and now ... */
@@ -1252,7 +1255,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 			break;
 		}
 
-		if (interruptible && signal_pending(current)) {
+		if (signal_pending_state(state, current)) {
 			ret = -ERESTARTSYS;
 			break;
 		}
-- 
2.6.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-11-29  8:48 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-29  8:47 i915_wait_request scaling Chris Wilson
2015-11-29  8:47 ` Chris Wilson [this message]
2015-11-30 10:01   ` [PATCH 01/15] drm/i915: Break busywaiting for requests on pending signals Tvrtko Ursulin
2015-11-29  8:48 ` [PATCH 02/15] drm/i915: Limit the busy wait on requests to 10us not 10ms! Chris Wilson
2015-11-30 10:02   ` Tvrtko Ursulin
2015-11-30 10:08     ` Chris Wilson
2015-11-29  8:48 ` [PATCH 03/15] drm/i915: Only spin whilst waiting on the current request Chris Wilson
2015-11-30 10:06   ` Tvrtko Ursulin
2015-12-01 15:47     ` Dave Gordon
2015-12-01 15:58       ` Chris Wilson
2015-12-01 16:44         ` Dave Gordon
2015-12-03  8:52           ` Daniel Vetter
2015-11-29  8:48 ` [PATCH 04/15] drm/i915: Cache the reset_counter for the request Chris Wilson
2015-12-01  8:31   ` Daniel Vetter
2015-12-01  8:47     ` Chris Wilson
2015-12-01  9:15       ` Chris Wilson
2015-12-01 11:05         ` [PATCH 1/3] drm/i915: Hide the atomic_read(reset_counter) behind a helper Chris Wilson
2015-12-01 11:05           ` [PATCH 2/3] drm/i915: Store the reset counter when constructing a request Chris Wilson
2015-12-03  8:59             ` Daniel Vetter
2015-12-01 11:05           ` [PATCH 3/3] drm/i915: Prevent leaking of -EIO from i915_wait_request() Chris Wilson
2015-12-03  9:14             ` Daniel Vetter
2015-12-03  9:41               ` Chris Wilson
2015-12-11  9:02               ` Chris Wilson
2015-12-11 16:46                 ` Daniel Vetter
2015-12-03  8:57           ` [PATCH 1/3] drm/i915: Hide the atomic_read(reset_counter) behind a helper Daniel Vetter
2015-12-03  9:02             ` Chris Wilson
2015-12-03  9:20               ` Daniel Vetter
2015-11-29  8:48 ` [PATCH 05/15] drm/i915: Suppress error message when GPU resets are disabled Chris Wilson
2015-12-01  8:30   ` Daniel Vetter
2015-11-29  8:48 ` [PATCH 06/15] drm/i915: Delay queuing hangcheck to wait-request Chris Wilson
2015-11-29  8:48 ` [PATCH 07/15] drm/i915: Check the timeout passed to i915_wait_request Chris Wilson
2015-11-30 10:14   ` Tvrtko Ursulin
2015-11-30 10:19     ` Chris Wilson
2015-11-30 10:27       ` Tvrtko Ursulin
2015-11-30 10:22   ` Chris Wilson
2015-11-30 10:28     ` Tvrtko Ursulin
2015-11-29  8:48 ` [PATCH 08/15] drm/i915: Slaughter the thundering i915_wait_request herd Chris Wilson
2015-11-30 10:53   ` Chris Wilson
2015-11-30 12:09     ` Tvrtko Ursulin
2015-11-30 12:38       ` Chris Wilson
2015-11-30 13:33         ` Tvrtko Ursulin
2015-11-30 14:30           ` Chris Wilson
2015-11-30 12:05   ` Tvrtko Ursulin
2015-11-30 12:30     ` Chris Wilson
2015-11-30 13:32       ` Tvrtko Ursulin
2015-11-30 14:18         ` Chris Wilson
2015-12-01 17:06           ` Dave Gordon
2015-11-30 14:26         ` Chris Wilson
2015-11-30 14:34   ` [PATCH v4] " Chris Wilson
2015-11-30 16:30     ` Chris Wilson
2015-11-30 16:40       ` Chris Wilson
2015-12-01 18:34     ` Dave Gordon
2015-12-03 16:22       ` [PATCH v7] " Chris Wilson
2015-12-07 15:08         ` Tvrtko Ursulin
2015-12-08 10:44           ` Chris Wilson
2015-12-08 14:03             ` Tvrtko Ursulin
2015-12-08 14:33               ` Chris Wilson
2015-11-23 11:34                 ` [RFC 00/12] Convert requests to use struct fence John.C.Harrison
2015-11-23 11:34                   ` [RFC 01/12] staging/android/sync: Support sync points created from dma-fences John.C.Harrison
2015-11-23 13:29                     ` Maarten Lankhorst
2015-11-23 13:31                     ` [Intel-gfx] " Tvrtko Ursulin
2015-11-23 11:34                   ` [RFC 02/12] staging/android/sync: add sync_fence_create_dma John.C.Harrison
2015-11-23 13:27                     ` Maarten Lankhorst
2015-11-23 13:38                       ` John Harrison
2015-11-23 13:44                         ` Tvrtko Ursulin
2015-11-23 13:48                           ` Maarten Lankhorst
2015-11-23 11:34                   ` [RFC 03/12] staging/android/sync: Move sync framework out of staging John.C.Harrison
2015-11-23 11:34                   ` [RFC 04/12] drm/i915: Convert requests to use struct fence John.C.Harrison
2015-11-23 11:34                   ` [RFC 05/12] drm/i915: Removed now redudant parameter to i915_gem_request_completed() John.C.Harrison
2015-11-23 11:34                   ` [RFC 06/12] drm/i915: Add per context timelines to fence object John.C.Harrison
2015-11-23 11:34                   ` [RFC 07/12] drm/i915: Delay the freeing of requests until retire time John.C.Harrison
2015-11-23 11:34                   ` [RFC 08/12] drm/i915: Interrupt driven fences John.C.Harrison
2015-12-11 12:17                     ` Tvrtko Ursulin
2015-11-23 11:34                   ` [RFC 09/12] drm/i915: Updated request structure tracing John.C.Harrison
2015-11-23 11:34                   ` [RFC 10/12] android/sync: Fix reversed sense of signaled fence John.C.Harrison
2015-11-23 11:34                   ` [RFC 11/12] drm/i915: Add sync framework support to execbuff IOCTL John.C.Harrison
2015-11-23 11:34                   ` [RFC 12/12] drm/i915: Cache last IRQ seqno to reduce IRQ overhead John.C.Harrison
2015-11-23 11:38                   ` [RFC 00/12] Convert requests to use struct fence John Harrison
2015-12-08 14:53                   ` [PATCH v7] drm/i915: Slaughter the thundering i915_wait_request herd Dave Gordon
2015-11-30 15:45   ` [PATCH] drm/i915: Convert trace-irq to the breadcrumb waiter Chris Wilson
2015-11-29  8:48 ` [PATCH 09/15] drm/i915: Separate out the seqno-barrier from engine->get_seqno Chris Wilson
2015-11-29  8:48 ` [PATCH 10/15] drm/i915: Remove the lazy_coherency parameter from request-completed? Chris Wilson
2015-11-29  8:48 ` [PATCH 11/15] drm/i915: Use HWS for seqno tracking everywhere Chris Wilson
2015-11-29  8:48 ` [PATCH 12/15] drm/i915: Reduce seqno/irq barrier to a clflush on legacy gen6+ Chris Wilson
2015-11-29  8:48 ` [PATCH 13/15] drm/i915: Stop setting wraparound seqno on initialisation Chris Wilson
2015-12-01 16:57   ` Dave Gordon
2015-12-04  9:36     ` Daniel Vetter
2015-12-04  9:51       ` Chris Wilson
2015-11-29  8:48 ` [PATCH 14/15] drm/i915: Only query timestamp when measuring elapsed time Chris Wilson
2015-11-30 10:19   ` Tvrtko Ursulin
2015-11-30 14:31     ` Chris Wilson
2015-11-29  8:48 ` [PATCH 15/15] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson

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=1448786893-2522-2-git-send-email-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=axboe@kernel.dk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=eero.t.tamminen@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=stable@kernel.vger.org \
    --cc=valtteri.rantala@intel.com \
    /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 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.