From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Do not store the error pointer for a failed userptr registration Date: Mon, 29 Sep 2014 16:19:16 +0300 Message-ID: <87d2aelg2z.fsf@intel.com> References: <1411723862-7782-1-git-send-email-chris@chris-wilson.co.uk> <5425373A.809@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id F00A46E25D for ; Mon, 29 Sep 2014 06:19:19 -0700 (PDT) In-Reply-To: <5425373A.809@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Tvrtko Ursulin , Chris Wilson , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Fri, 26 Sep 2014, 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: [] 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:[] [] 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] [] __i915_mm_struct_free__worker+0x42/0x80 >> [ 73.429116] [] process_one_work+0x1ba/0x610 >> [ 73.429632] [] ? process_one_work+0x14f/0x610 >> [ 73.430153] [] worker_thread+0x6b/0x4a0 >> [ 73.430671] [] ? trace_hardirqs_on+0xd/0x10 >> [ 73.431501] [] ? process_one_work+0x610/0x610 >> [ 73.432030] [] kthread+0xf6/0x110 >> [ 73.432561] [] ? __kthread_parkme+0x80/0x80 >> [ 73.433100] [] ret_from_fork+0x7c/0xb0 >> [ 73.433644] [] ? __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 [] mmu_notifier_unregister+0x23/0x130 >> [ 73.437017] RSP >> [ 73.437704] CR2: 000000000000004c >> >> Fixes regression from commit ad46cb533d586fdb256855437af876617c6cf609 >> Author: Chris Wilson >> 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 >> Cc: Jacek Danecki >> Cc: "Gong, Zhipeng" >> Cc: Jacek Danecki >> Cc: "Ursulin, Tvrtko" >> Cc: stable@vger.kernel.org >> --- >> 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 Pushed to drm-intel-fixes, thanks for the patch and review. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center