All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
@ 2019-05-23  6:49 Chris Wilson
  2019-05-23  9:21 ` Tvrtko Ursulin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-23  6:49 UTC (permalink / raw)
  To: intel-gfx

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>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9ed41aefb456..5a350251766a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1828,11 +1828,33 @@ 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);
+	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 +2033,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 +2051,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);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 98fc71053f7c..38496039456b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -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)
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23  6:49 [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup Chris Wilson
@ 2019-05-23  9:21 ` Tvrtko Ursulin
  2019-05-23 10:01   ` Tvrtko Ursulin
  2019-05-23 14:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23  9:21 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 23/05/2019 07:49, Chris Wilson wrote:
> 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>
> ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
>   drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
>   2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9ed41aefb456..5a350251766a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1828,11 +1828,33 @@ 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);
> +	struct drm_i915_private *i915 = work->vma->vm->i915;

Does the sequence need an extra reference on ppgtt for dereferencing the 
vm here? Alternatively storing i915 in the work struct could work around 
that.

Regards,

Tvrtko

> +
> +	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 +2033,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 +2051,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);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 98fc71053f7c..38496039456b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -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)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23  9:21 ` Tvrtko Ursulin
@ 2019-05-23 10:01   ` Tvrtko Ursulin
  2019-05-23 10:07     ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23 10:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 23/05/2019 10:21, Tvrtko Ursulin wrote:
> 
> On 23/05/2019 07:49, Chris Wilson wrote:
>> 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>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
>>   2 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 9ed41aefb456..5a350251766a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -1828,11 +1828,33 @@ 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);
>> +    struct drm_i915_private *i915 = work->vma->vm->i915;
> 
> Does the sequence need an extra reference on ppgtt for dereferencing the 
> vm here? Alternatively storing i915 in the work struct could work around 
> that.

Nope - vma has a pointer to vm as well which it will dereference. So if 
reference is needed it's needed - it looks like it is to me, since only 
contexts take them, which vma's still rely on, right?

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23 10:01   ` Tvrtko Ursulin
@ 2019-05-23 10:07     ` Chris Wilson
  2019-05-23 10:14       ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-05-23 10:07 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-05-23 11:01:09)
> 
> On 23/05/2019 10:21, Tvrtko Ursulin wrote:
> > 
> > On 23/05/2019 07:49, Chris Wilson wrote:
> >> 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>
> >> ---
> >>   drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
> >>   drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
> >>   2 files changed, 32 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> >> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> index 9ed41aefb456..5a350251766a 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> @@ -1828,11 +1828,33 @@ 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);
> >> +    struct drm_i915_private *i915 = work->vma->vm->i915;
> > 
> > Does the sequence need an extra reference on ppgtt for dereferencing the 
> > vm here? Alternatively storing i915 in the work struct could work around 
> > that.
> 
> Nope - vma has a pointer to vm as well which it will dereference. So if 
> reference is needed it's needed - it looks like it is to me, since only 
> contexts take them, which vma's still rely on, right?

Nah, vm is decidedly dead at this point, we just need to stuff the i915
pointer into the cleanup_work.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23 10:07     ` Chris Wilson
@ 2019-05-23 10:14       ` Chris Wilson
  2019-05-23 10:19         ` Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2019-05-23 10:14 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Chris Wilson (2019-05-23 11:07:50)
> Quoting Tvrtko Ursulin (2019-05-23 11:01:09)
> > 
> > On 23/05/2019 10:21, Tvrtko Ursulin wrote:
> > > 
> > > On 23/05/2019 07:49, Chris Wilson wrote:
> > >> 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>
> > >> ---
> > >>   drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
> > >>   drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
> > >>   2 files changed, 32 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > >> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >> index 9ed41aefb456..5a350251766a 100644
> > >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > >> @@ -1828,11 +1828,33 @@ 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);
> > >> +    struct drm_i915_private *i915 = work->vma->vm->i915;
> > > 
> > > Does the sequence need an extra reference on ppgtt for dereferencing the 
> > > vm here? Alternatively storing i915 in the work struct could work around 
> > > that.
> > 
> > Nope - vma has a pointer to vm as well which it will dereference. So if 
> > reference is needed it's needed - it looks like it is to me, since only 
> > contexts take them, which vma's still rely on, right?
> 
> Nah, vm is decidedly dead at this point, we just need to stuff the i915
> pointer into the cleanup_work.

Argh. vma is in ggtt, so vma->vm is alive and kicking -- not the vm we
just destroyed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23 10:14       ` Chris Wilson
@ 2019-05-23 10:19         ` Tvrtko Ursulin
  2019-05-23 10:20           ` Tvrtko Ursulin
  2019-05-23 10:21           ` Chris Wilson
  0 siblings, 2 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23 10:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 23/05/2019 11:14, Chris Wilson wrote:
> Quoting Chris Wilson (2019-05-23 11:07:50)
>> Quoting Tvrtko Ursulin (2019-05-23 11:01:09)
>>>
>>> On 23/05/2019 10:21, Tvrtko Ursulin wrote:
>>>>
>>>> On 23/05/2019 07:49, Chris Wilson wrote:
>>>>> 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>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
>>>>>    drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
>>>>>    2 files changed, 32 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> index 9ed41aefb456..5a350251766a 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> @@ -1828,11 +1828,33 @@ 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);
>>>>> +    struct drm_i915_private *i915 = work->vma->vm->i915;
>>>>
>>>> Does the sequence need an extra reference on ppgtt for dereferencing the
>>>> vm here? Alternatively storing i915 in the work struct could work around
>>>> that.
>>>
>>> Nope - vma has a pointer to vm as well which it will dereference. So if
>>> reference is needed it's needed - it looks like it is to me, since only
>>> contexts take them, which vma's still rely on, right?
>>
>> Nah, vm is decidedly dead at this point, we just need to stuff the i915
>> pointer into the cleanup_work.
> 
> Argh. vma is in ggtt, so vma->vm is alive and kicking -- not the vm we
> just destroyed.

Why is this an argh? :) Doesn't it mean you were right - just storing a 
pointer to i915 in work struct should work. I missed the fact they are 
two separate VMs.

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23 10:19         ` Tvrtko Ursulin
@ 2019-05-23 10:20           ` Tvrtko Ursulin
  2019-05-23 10:21           ` Chris Wilson
  1 sibling, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2019-05-23 10:20 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 23/05/2019 11:19, Tvrtko Ursulin wrote:
> 
> On 23/05/2019 11:14, Chris Wilson wrote:
>> Quoting Chris Wilson (2019-05-23 11:07:50)
>>> Quoting Tvrtko Ursulin (2019-05-23 11:01:09)
>>>>
>>>> On 23/05/2019 10:21, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 23/05/2019 07:49, Chris Wilson wrote:
>>>>>> 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>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/i915_gem_gtt.c | 32 
>>>>>> +++++++++++++++++++++++++++--
>>>>>>    drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
>>>>>>    2 files changed, 32 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> index 9ed41aefb456..5a350251766a 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>> @@ -1828,11 +1828,33 @@ 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);
>>>>>> +    struct drm_i915_private *i915 = work->vma->vm->i915;
>>>>>
>>>>> Does the sequence need an extra reference on ppgtt for 
>>>>> dereferencing the
>>>>> vm here? Alternatively storing i915 in the work struct could work 
>>>>> around
>>>>> that.
>>>>
>>>> Nope - vma has a pointer to vm as well which it will dereference. So if
>>>> reference is needed it's needed - it looks like it is to me, since only
>>>> contexts take them, which vma's still rely on, right?
>>>
>>> Nah, vm is decidedly dead at this point, we just need to stuff the i915
>>> pointer into the cleanup_work.
>>
>> Argh. vma is in ggtt, so vma->vm is alive and kicking -- not the vm we
>> just destroyed.
> 
> Why is this an argh? :) Doesn't it mean you were right - just storing a 
> pointer to i915 in work struct should work. I missed the fact they are 
> two separate VMs.

Apart from the locally dereferenced vma being the same ggtt one... :)) 
Okay.. Is it worth a comment?

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

Regards,

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23 10:19         ` Tvrtko Ursulin
  2019-05-23 10:20           ` Tvrtko Ursulin
@ 2019-05-23 10:21           ` Chris Wilson
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2019-05-23 10:21 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-05-23 11:19:30)
> 
> On 23/05/2019 11:14, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-05-23 11:07:50)
> >> Quoting Tvrtko Ursulin (2019-05-23 11:01:09)
> >>>
> >>> On 23/05/2019 10:21, Tvrtko Ursulin wrote:
> >>>>
> >>>> On 23/05/2019 07:49, Chris Wilson wrote:
> >>>>> 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>
> >>>>> ---
> >>>>>    drivers/gpu/drm/i915/i915_gem_gtt.c | 32 +++++++++++++++++++++++++++--
> >>>>>    drivers/gpu/drm/i915/i915_gem_gtt.h |  2 ++
> >>>>>    2 files changed, 32 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> index 9ed41aefb456..5a350251766a 100644
> >>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> @@ -1828,11 +1828,33 @@ 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);
> >>>>> +    struct drm_i915_private *i915 = work->vma->vm->i915;
> >>>>
> >>>> Does the sequence need an extra reference on ppgtt for dereferencing the
> >>>> vm here? Alternatively storing i915 in the work struct could work around
> >>>> that.
> >>>
> >>> Nope - vma has a pointer to vm as well which it will dereference. So if
> >>> reference is needed it's needed - it looks like it is to me, since only
> >>> contexts take them, which vma's still rely on, right?
> >>
> >> Nah, vm is decidedly dead at this point, we just need to stuff the i915
> >> pointer into the cleanup_work.
> > 
> > Argh. vma is in ggtt, so vma->vm is alive and kicking -- not the vm we
> > just destroyed.
> 
> Why is this an argh? :) Doesn't it mean you were right - just storing a 
> pointer to i915 in work struct should work. I missed the fact they are 
> two separate VMs.

We don't need to store an extra pointer as vma->vm is valid as
i915->ggtt isn't destroyed until after the worker is flushed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23  6:49 [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup Chris Wilson
  2019-05-23  9:21 ` Tvrtko Ursulin
@ 2019-05-23 14:28 ` Patchwork
  2019-05-23 14:58 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-05-24 21:48 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-23 14:28 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
URL   : https://patchwork.freedesktop.org/series/61012/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6c62c85d4558 drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
-:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#13: 
<4> [257.740160] WARN_ON(debug_locks && !lock_is_held(&(&vma->vm->i915->drm.struct_mutex)->dep_map))

-:46: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e0695db7298e ("drm/i915: Create/destroy VM (ppGTT) for use with contexts")'
#46: 
References: e0695db7298e ("drm/i915: Create/destroy VM (ppGTT) for use with contexts")

total: 1 errors, 1 warnings, 0 checks, 64 lines checked

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23  6:49 [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup Chris Wilson
  2019-05-23  9:21 ` Tvrtko Ursulin
  2019-05-23 14:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2019-05-23 14:58 ` Patchwork
  2019-05-24 21:48 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-23 14:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
URL   : https://patchwork.freedesktop.org/series/61012/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6132 -> Patchwork_13076
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/

Known issues
------------

  Here are the changes found in Patchwork_13076 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [PASS][1] -> [DMESG-FAIL][2] ([fdo#110235])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-bxt-dsi:         [PASS][3] -> [INCOMPLETE][4] ([fdo#103927])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-dsi}:       [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_reloc@basic-write-gtt-noreloc:
    - {fi-icl-u3}:        [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html

  
#### Warnings ####

  * igt@i915_selftest@live_hangcheck:
    - fi-apl-guc:         [INCOMPLETE][9] ([fdo#103927] / [fdo#110624]) -> [FAIL][10] ([fdo#110623])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-apl-guc/igt@i915_selftest@live_hangcheck.html

  * igt@runner@aborted:
    - fi-apl-guc:         [FAIL][11] ([fdo#110624]) -> [FAIL][12] ([fdo#110622])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/fi-apl-guc/igt@runner@aborted.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/fi-apl-guc/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622
  [fdo#110623]: https://bugs.freedesktop.org/show_bug.cgi?id=110623
  [fdo#110624]: https://bugs.freedesktop.org/show_bug.cgi?id=110624
  [fdo#110718]: https://bugs.freedesktop.org/show_bug.cgi?id=110718


Participating hosts (54 -> 41)
------------------------------

  Missing    (13): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-hsw-4770 fi-ivb-3770 fi-byt-n2820 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6132 -> Patchwork_13076

  CI_DRM_6132: 78850b480c542b2e10da5a93afac2e13307909cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5010: 631f3ac2e78c8d6332afc693bf290ae23d8d5685 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13076: 6c62c85d45586f011c27745dc2df80dcd6a28b58 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6c62c85d4558 drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup

== Logs ==

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
  2019-05-23  6:49 [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup Chris Wilson
                   ` (2 preceding siblings ...)
  2019-05-23 14:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-05-24 21:48 ` Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2019-05-24 21:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup
URL   : https://patchwork.freedesktop.org/series/61012/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6132_full -> Patchwork_13076_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_13076_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@forked-medium-copy-xy:
    - shard-iclb:         [PASS][1] -> [TIMEOUT][2] ([fdo#109673])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb8/igt@gem_mmap_gtt@forked-medium-copy-xy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb6/igt@gem_mmap_gtt@forked-medium-copy-xy.html

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#104108] / [fdo#107807])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl10/igt@i915_pm_rpm@system-suspend-modeset.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl1/igt@i915_pm_rpm@system-suspend-modeset.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl7/igt@i915_suspend@forcewake.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl5/igt@i915_suspend@forcewake.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([fdo#104873])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl5/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([fdo#100368])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl2/igt@kms_flip@plain-flip-ts-check-interruptible.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl5/igt@kms_flip@plain-flip-ts-check-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-glk:          [PASS][13] -> [INCOMPLETE][14] ([fdo#103359] / [k.org#198133])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-glk8/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-wc.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#108566]) +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145] / [fdo#110403])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109642])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb5/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#108341])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb6/igt@kms_psr@no_drrs.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb1/igt@kms_psr@no_drrs.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [PASS][23] -> [FAIL][24] ([fdo#99912])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-kbl2/igt@kms_setmode@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-kbl7/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][25] ([fdo#104108]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl5/igt@gem_softpin@noreloc-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl10/igt@gem_softpin@noreloc-s3.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +6 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-apl3/igt@gem_workarounds@suspend-resume.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-apl7/igt@gem_workarounds@suspend-resume.html

  * igt@kms_flip_tiling@flip-to-y-tiled:
    - shard-iclb:         [FAIL][29] ([fdo#107931] / [fdo#108134]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb3/igt@kms_flip_tiling@flip-to-y-tiled.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb4/igt@kms_flip_tiling@flip-to-y-tiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-iclb:         [FAIL][31] ([fdo#103167]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move:
    - shard-skl:          [FAIL][33] ([fdo#108040]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl10/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-move.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-skl:          [FAIL][35] ([fdo#103166]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][37] ([fdo#108145]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][39] ([fdo#103166]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][43] ([fdo#99912]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6132/shard-apl3/igt@kms_setmode@basic.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13076/shard-apl7/igt@kms_setmode@basic.html

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107931]: https://bugs.freedesktop.org/show_bug.cgi?id=107931
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  Additional (1): pig-snb-2600 
  Missing    (1): pig-hsw-4770r 


Build changes
-------------

  * Linux: CI_DRM_6132 -> Patchwork_13076

  CI_DRM_6132: 78850b480c542b2e10da5a93afac2e13307909cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5010: 631f3ac2e78c8d6332afc693bf290ae23d8d5685 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13076: 6c62c85d45586f011c27745dc2df80dcd6a28b58 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-24 21:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23  6:49 [PATCH] drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup Chris Wilson
2019-05-23  9:21 ` Tvrtko Ursulin
2019-05-23 10:01   ` Tvrtko Ursulin
2019-05-23 10:07     ` Chris Wilson
2019-05-23 10:14       ` Chris Wilson
2019-05-23 10:19         ` Tvrtko Ursulin
2019-05-23 10:20           ` Tvrtko Ursulin
2019-05-23 10:21           ` Chris Wilson
2019-05-23 14:28 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-05-23 14:58 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-24 21:48 ` ✓ Fi.CI.IGT: " Patchwork

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.