OSDN Git Service

drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
authorChris Wilson <chris@chris-wilson.co.uk>
Thu, 23 May 2019 06:49:33 +0000 (07:49 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Thu, 23 May 2019 20:12:12 +0000 (21:12 +0100)
We rearranged the vm_destroy_ioctl to avoid taking struct_mutex, little
realising that buried underneath the gen6 ppgtt release path was a
struct_mutex requirement (to remove its GGTT vma). Until that
struct_mutex is vanquished, take a detour in gen6_ppgtt_cleanup to do
the i915_vma_destroy from inside a worker under the struct_mutex.

<4> [257.740160] WARN_ON(debug_locks && !lock_is_held(&(&vma->vm->i915->drm.struct_mutex)->dep_map))
<4> [257.740213] WARNING: CPU: 3 PID: 1507 at drivers/gpu/drm/i915/i915_vma.c:841 i915_vma_destroy+0x1ae/0x3a0 [i915]
<4> [257.740214] Modules linked in: snd_hda_codec_hdmi i915 x86_pkg_temp_thermal mei_hdcp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core r8169 realtek snd_pcm mei_me mei prime_numbers lpc_ich
<4> [257.740224] CPU: 3 PID: 1507 Comm: gem_vm_create Tainted: G     U            5.2.0-rc1-CI-CI_DRM_6118+ #1
<4> [257.740225] Hardware name: MSI MS-7924/Z97M-G43(MS-7924), BIOS V1.12 02/15/2016
<4> [257.740249] RIP: 0010:i915_vma_destroy+0x1ae/0x3a0 [i915]
<4> [257.740250] Code: 00 00 00 48 81 c7 c8 00 00 00 e8 ed 08 f0 e0 85 c0 0f 85 78 fe ff ff 48 c7 c6 e8 ec 30 a0 48 c7 c7 da 55 33 a0 e8 42 8c e9 e0 <0f> 0b 8b 83 40 01 00 00 85 c0 0f 84 63 fe ff ff 48 c7 c1 c1 58 33
<4> [257.740251] RSP: 0018:ffffc90000aafc68 EFLAGS: 00010282
<4> [257.740252] RAX: 0000000000000000 RBX: ffff8883f7957840 RCX: 0000000000000003
<4> [257.740253] RDX: 0000000000000046 RSI: 0000000000000006 RDI: ffffffff8212d1b9
<4> [257.740254] RBP: ffffc90000aafcc8 R08: 0000000000000000 R09: 0000000000000000
<4> [257.740255] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8883f4d5c2a8
<4> [257.740256] R13: ffff8883f4d5d680 R14: ffff8883f4d5c668 R15: ffff8883f4d5c2f0
<4> [257.740257] FS:  00007f777fa8fe40(0000) GS:ffff88840f780000(0000) knlGS:0000000000000000
<4> [257.740258] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [257.740259] CR2: 00007f777f6522b0 CR3: 00000003c612a006 CR4: 00000000001606e0
<4> [257.740260] Call Trace:
<4> [257.740283]  gen6_ppgtt_cleanup+0x25/0x60 [i915]
<4> [257.740306]  i915_ppgtt_release+0x102/0x290 [i915]
<4> [257.740330]  i915_gem_vm_destroy_ioctl+0x7c/0xa0 [i915]
<4> [257.740376]  ? i915_gem_vm_create_ioctl+0x160/0x160 [i915]
<4> [257.740379]  drm_ioctl_kernel+0x83/0xf0
<4> [257.740382]  drm_ioctl+0x2f3/0x3b0
<4> [257.740422]  ? i915_gem_vm_create_ioctl+0x160/0x160 [i915]
<4> [257.740426]  ? _raw_spin_unlock_irqrestore+0x39/0x60
<4> [257.740430]  do_vfs_ioctl+0xa0/0x6e0
<4> [257.740433]  ? lock_acquire+0xa6/0x1c0
<4> [257.740436]  ? __task_pid_nr_ns+0xb9/0x1f0
<4> [257.740439]  ksys_ioctl+0x35/0x60
<4> [257.740441]  __x64_sys_ioctl+0x11/0x20
<4> [257.740443]  do_syscall_64+0x55/0x1c0
<4> [257.740445]  entry_SYSCALL_64_after_hwframe+0x49/0xbe

References: e0695db7298e ("drm/i915: Create/destroy VM (ppGTT) for use with contexts")
Fixes: 7f3f317a66ca ("drm/i915: Restore control over ppgtt for context creation ABI")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190523064933.23604-1-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_gem_gtt.c
drivers/gpu/drm/i915/i915_gem_gtt.h

index 9ed41ae..8d8a4b0 100644 (file)
@@ -1828,11 +1828,34 @@ static void gen6_ppgtt_free_pd(struct gen6_hw_ppgtt *ppgtt)
                        free_pt(&ppgtt->base.vm, pt);
 }
 
+struct gen6_ppgtt_cleanup_work {
+       struct work_struct base;
+       struct i915_vma *vma;
+};
+
+static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
+{
+       struct gen6_ppgtt_cleanup_work *work =
+               container_of(wrk, typeof(*work), base);
+       /* Side note, vma->vm is the GGTT not the ppgtt we just destroyed! */
+       struct drm_i915_private *i915 = work->vma->vm->i915;
+
+       mutex_lock(&i915->drm.struct_mutex);
+       i915_vma_destroy(work->vma);
+       mutex_unlock(&i915->drm.struct_mutex);
+
+       kfree(work);
+}
+
 static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
        struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
+       struct gen6_ppgtt_cleanup_work *work = ppgtt->work;
 
-       i915_vma_destroy(ppgtt->vma);
+       /* FIXME remove the struct_mutex to bring the locking under control */
+       INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
+       work->vma = ppgtt->vma;
+       schedule_work(&work->base);
 
        gen6_ppgtt_free_pd(ppgtt);
        gen6_ppgtt_free_scratch(vm);
@@ -2011,9 +2034,13 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
        ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode;
 
+       ppgtt->work = kmalloc(sizeof(*ppgtt->work), GFP_KERNEL);
+       if (!ppgtt->work)
+               goto err_free;
+
        err = gen6_ppgtt_init_scratch(ppgtt);
        if (err)
-               goto err_free;
+               goto err_work;
 
        ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
        if (IS_ERR(ppgtt->vma)) {
@@ -2025,6 +2052,8 @@ static struct i915_hw_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 err_scratch:
        gen6_ppgtt_free_scratch(&ppgtt->base.vm);
+err_work:
+       kfree(ppgtt->work);
 err_free:
        kfree(ppgtt);
        return ERR_PTR(err);
index 98fc710..3849603 100644 (file)
@@ -424,6 +424,8 @@ struct gen6_hw_ppgtt {
 
        unsigned int pin_count;
        bool scan_for_unused_pt;
+
+       struct gen6_ppgtt_cleanup_work *work;
 };
 
 #define __to_gen6_ppgtt(base) container_of(base, struct gen6_hw_ppgtt, base)