A bunch of execbuf regression fixes

A bunch of execbuf regression fixes

[Chris Wilson]

A couple of regression fixes that have been outstanding for a few weeks,
plus a pair of recent findings
-Chris

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

[PATCH 1/4] drm/i915: Remove assertion from raw __i915_vma_unpin()

[Chris Wilson]

After we detect a i915_vma pin overflow, we call __i915_vma_unpin to
cleanup. However, on an overflow the pin_count bitfield will be zero,
triggering an assertion, even though we the intention is to merely warn
and report the error back to the user (as historically the culprit has
be a leak in the display code).

Fixes: 20dfbde463c8 ("drm/i915: Wrap vma->pin_count accessors with small inline helpers")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 4a673fc1a432..20cf272c97b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -284,12 +284,12 @@ static inline void __i915_vma_pin(struct i915_vma *vma)
 
 static inline void __i915_vma_unpin(struct i915_vma *vma)
 {
-	GEM_BUG_ON(!i915_vma_is_pinned(vma));
 	vma->flags--;
 }
 
 static inline void i915_vma_unpin(struct i915_vma *vma)
 {
+	GEM_BUG_ON(!i915_vma_is_pinned(vma));
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	__i915_vma_unpin(vma);
 }
-- 
2.13.3

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

[PATCH 2/4] drm/i915: Only mark the execobject as pinned on success

[Chris Wilson]

If we fail to acquire a fence (for old school fenced GPU access) then we
unwind the vma reservation, including its pin. However, we were making
the execobject as holding the pin before erring out, leading to a double
unpin:

[ 3193.991802] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:287!
[ 3193.998131] invalid opcode: 0000 [#1] PREEMPT SMP
[ 3194.002816] Modules linked in: snd_hda_intel i915 vgem snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich mei_me e1000e mei prime_numbers ptp pps_core [last unloaded: i915]
[ 3194.022841] CPU: 0 PID: 8123 Comm: kms_flip Tainted: G     U          4.13.0-rc1-CI-CI_DRM_471+ #1
[ 3194.031765] Hardware name: Dell Inc. OptiPlex 755                 /0PU052, BIOS A04 11/05/2007
[ 3194.040343] task: ffff8800785d4c40 task.stack: ffffc90001768000
[ 3194.046339] RIP: 0010:eb_release_vmas.isra.6+0x119/0x180 [i915]
[ 3194.052234] RSP: 0018:ffffc9000176ba80 EFLAGS: 00010246
[ 3194.057439] RAX: 00000000000003c0 RBX: ffff8800710fc2d8 RCX: ffff8800588e4f48
[ 3194.064546] RDX: ffffffff1fffffff RSI: 00000000ffffffff RDI: ffff8800588e00d0
[ 3194.071654] RBP: ffffc9000176bab0 R08: 0000000000000000 R09: 0000000000000000
[ 3194.078761] R10: 0000000000000040 R11: 0000000000000001 R12: ffff880060822f00
[ 3194.085867] R13: 0000000000000310 R14: 00000000000003b8 R15: ffffc9000176bbb0
[ 3194.092975] FS:  00007fd2b94aba40(0000) GS:ffff88007d200000(0000) knlGS:0000000000000000
[ 3194.101033] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3194.106754] CR2: 00007ffbec3ff000 CR3: 0000000074e67000 CR4: 00000000000006f0
[ 3194.113861] Call Trace:
[ 3194.116321]  eb_relocate_slow+0x67/0x4e0 [i915]
[ 3194.120861]  i915_gem_do_execbuffer+0x429/0x1260 [i915]
[ 3194.126070]  ? lock_acquire+0xb5/0x210
[ 3194.129803]  ? __might_fault+0x39/0x90
[ 3194.133563]  i915_gem_execbuffer2+0x9b/0x1b0 [i915]
[ 3194.138447]  ? i915_gem_execbuffer+0x2b0/0x2b0 [i915]
[ 3194.143478]  drm_ioctl_kernel+0x64/0xb0
[ 3194.147298]  drm_ioctl+0x2cd/0x390
[ 3194.150710]  ? i915_gem_execbuffer+0x2b0/0x2b0 [i915]
[ 3194.155741]  ? finish_task_switch+0xa5/0x210
[ 3194.159993]  ? finish_task_switch+0x6a/0x210
[ 3194.164247]  do_vfs_ioctl+0x90/0x670
[ 3194.167806]  ? entry_SYSCALL_64_fastpath+0x5/0xb1
[ 3194.172492]  ? __this_cpu_preempt_check+0x13/0x20
[ 3194.177176]  ? trace_hardirqs_on_caller+0xe7/0x1c0
[ 3194.181946]  SyS_ioctl+0x3c/0x70
[ 3194.185159]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[ 3194.189756] RIP: 0033:0x7fd2b76a8587
[ 3194.193314] RSP: 002b:00007fff074845b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 3194.200855] RAX: ffffffffffffffda RBX: ffffffff8146da43 RCX: 00007fd2b76a8587
[ 3194.207962] RDX: 00007fff074846e0 RSI: 0000000040406469 RDI: 0000000000000003
[ 3194.215068] RBP: ffffc9000176bf88 R08: 0000000000000000 R09: 0000000000000003
[ 3194.222175] R10: 00007fd2b796bb58 R11: 0000000000000246 R12: 00007fff07484880
[ 3194.229280] R13: 0000000000000003 R14: 0000000040406469 R15: 0000000000000000
[ 3194.236386]  ? __this_cpu_preempt_check+0x13/0x20
[ 3194.241070] Code: 24 b0 00 00 00 48 85 c9 0f 84 6c ff ff ff 8b 41 20 85 c0 7e 73 83 e8 01 89 41 20 41 8b 84 24 e8 00 00 00 a8 0f 0f 85 5f ff ff ff <0f> 0b 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d f3 c3 49 8b 84
[ 3194.259943] RIP: eb_release_vmas.isra.6+0x119/0x180 [i915] RSP: ffffc9000176ba80
[ 3194.268047] ---[ end trace 1d7348c6575d8800 ]---
[ 3673.658819] softdog: Initiating panic
[ 3673.662471] Kernel panic - not syncing: Software Watchdog Timer expired
[ 3673.669066] Kernel Offset: disabled
[ 3673.672541] Rebooting in 1 seconds..

Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index fe3e0d40034c..f30675c6bc4a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -560,9 +560,6 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
 		eb->args->flags |= __EXEC_HAS_RELOC;
 	}
 
-	entry->flags |= __EXEC_OBJECT_HAS_PIN;
-	GEM_BUG_ON(eb_vma_misplaced(entry, vma));
-
 	if (unlikely(entry->flags & EXEC_OBJECT_NEEDS_FENCE)) {
 		err = i915_vma_get_fence(vma);
 		if (unlikely(err)) {
@@ -574,6 +571,9 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
 			entry->flags |= __EXEC_OBJECT_HAS_FENCE;
 	}
 
+	entry->flags |= __EXEC_OBJECT_HAS_PIN;
+	GEM_BUG_ON(eb_vma_misplaced(entry, vma));
+
 	return 0;
 }
 
-- 
2.13.3

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

[PATCH 3/4] drm/i915: Only skip updating execobject.offset after error

[Chris Wilson]

I was being overly paranoid in not updating the execobject.offset after
performing the fallback copy where we set reloc.presumed_offset to -1.
The thinking was to ensure that a subsequent NORELOC execbuf would be
forced to process the invalid relocations. However this is overkill so
long as we *only* update the execobject.offset following a successful
update of the relocation value witin the batch. If we have to repeat the
execbuf due to a later interruption, then we may skip the relocations on
the second pass (honouring NORELOC) since the execobject.offset match
the actual offsets (even though reloc.presumed_offset is garbage).

Subsequent calls to execbuf with NORELOC should themselves ensure that
the reloc.presumed_offset have been corrected in case of future
migration.

Reporting back the actual execobject.offset, even when
reloc.presumed_offset is garbage, ensures that reuse of those objects
use the latest information to avoid relocations.

Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101635
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f30675c6bc4a..ae47e1837415 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1776,7 +1776,7 @@ static noinline int eb_relocate_slow(struct i915_execbuffer *eb)
 		}
 	}
 
-	return err ?: have_copy;
+	return err;
 }
 
 static int eb_relocate(struct i915_execbuffer *eb)
@@ -2210,7 +2210,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		goto err_rpm;
 
 	err = eb_relocate(&eb);
-	if (err)
+	if (err) {
 		/*
 		 * If the user expects the execobject.offset and
 		 * reloc.presumed_offset to be an exact match,
@@ -2219,8 +2219,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		 * relocation.
 		 */
 		args->flags &= ~__EXEC_HAS_RELOC;
-	if (err < 0)
 		goto err_vma;
+	}
 
 	if (unlikely(eb.batch->exec_entry->flags & EXEC_OBJECT_WRITE)) {
 		DRM_DEBUG("Attempting to use self-modifying batch buffer\n");
-- 
2.13.3

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

[PATCH 4/4] drm/i915: Force CPU synchronisation even if userspace requests ASYNC

[Chris Wilson]

The goal here was to minimise doing any thing or any check inside the
kernel that was not strictly required. For a userspace that assumes
complete control over the cache domains, the kernel is usually using
outdated information and may trigger clflushes where none were
required.

However, swapping is a situation where userspace has no knowledge of the
domain transfer, and will leave the object in the CPU cache. The kernel
must flush this out to the backing storage prior to use with the GPU. As
we use an asynchronous task tracked by an implicit fence for this, we
also need to cancel the ASYNC flag on the object so that the object will
wait for the clflush to complete before being executed. This also absolves
userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915:
Enable userspace to opt-out of implicit fencing") that its needed to ensure
that the object was out of the CPU cache prior to use on the GPU.

Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/i915_gem_clflush.c    |  7 ++++---
 drivers/gpu/drm/i915/i915_gem_clflush.h    |  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++++++----
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
index 152f16c11878..348b29a845c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -114,7 +114,7 @@ i915_clflush_notify(struct i915_sw_fence *fence,
 	return NOTIFY_DONE;
 }
 
-void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
+bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 			     unsigned int flags)
 {
 	struct clflush *clflush;
@@ -128,7 +128,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 	 */
 	if (!i915_gem_object_has_struct_page(obj)) {
 		obj->cache_dirty = false;
-		return;
+		return false;
 	}
 
 	/* If the GPU is snooping the contents of the CPU cache,
@@ -140,7 +140,7 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 	 * tracking.
 	 */
 	if (!(flags & I915_CLFLUSH_FORCE) && obj->cache_coherent)
-		return;
+		return false;
 
 	trace_i915_gem_object_clflush(obj);
 
@@ -179,4 +179,5 @@ void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 	}
 
 	obj->cache_dirty = false;
+	return true;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.h b/drivers/gpu/drm/i915/i915_gem_clflush.h
index 2455a7820937..f390247561b3 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.h
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.h
@@ -28,7 +28,7 @@
 struct drm_i915_private;
 struct drm_i915_gem_object;
 
-void i915_gem_clflush_object(struct drm_i915_gem_object *obj,
+bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 			     unsigned int flags);
 #define I915_CLFLUSH_FORCE BIT(0)
 #define I915_CLFLUSH_SYNC BIT(1)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ae47e1837415..5fa44767c29e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1826,7 +1826,7 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 	int err;
 
 	for (i = 0; i < count; i++) {
-		const struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
+		struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
 		struct i915_vma *vma = exec_to_vma(entry);
 		struct drm_i915_gem_object *obj = vma->obj;
 
@@ -1842,12 +1842,14 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
 			eb->request->capture_list = capture;
 		}
 
+		if (unlikely(obj->cache_dirty && !obj->cache_coherent)) {
+			if (i915_gem_clflush_object(obj, 0))
+				entry->flags &= ~EXEC_OBJECT_ASYNC;
+		}
+
 		if (entry->flags & EXEC_OBJECT_ASYNC)
 			goto skip_flushes;
 
-		if (unlikely(obj->cache_dirty && !obj->cache_coherent))
-			i915_gem_clflush_object(obj, 0);
-
 		err = i915_gem_request_await_object
 			(eb->request, obj, entry->flags & EXEC_OBJECT_WRITE);
 		if (err)
-- 
2.13.3

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

✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Remove assertion from raw __i915_vma_unpin()

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Remove assertion from raw __i915_vma_unpin()
URL   : https://patchwork.freedesktop.org/series/27714/
State : warning

== Summary ==

Series 27714v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27714/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_ringfill:
        Subgroup basic-default-forked:
                pass       -> SKIP       (fi-bxt-j4205)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:433s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:354s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:537s
fi-bxt-j4205     total:279  pass:259  dwarn:0   dfail:0   fail:0   skip:20  time:515s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:484s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:597s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:444s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:501s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:479s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:577s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:583s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:569s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:467s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:587s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:472s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:438s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:499s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:551s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:406s

2a4f73050a961e1a176d35561e39386da9ea9c67 drm-tip: 2017y-07m-21d-15h-30m-47s UTC integration manifest
9865035 drm/i915: Force CPU synchronisation even if userspace requests ASYNC
9eafebb drm/i915: Only skip updating execobject.offset after error
ad29610 drm/i915: Only mark the execobject as pinned on success
fd9bec2 drm/i915: Remove assertion from raw __i915_vma_unpin()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5259/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Remove assertion from raw __i915_vma_unpin()

[Joonas Lahtinen]

On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote:
> After we detect a i915_vma pin overflow, we call __i915_vma_unpin to
> cleanup. However, on an overflow the pin_count bitfield will be zero,
> triggering an assertion, even though we the intention is to merely warn
> and report the error back to the user (as historically the culprit has
> be a leak in the display code).
> 
> Fixes: 20dfbde463c8 ("drm/i915: Wrap vma->pin_count accessors with small inline helpers")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Only mark the execobject as pinned on success

[Joonas Lahtinen]

On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote:
> If we fail to acquire a fence (for old school fenced GPU access) then we
> unwind the vma reservation, including its pin. However, we were making
> the execobject as holding the pin before erring out, leading to a double
> unpin:
> 
> [ 3193.991802] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:287!
> [ 3193.998131] invalid opcode: 0000 [#1] PREEMPT SMP
> [ 3194.002816] Modules linked in: snd_hda_intel i915 vgem snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich mei_me e1000e mei prime_numbers ptp pps_core [last unloaded: i915]
> [ 3194.022841] CPU: 0 PID: 8123 Comm: kms_flip Tainted: G     U          4.13.0-rc1-CI-CI_DRM_471+ #1
> [ 3194.031765] Hardware name: Dell Inc. OptiPlex 755                 /0PU052, BIOS A04 11/05/2007
> [ 3194.040343] task: ffff8800785d4c40 task.stack: ffffc90001768000
> [ 3194.046339] RIP: 0010:eb_release_vmas.isra.6+0x119/0x180 [i915]
> [ 3194.052234] RSP: 0018:ffffc9000176ba80 EFLAGS: 00010246
> [ 3194.057439] RAX: 00000000000003c0 RBX: ffff8800710fc2d8 RCX: ffff8800588e4f48
> [ 3194.064546] RDX: ffffffff1fffffff RSI: 00000000ffffffff RDI: ffff8800588e00d0
> [ 3194.071654] RBP: ffffc9000176bab0 R08: 0000000000000000 R09: 0000000000000000
> [ 3194.078761] R10: 0000000000000040 R11: 0000000000000001 R12: ffff880060822f00
> [ 3194.085867] R13: 0000000000000310 R14: 00000000000003b8 R15: ffffc9000176bbb0
> [ 3194.092975] FS:  00007fd2b94aba40(0000) GS:ffff88007d200000(0000) knlGS:0000000000000000
> [ 3194.101033] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 3194.106754] CR2: 00007ffbec3ff000 CR3: 0000000074e67000 CR4: 00000000000006f0
> [ 3194.113861] Call Trace:
> [ 3194.116321]  eb_relocate_slow+0x67/0x4e0 [i915]
> [ 3194.120861]  i915_gem_do_execbuffer+0x429/0x1260 [i915]
> [ 3194.126070]  ? lock_acquire+0xb5/0x210
> [ 3194.129803]  ? __might_fault+0x39/0x90
> [ 3194.133563]  i915_gem_execbuffer2+0x9b/0x1b0 [i915]
> [ 3194.138447]  ? i915_gem_execbuffer+0x2b0/0x2b0 [i915]
> [ 3194.143478]  drm_ioctl_kernel+0x64/0xb0
> [ 3194.147298]  drm_ioctl+0x2cd/0x390
> [ 3194.150710]  ? i915_gem_execbuffer+0x2b0/0x2b0 [i915]
> [ 3194.155741]  ? finish_task_switch+0xa5/0x210
> [ 3194.159993]  ? finish_task_switch+0x6a/0x210
> [ 3194.164247]  do_vfs_ioctl+0x90/0x670
> [ 3194.167806]  ? entry_SYSCALL_64_fastpath+0x5/0xb1
> [ 3194.172492]  ? __this_cpu_preempt_check+0x13/0x20
> [ 3194.177176]  ? trace_hardirqs_on_caller+0xe7/0x1c0
> [ 3194.181946]  SyS_ioctl+0x3c/0x70
> [ 3194.185159]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [ 3194.189756] RIP: 0033:0x7fd2b76a8587
> [ 3194.193314] RSP: 002b:00007fff074845b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 3194.200855] RAX: ffffffffffffffda RBX: ffffffff8146da43 RCX: 00007fd2b76a8587
> [ 3194.207962] RDX: 00007fff074846e0 RSI: 0000000040406469 RDI: 0000000000000003
> [ 3194.215068] RBP: ffffc9000176bf88 R08: 0000000000000000 R09: 0000000000000003
> [ 3194.222175] R10: 00007fd2b796bb58 R11: 0000000000000246 R12: 00007fff07484880
> [ 3194.229280] R13: 0000000000000003 R14: 0000000040406469 R15: 0000000000000000
> [ 3194.236386]  ? __this_cpu_preempt_check+0x13/0x20
> [ 3194.241070] Code: 24 b0 00 00 00 48 85 c9 0f 84 6c ff ff ff 8b 41 20 85 c0 7e 73 83 e8 01 89 41 20 41 8b 84 24 e8 00 00 00 a8 0f 0f 85 5f ff ff ff <0f> 0b 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d f3 c3 49 8b 84
> [ 3194.259943] RIP: eb_release_vmas.isra.6+0x119/0x180 [i915] RSP: ffffc9000176ba80
> [ 3194.268047] ---[ end trace 1d7348c6575d8800 ]---
> [ 3673.658819] softdog: Initiating panic
> [ 3673.662471] Kernel panic - not syncing: Software Watchdog Timer expired
> [ 3673.669066] Kernel Offset: disabled
> [ 3673.672541] Rebooting in 1 seconds..
> 
> Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
> Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Only skip updating execobject.offset after error

[Joonas Lahtinen]

On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote:
> I was being overly paranoid in not updating the execobject.offset after
> performing the fallback copy where we set reloc.presumed_offset to -1.
> The thinking was to ensure that a subsequent NORELOC execbuf would be
> forced to process the invalid relocations. However this is overkill so
> long as we *only* update the execobject.offset following a successful
> update of the relocation value witin the batch. If we have to repeat the
> execbuf due to a later interruption, then we may skip the relocations on
> the second pass (honouring NORELOC) since the execobject.offset match
> the actual offsets (even though reloc.presumed_offset is garbage).
> 
> Subsequent calls to execbuf with NORELOC should themselves ensure that
> the reloc.presumed_offset have been corrected in case of future
> migration.
> 
> Reporting back the actual execobject.offset, even when
> reloc.presumed_offset is garbage, ensures that reuse of those objects
> use the latest information to avoid relocations.
> 
> Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101635
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Force CPU synchronisation even if userspace requests ASYNC

[Joonas Lahtinen]

On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote:
> The goal here was to minimise doing any thing or any check inside the
> kernel that was not strictly required. For a userspace that assumes
> complete control over the cache domains, the kernel is usually using
> outdated information and may trigger clflushes where none were
> required.
> 
> However, swapping is a situation where userspace has no knowledge of the
> domain transfer, and will leave the object in the CPU cache. The kernel
> must flush this out to the backing storage prior to use with the GPU. As
> we use an asynchronous task tracked by an implicit fence for this, we
> also need to cancel the ASYNC flag on the object so that the object will
> wait for the clflush to complete before being executed. This also absolves
> userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915:
> Enable userspace to opt-out of implicit fencing") that its needed to ensure
> that the object was out of the CPU cache prior to use on the GPU.
> 
> Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

All explained out, makes perfect sense.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Force CPU synchronisation even if userspace requests ASYNC

[Chris Wilson]

Quoting Joonas Lahtinen (2017-07-26 11:19:26)
> On pe, 2017-07-21 at 15:50 +0100, Chris Wilson wrote:
> > The goal here was to minimise doing any thing or any check inside the
> > kernel that was not strictly required. For a userspace that assumes
> > complete control over the cache domains, the kernel is usually using
> > outdated information and may trigger clflushes where none were
> > required.
> > 
> > However, swapping is a situation where userspace has no knowledge of the
> > domain transfer, and will leave the object in the CPU cache. The kernel
> > must flush this out to the backing storage prior to use with the GPU. As
> > we use an asynchronous task tracked by an implicit fence for this, we
> > also need to cancel the ASYNC flag on the object so that the object will
> > wait for the clflush to complete before being executed. This also absolves
> > userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915:
> > Enable userspace to opt-out of implicit fencing") that its needed to ensure
> > that the object was out of the CPU cache prior to use on the GPU.
> > 
> > Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
> 
> All explained out, makes perfect sense.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

And finally pushed, hopefully making it to v4.12.y before too many people
hit the vk-on-(bsw,bxt,glk) bug.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx