From patchwork Mon Jul 1 10:00:09 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 2806841 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 74CA3BF4A1 for ; Mon, 1 Jul 2013 10:02:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0B0632017C for ; Mon, 1 Jul 2013 10:02:22 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A486D2017A for ; Mon, 1 Jul 2013 10:02:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B9C96E5F66 for ; Mon, 1 Jul 2013 03:02:20 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D4F7E5CD3 for ; Mon, 1 Jul 2013 03:02:05 -0700 (PDT) Received: from lillypilly.canonical.com ([91.189.89.62]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1Utavs-0007jn-Fv; Mon, 01 Jul 2013 10:02:04 +0000 Received: by lillypilly.canonical.com (Postfix, from userid 3489) id C840C26C2A5D; Mon, 1 Jul 2013 10:00:17 +0000 (UTC) Subject: [PATCH] drm/nouveau: fix locking in nouveau_crtc_page_flip To: dri-devel@lists.freedesktop.org From: Maarten Lankhorst Date: Mon, 01 Jul 2013 12:00:09 +0200 Message-ID: <20130701100009.6275.17224.stgit@patser> User-Agent: StGit/0.15 MIME-Version: 1.0 Cc: nouveau@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This is a bit messed up because chan->cli->mutex is a different class, depending on whether it is the global drm client or not. This is because the global cli->mutex lock can be taken for eviction, so locking it before pinning the buffer objects may result in a deadlock. The locking order from outer to inner is: - &cli->mutex - ttm_bo - &drm_client_lock (global &cli->mutex) Fixes the following lockdep splat: ====================================================== [ INFO: possible circular locking dependency detected ] 3.10.0-rc7+ #101 Not tainted ------------------------------------------------------- mplayer/4979 is trying to acquire lock: (&drm_client_lock_class_key){+.+.+.}, at: [] nouveau_bo_move_m2mf.isra.13+0x4d/0x130 [nouveau] but task is already holding lock: (reservation_ww_class_mutex){+.+.+.}, at: [] ttm_bo_vm_fault+0x47/0x394 [ttm] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (reservation_ww_class_mutex){+.+.+.}: [] lock_acquire+0x90/0x1f9 [] mutex_lock_nested+0x56/0x3bb [] nouveau_bo_pin+0x3c/0x15b [nouveau] [] nouveau_crtc_page_flip+0xef/0x68b [nouveau] [] drm_mode_page_flip_ioctl+0x289/0x336 [drm] [] drm_ioctl+0x4d3/0x619 [drm] [] do_vfs_ioctl+0x90/0x50a [] SyS_ioctl+0x8f/0x9e [] tracesys+0xdd/0xe2 -> #0 (&drm_client_lock_class_key){+.+.+.}: [] __lock_acquire+0x1c29/0x1c2b [] lock_acquire+0x90/0x1f9 [] mutex_lock_nested+0x56/0x3bb [] nouveau_bo_move_m2mf.isra.13+0x4d/0x130 [nouveau] [] nouveau_bo_move+0xb9/0x3cb [nouveau] [] ttm_bo_handle_move_mem+0x24e/0x6b0 [ttm] [] ttm_bo_move_buffer+0x157/0x164 [ttm] [] ttm_bo_validate+0xa0/0x129 [ttm] [] nouveau_bo_validate+0x1c/0x1e [nouveau] [] nouveau_ttm_fault_reserve_notify+0xd0/0xd7 [nouveau] [] ttm_bo_vm_fault+0x69/0x394 [ttm] [] __do_fault+0x6e/0x496 [] handle_pte_fault+0x84/0x861 [] handle_mm_fault+0x1e2/0x2b1 [] __do_page_fault+0x15e/0x517 [] do_page_fault+0x37/0x6b [] page_fault+0x22/0x30 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(reservation_ww_class_mutex); lock(&drm_client_lock_class_key); lock(reservation_ww_class_mutex); lock(&drm_client_lock_class_key); *** DEADLOCK *** 2 locks held by mplayer/4979: #0: (&mm->mmap_sem){++++++}, at: [] __do_page_fault+0xff/0x517 #1: (reservation_ww_class_mutex){+.+.+.}, at: [] ttm_bo_vm_fault+0x47/0x394 [ttm] stack backtrace: CPU: 3 PID: 4979 Comm: mplayer Not tainted 3.10.0-rc7+ #101 Hardware name: Acer Aspire M3985/Aspire M3985, BIOS P01-A1 03/12/2012 ffffffff8214cdf0 ffff8803c8549678 ffffffff816eb3d8 ffff8803c85496c8 ffffffff816e5f4f ffff8803c85f86e0 ffff8803c8549750 ffff8803c85f8708 ffff8803c85f8000 ffff8803c85f8708 ffff8803c85f8730 ffffffff8214cdf0 Call Trace: [] dump_stack+0x19/0x1b [] print_circular_bug+0x1fb/0x20c [] __lock_acquire+0x1c29/0x1c2b [] lock_acquire+0x90/0x1f9 [] ? nouveau_bo_move_m2mf.isra.13+0x4d/0x130 [nouveau] [] ? mark_held_locks+0x6d/0x117 [] mutex_lock_nested+0x56/0x3bb [] ? nouveau_bo_move_m2mf.isra.13+0x4d/0x130 [nouveau] [] ? trace_hardirqs_on+0xd/0xf [] nouveau_bo_move_m2mf.isra.13+0x4d/0x130 [nouveau] [] nouveau_bo_move+0xb9/0x3cb [nouveau] [] ttm_bo_handle_move_mem+0x24e/0x6b0 [ttm] [] ? ttm_bo_mem_space+0x18e/0x3e1 [ttm] [] ttm_bo_move_buffer+0x157/0x164 [ttm] [] ? __lock_is_held+0x4f/0x63 [] ttm_bo_validate+0xa0/0x129 [ttm] [] nouveau_bo_validate+0x1c/0x1e [nouveau] [] nouveau_ttm_fault_reserve_notify+0xd0/0xd7 [nouveau] [] ttm_bo_vm_fault+0x69/0x394 [ttm] [] __do_fault+0x6e/0x496 [] ? __mutex_unlock_slowpath+0xef/0x189 [] handle_pte_fault+0x84/0x861 [] ? __do_page_fault+0xff/0x517 [] ? drm_ioctl+0x12f/0x619 [drm] [] handle_mm_fault+0x1e2/0x2b1 [] __do_page_fault+0x15e/0x517 [] ? avc_has_perm_flags+0x3c/0x204 [] ? rcu_eqs_enter_common.isra.61+0x9c/0x1c6 [] ? user_enter+0x75/0xaf [] ? rcu_eqs_exit+0x5c/0x9c [] do_page_fault+0x37/0x6b [] page_fault+0x22/0x30 Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/nouveau/nouveau_display.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 5ee23a4..d89d3ef 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -595,7 +595,8 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, chan = drm->channel; spin_unlock(&old_bo->bo.bdev->fence_lock); - mutex_lock(&chan->cli->mutex); + if (chan->cli != &drm->client) + mutex_lock(&chan->cli->mutex); if (new_bo != old_bo) { ret = nouveau_bo_pin(new_bo, TTM_PL_FLAG_VRAM); @@ -613,7 +614,8 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, ret = ttm_bo_reserve(&new_bo->bo, false, false, false, 0); if (ret) { - mutex_unlock(&chan->cli->mutex); + if (chan->cli != &drm->client) + mutex_unlock(&chan->cli->mutex); goto fail_free; } @@ -623,6 +625,9 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, fb->bits_per_pixel, fb->pitches[0], crtc->x, crtc->y, new_bo->bo.offset }; + if (chan->cli == &drm->client) + mutex_lock(&chan->cli->mutex); + /* Emit a page flip */ if (nv_device(drm->device)->card_type >= NV_50) { ret = nv50_display_flip_next(crtc, fb, chan, 0);