Re: [PATCH] drm/i915: Do not store the error pointer for a failed userptr registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 26, 2014 at 10:51:54AM +0100, Tvrtko Ursulin wrote:
> 
> On 09/26/2014 10:31 AM, Chris Wilson wrote:
> >If we fail to create our mmu notification, we report the error back and
> >currently store the error inside the i915_mm_struct. This not only causes
> >subsequent registerations of the same mm to fail (an issue if the first
> >was interrupted by a signal and needed to be restarted) but also causes
> >us to eventually try and free the error pointer.
> >
> >[   73.419599] BUG: unable to handle kernel NULL pointer dereference at 000000000000004c
> >[   73.419831] IP: [<ffffffff8114af33>] mmu_notifier_unregister+0x23/0x130
> >[   73.420065] PGD 8650c067 PUD 870bb067 PMD 0
> >[   73.420319] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
> >[   73.420580] CPU: 0 PID: 42 Comm: kworker/0:1 Tainted: G        W      3.17.0-rc6+ #1561
> >[   73.420837] Hardware name: Intel Corporation SandyBridge Platform/LosLunas CRB, BIOS ASNBCPT1.86C.0075.P00.1106281639 06/28/2011
> >[   73.421405] Workqueue: events __i915_mm_struct_free__worker
> >[   73.421724] task: ffff880088a81220 ti: ffff880088168000 task.ti: ffff880088168000
> >[   73.422051] RIP: 0010:[<ffffffff8114af33>]  [<ffffffff8114af33>] mmu_notifier_unregister+0x23/0x130
> >[   73.422410] RSP: 0018:ffff88008816bd50  EFLAGS: 00010286
> >[   73.422765] RAX: 0000000000000003 RBX: ffff880086485400 RCX: 0000000000000000
> >[   73.423137] RDX: ffff88016d80ee90 RSI: ffff880086485400 RDI: 0000000000000044
> >[   73.423513] RBP: ffff88008816bd70 R08: 0000000000000001 R09: 0000000000000000
> >[   73.423895] R10: 0000000000000320 R11: 0000000000000001 R12: 0000000000000044
> >[   73.424282] R13: ffff880166e5f008 R14: ffff88016d815200 R15: ffff880166e5f040
> >[   73.424682] FS:  0000000000000000(0000) GS:ffff88016d800000(0000) knlGS:0000000000000000
> >[   73.425099] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >[   73.425537] CR2: 000000000000004c CR3: 0000000087f5f000 CR4: 00000000000407f0
> >[   73.426157] Stack:
> >[   73.426597]  ffff880088a81248 ffff880166e5f038 fffffffffffffffc ffff880166e5f008
> >[   73.427096]  ffff88008816bd98 ffffffff814a75f2 ffff880166e5f038 ffff8800880f8a28
> >[   73.427603]  ffff88016d812ac0 ffff88008816be00 ffffffff8106321a ffffffff810631af
> >[   73.428119] Call Trace:
> >[   73.428606]  [<ffffffff814a75f2>] __i915_mm_struct_free__worker+0x42/0x80
> >[   73.429116]  [<ffffffff8106321a>] process_one_work+0x1ba/0x610
> >[   73.429632]  [<ffffffff810631af>] ? process_one_work+0x14f/0x610
> >[   73.430153]  [<ffffffff810636db>] worker_thread+0x6b/0x4a0
> >[   73.430671]  [<ffffffff8108d67d>] ? trace_hardirqs_on+0xd/0x10
> >[   73.431501]  [<ffffffff81063670>] ? process_one_work+0x610/0x610
> >[   73.432030]  [<ffffffff8106a206>] kthread+0xf6/0x110
> >[   73.432561]  [<ffffffff8106a110>] ? __kthread_parkme+0x80/0x80
> >[   73.433100]  [<ffffffff8169c22c>] ret_from_fork+0x7c/0xb0
> >[   73.433644]  [<ffffffff8106a110>] ? __kthread_parkme+0x80/0x80
> >[   73.434194] Code: 0f 1f 84 00 00 00 00 00 66 66 66 66 90 8b 46 4c 85 c0 0f 8e 10 01 00 00 55 48 89 e5 41 55 41 54 53 48 89 f3 49 89 fc 48 83 ec 08 <48> 83 7f 08 00 0f 84 b1 00 00 00 48 c7 c7 40 e6 ac 82 e8 26 65
> >[   73.435942] RIP  [<ffffffff8114af33>] mmu_notifier_unregister+0x23/0x130
> >[   73.437017]  RSP <ffff88008816bd50>
> >[   73.437704] CR2: 000000000000004c
> >
> >Fixes regression from commit ad46cb533d586fdb256855437af876617c6cf609
> >Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Date:   Thu Aug 7 14:20:40 2014 +0100
> >
> >     drm/i915: Prevent recursive deadlock on releasing a busy userptr
> >
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84207
> >Testcase: igt/gem_render_copy_redux
> >Testcase: igt/gem_userptr_blits/create-destroy-sync
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Jacek Danecki <jacek.danecki@xxxxxxxxx>
> >Cc: "Gong, Zhipeng" <zhipeng.gong@xxxxxxxxx>
> >Cc: Jacek Danecki <jacek.danecki@xxxxxxxxx>
> >Cc: "Ursulin, Tvrtko" <tvrtko.ursulin@xxxxxxxxx>
> >Cc: stable@xxxxxxxxxxxxxxx
> >---
> >  drivers/gpu/drm/i915/i915_gem_userptr.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >index 903a875..ecd5c84 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >@@ -293,15 +293,23 @@ i915_gem_userptr_release__mmu_notifier(struct drm_i915_gem_object *obj)
> >  static struct i915_mmu_notifier *
> >  i915_mmu_notifier_find(struct i915_mm_struct *mm)
> >  {
> >-	if (mm->mn == NULL) {
> >-		down_write(&mm->mm->mmap_sem);
> >-		mutex_lock(&to_i915(mm->dev)->mm_lock);
> >-		if (mm->mn == NULL)
> >-			mm->mn = i915_mmu_notifier_create(mm->mm);
> >-		mutex_unlock(&to_i915(mm->dev)->mm_lock);
> >-		up_write(&mm->mm->mmap_sem);
> >+	struct i915_mmu_notifier *mn = mm->mn;
> >+
> >+	mn = mm->mn;
> >+	if (mn)
> >+		return mn;
> >+
> >+	down_write(&mm->mm->mmap_sem);
> >+	mutex_lock(&to_i915(mm->dev)->mm_lock);
> >+	if ((mn = mm->mn) == NULL) {
> >+		mn = i915_mmu_notifier_create(mm->mm);
> >+		if (!IS_ERR(mn))
> >+			mm->mn = mn;
> >  	}
> >-	return mm->mn;
> >+	mutex_unlock(&to_i915(mm->dev)->mm_lock);
> >+	up_write(&mm->mm->mmap_sem);
> >+
> >+	return mn;
> >  }
> >
> >  static int
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Queued for dinf (to give it a bit more vetting time), thanks for the patch
and review.
-Daniel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux