[Intel-gfx] [PATCH] drm/i915: we can't move_to_active before intel_ring_begin
Daniel Vetter
daniel.vetter at ffwll.ch
Wed Sep 25 16:31:48 CEST 2013
Otherwise we can die in a fire of not-yet-allocated lazy requests when
we expect them to be there:
[ 4405.463113] ------------[ cut here ]------------
[ 4405.464769] kernel BUG at /home/user/src/linux/drivers/gpu/drm/i915/intel_ringbuffer.h:268!
[ 4405.466392] invalid opcode: 0000 [#1] PREEMPT SMP
[ 4405.468007] Modules linked in: snd_hda_codec_hdmi hid_generic snd_hda_codec_realtek usbhid hid iTCO_wdt snd_hda_intel psmouse i2c_i801 snd_hda_codec pcspkr snd_hwdep i915 snd_pcm snd_page_alloc snd_timer cfbfillrect snd cfbimgblt lpc_ich cfbcopyarea soundcore drm_kms_helper mfd_core evdev wmi
[ 4405.471387] CPU: 5 PID: 9300 Comm: gem_storedw_bat Not tainted 3.12.0-rc2-drm_vbl+ #1
[ 4405.473047] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0100.2013.0517.0942 05/17/2013
[ 4405.474712] task: ffff8800618d4b00 ti: ffff88010a806000 task.ti: ffff88010a806000
[ 4405.476370] RIP: 0010:[<ffffffffa009ffa9>] [<ffffffffa009ffa9>] i915_vma_move_to_active+0x1b9/0x1e0 [i915]
[ 4405.478045] RSP: 0018:ffff88010a807be8 EFLAGS: 00010246
[ 4405.479689] RAX: ffff88011a681000 RBX: ffff8800b364f9c0 RCX: ffff88011902b898
[ 4405.481321] RDX: ffff8800d4a1b6e0 RSI: ffff8800d4a1a8b8 RDI: ffff88011902b840
[ 4405.482932] RBP: ffff88010a807c08 R08: 0000000000000001 R09: 0000000000000000
[ 4405.484526] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800d4a1a8b8
[ 4405.486100] R13: 0000000000000000 R14: ffff8800d4a18000 R15: ffff8800b364f9c0
[ 4405.487664] FS: 00007f36c1a738c0(0000) GS:ffff88011f340000(0000) knlGS:0000000000000000
[ 4405.489216] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4405.490747] CR2: 00007fff1b28ea30 CR3: 0000000119e0d000 CR4: 00000000001407e0
[ 4405.492276] Stack:
[ 4405.493774] ffff88010a807dd8 ffff8800d4a1a8b8 ffff8800d3c1c400 ffffffffa00ac060
[ 4405.495276] ffff88010a807d28 ffffffffa00aa0db ffff88010a807cb8 ffffffff810aa4e4
[ 4405.496776] 0000000000000003 ffff880000000001 ffff8800618d50e8 ffffffff81a9da00
[ 4405.498265] Call Trace:
[ 4405.499735] [<ffffffffa00ac060>] ? gen6_ppgtt_insert_entries+0x1a0/0x1a0 [i915]
[ 4405.501218] [<ffffffffa00aa0db>] i915_gem_do_execbuffer.clone.18+0x65b/0x12e0 [i915]
[ 4405.502685] [<ffffffff810aa4e4>] ? lock_release_non_nested+0xa4/0x360
[ 4405.504134] [<ffffffffa00ab298>] i915_gem_execbuffer2+0xa8/0x290 [i915]
[ 4405.505573] [<ffffffff813552b9>] drm_ioctl+0x419/0x5c0
[ 4405.506991] [<ffffffff81128b12>] ? handle_mm_fault+0x352/0xa00
[ 4405.508399] [<ffffffffa00ab1f0>] ? i915_gem_execbuffer+0x490/0x490 [i915]
[ 4405.509792] [<ffffffff8103cd2c>] ? __do_page_fault+0x1fc/0x4b0
[ 4405.511170] [<ffffffff81165e66>] do_vfs_ioctl+0x96/0x560
[ 4405.512533] [<ffffffff81512163>] ? error_sti+0x5/0x6
[ 4405.513878] [<ffffffff81511d0d>] ? retint_swapgs+0xe/0x13
[ 4405.515208] [<ffffffff811663d1>] SyS_ioctl+0xa1/0xb0
[ 4405.516522] [<ffffffff8129ddee>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[ 4405.517830] [<ffffffff81512792>] system_call_fastpath+0x16/0x1b
[ 4405.519119] Code: f6 83 40 01 00 00 01 0f 85 11 ff ff ff b8 01 00 00 00 f0 0f c1 03 ff c0 83 f8 01 7e 10 80 8b 40 01 00 00 01 e9 f5 fe ff ff 0f 0b <0f> 0b 80 3d 61 55 08 00 01 74 e7 be 2f 00 00 00 48 c7 c7 18 03
[ 4405.520610] RIP [<ffffffffa009ffa9>] i915_vma_move_to_active+0x1b9/0x1e0 [i915]
[ 4405.522001] RSP <ffff88010a807be8>
[ 4405.523367] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0)
[ 4405.524751] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.526152] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3]
[ 4405.531096] [drm:intel_crtc_set_config], [CRTC:3] [FB:50] #connectors=1 (x y) (0 0)
[ 4405.532491] [drm:intel_set_config_compute_mode_changes], computed changes for [CRTC:3], mode_changed=0, fb_changed=0
[ 4405.533903] [drm:intel_modeset_stage_output_state], [CONNECTOR:16:HDMI-A-2] to [CRTC:3]
[ 4405.538888] ---[ end trace 53d1b708421bb5b3 ]---
This regression has been introduced in from Ben Widawsky's ppgtt/vma
enabling patch "drm/i915: Use the new vm [un]bind functions".
This should be exercised by
igt/gem_storedw_batches_loop/secure-dispatch.
v2: Clarify the copy&pasta comment and update it to suit the new
location of the move_to_active call for the batch vma.
Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Ben Widawsky <ben at bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 34 ++++++++++++++++++++----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 88c924f..f540207 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -929,6 +929,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
drm_i915_private_t *dev_priv = dev->dev_private;
struct eb_vmas *eb;
struct drm_i915_gem_object *batch_obj;
+ struct i915_vma *batch_vma;
struct drm_clip_rect *cliprects = NULL;
struct intel_ring_buffer *ring;
struct i915_ctx_hang_stats *hs;
@@ -1082,7 +1083,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
goto err;
/* take note of the batch buffer before we might reorder the lists */
- batch_obj = list_entry(eb->vmas.prev, struct i915_vma, exec_list)->obj;
+ batch_vma = list_entry(eb->vmas.prev, struct i915_vma, exec_list);
+ batch_obj = batch_vma->obj;
/* Move the objects en-masse into the GTT, evicting if necessary. */
need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
@@ -1118,6 +1120,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (flags & I915_DISPATCH_SECURE) {
struct i915_address_space *ggtt = obj_to_ggtt(batch_obj);
+ batch_vma = i915_gem_obj_lookup_or_create_vma(batch_obj,
+ ggtt);
+ if (IS_ERR(batch_vma)) {
+ DRM_DEBUG("Failed to lookup ggtt batch VMA\n");
+ ret = PTR_ERR(batch_vma);
+ goto err;
+ }
+
/* Assuming all privileged batches are in the global GTT means
* we need to make sure we have a global gtt offset, as well as
* the PTEs mapped. As mentioned above, we can forego this on
@@ -1128,21 +1138,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (ret)
goto err;
- ggtt->bind_vma(i915_gem_obj_to_ggtt(batch_obj),
+ ggtt->bind_vma(batch_vma,
batch_obj->cache_level,
GLOBAL_BIND);
- /* Since the active list is per VM, we need to make sure this
- * VMA ends up on the GGTT's active list to avoid premature
- * eviction.
- */
- i915_vma_move_to_active(i915_gem_obj_to_ggtt(batch_obj), ring);
-
i915_gem_object_unpin(batch_obj);
+ }
- exec_start += i915_gem_obj_ggtt_offset(batch_obj);
- } else
- exec_start += i915_gem_obj_offset(batch_obj, vm);
+ exec_start += batch_vma->node.start;
ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
if (ret)
@@ -1209,6 +1212,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
i915_gem_execbuffer_move_to_active(&eb->vmas, ring);
+ /*
+ * Since the active list is per VM and the batch might be executed from
+ * the global GTT we need to make sure that the VMA ends up on the
+ * active list there, too, to avoid premature eviction. If the patch is
+ * in the same address space doing a 2nd move_to_active doesn't hurt
+ * since this is idempotent.
+ */
+ i915_vma_move_to_active(batch_vma, ring);
+
i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
err:
--
1.8.4.rc3
More information about the Intel-gfx
mailing list