[Intel-gfx] [PATCH] drm/i915: Remove the vma from the object list upon close

[Tvrtko Ursulin]

On 04/11/2016 16:12, Chris Wilson wrote:
> Currently, the vma is being unlink from the object lookup on destroy.
> However, we are meant to be decoupling it upon close so that the user
> cannot access the closed vma whilst it remains active on the GPU.
>
> [   34.074858] kernel BUG at drivers/gpu/drm/i915/i915_gem_gtt.c:3561!
> [   34.074875] invalid opcode: 0000 [#1] PREEMPT SMP
> [   34.074888] Modules linked in: snd_hda_intel i915 x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel lpc_ich mei_me mei snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_codec snd_hwdep snd_hda_core i2c_designware_platform i2c_designware_core snd_pcm e1000e ptp pps_core sdhci_acpi sdhci mmc_core i2c_hid [last unloaded: i915]
> [   34.075010] CPU: 1 PID: 6224 Comm: gem_close_race Tainted: G     U          4.9.0-rc3-CI-CI_DRM_1800+ #1
> [   34.075034] Hardware name:                  /NUC5i7RYB, BIOS RYBDWi35.86A.0355.2016.0224.1501 02/24/2016
> [   34.075057] task: ffff8802459a8040 task.stack: ffffc90000524000
> [   34.075074] RIP: 0010:[<ffffffffa0392cbc>]  [<ffffffffa0392cbc>] i915_gem_obj_lookup_or_create_vma+0x8c/0xc0 [i915]
> [   34.075118] RSP: 0018:ffffc90000527b68  EFLAGS: 00010202
> [   34.075135] RAX: ffff8802426c5e40 RBX: 0000000000000000 RCX: ffff8802447fc2a8
> [   34.075158] RDX: 0000000000000000 RSI: ffff8802447fc2a8 RDI: ffff880248a4a880
> [   34.075181] RBP: ffffc90000527b88 R08: 0000000000000008 R09: 0000000000000000
> [   34.075203] R10: 0000000000000001 R11: 0000000000000000 R12: ffff880248a4a880
> [   34.075225] R13: ffff8802447fc2a8 R14: ffff880243e9afa8 R15: ffff880248a4a9c8
> [   34.075248] FS:  00007f9b43e59740(0000) GS:ffff880256c80000(0000) knlGS:0000000000000000
> [   34.075273] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   34.075292] CR2: 00007f9b43419140 CR3: 000000024455d000 CR4: 00000000003406e0
> [   34.075314] Stack:
> [   34.075323]  0000000000000000 ffffc90000527bd0 ffff880243cb8008 ffff880243e9afa8
> [   34.075353]  ffffc90000527c08 ffffffffa03874c7 ffffc90000527bb8 ffff880243e9afa8
> [   34.075383]  ffff880243e9afb0 ffffc90000527e10 ffff8802447fc2a8 ffff880243cb8040
> [   34.075414] Call Trace:
> [   34.075435]  [<ffffffffa03874c7>] eb_lookup_vmas.isra.7+0x247/0x330 [i915]
> [   34.075468]  [<ffffffffa0388c34>] i915_gem_do_execbuffer.isra.15+0x604/0x1a10 [i915]
> [   34.075507]  [<ffffffffa039c957>] ? i915_gem_object_get_sg+0x347/0x380 [i915]
> [   34.075532]  [<ffffffff811a69ce>] ? __might_fault+0x3e/0x90
> [   34.075562]  [<ffffffffa038a430>] i915_gem_execbuffer2+0xc0/0x250 [i915]
> [   34.075585]  [<ffffffff81552926>] drm_ioctl+0x1f6/0x480
> [   34.075604]  [<ffffffff8100107a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [   34.075635]  [<ffffffffa038a370>] ? i915_gem_execbuffer+0x330/0x330 [i915]
> [   34.075658]  [<ffffffff81202d2e>] do_vfs_ioctl+0x8e/0x690
> [   34.075677]  [<ffffffff8181582d>] ? _raw_spin_unlock_irqrestore+0x3d/0x60
> [   34.075700]  [<ffffffff810fcd51>] ? SyS_timer_settime+0x141/0x1e0
> [   34.075721]  [<ffffffff810d6de2>] ? trace_hardirqs_on_caller+0x122/0x1b0
> [   34.075742]  [<ffffffff8120336c>] SyS_ioctl+0x3c/0x70
> [   34.075760]  [<ffffffff8181602e>] entry_SYSCALL_64_fastpath+0x1c/0xb1
> [   34.075781] Code: 44 a0 48 c7 c2 9a 7e 43 a0 be e0 0d 00 00 48 c7 c7 a0 45 44 a0 e8 55 b8 ce e0 48 85 db 74 a3 49 83 bd f8 03 00 00 00 74 99 0f 0b <0f> 0b 48 89 da 4c 89 ee 4c 89 e7 e8 04 a9 ff ff 48 89 da 49 89
> [   34.075955] RIP  [<ffffffffa0392cbc>] i915_gem_obj_lookup_or_create_vma+0x8c/0xc0 [i915]
> [   34.075994]  RSP <ffffc90000527b68>
>
> Testcase: igt/gem_close_race/basic-threads
> Fixes: db6c2b4151f2 ("drm/i915: Store the vma in an rbtree...")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 52999e51a946..c5e77e040627 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3403,7 +3403,6 @@ void i915_vma_destroy(struct i915_vma *vma)
>  	GEM_BUG_ON(!i915_vma_is_closed(vma));
>  	GEM_BUG_ON(vma->fence);
>
> -	rb_erase(&vma->obj_node, &vma->obj->vma_tree);
>  	list_del(&vma->vm_link);
>  	if (!i915_vma_is_ggtt(vma))
>  		i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm));
> @@ -3416,7 +3415,9 @@ void i915_vma_close(struct i915_vma *vma)
>  	GEM_BUG_ON(i915_vma_is_closed(vma));
>  	vma->flags |= I915_VMA_CLOSED;
>
> -	list_del_init(&vma->obj_link);
> +	list_del(&vma->obj_link);
> +	rb_erase(&vma->obj_node, &vma->obj->vma_tree);
> +
>  	if (!i915_vma_is_active(vma) && !i915_vma_is_pinned(vma))
>  		WARN_ON(i915_vma_unbind(vma));
>  }
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko