From patchwork Wed Dec 19 17:21:10 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 1896251 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 4A3ED3FC64 for ; Wed, 19 Dec 2012 17:22:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 33733E6550 for ; Wed, 19 Dec 2012 09:22:35 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 6C55AE6515 for ; Wed, 19 Dec 2012 09:21:11 -0800 (PST) Received: from 5ed48cef.cm-7-5c.dynamic.ziggo.nl ([94.212.140.239] helo=[192.168.1.128]) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1TlNKQ-0004Yw-CH; Wed, 19 Dec 2012 17:21:10 +0000 Message-ID: <50D1F786.3020300@canonical.com> Date: Wed, 19 Dec 2012 18:21:10 +0100 From: Maarten Lankhorst User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Paul Menzel Subject: Re: [PATCH] drm/ttm: fix delayed ttm_bo_cleanup_refs_and_unlock delayed handling References: <50D1CCCB.1070205@canonical.com> In-Reply-To: <50D1CCCB.1070205@canonical.com> Cc: "dri-devel@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 Op 19-12-12 15:18, Maarten Lankhorst schreef: > Fix regression introduced by 85b144f860176 > > Signed-off-by: Maarten Lankhorst > Reported-by: Markus Trippelsdorf > Hey Paul Menzel, I just wanted to have the fix out asap, and had to leave right after. Updated commit message below: Fix regression introduced by 85b144f860176 "drm/ttm: call ttm_bo_cleanup_refs with reservation and lru lock held, v3" Slowpath ttm_bo_cleanup_refs_and_unlock accidentally tried to increase refcount on &bo->sync_obj instead of bo->sync_obj. The compiler didn't complain since sync_obj_ref takes a void pointer, so it was still valid c. This could result in lockups, memory corruptions, and warnings like these when graphics card VRAM usage is high: ------------[ cut here ]------------ WARNING: at include/linux/kref.h:42 radeon_fence_ref+0x2c/0x40() Hardware name: System Product Name Pid: 157, comm: X Not tainted 3.7.0-rc7-00520-g85b144f-dirty #174 Call Trace: [] ? warn_slowpath_common+0x74/0xb0 [] ? radeon_fence_ref+0x2c/0x40 [] ? ttm_bo_cleanup_refs_and_unlock+0x18c/0x2d0 [] ? ttm_mem_evict_first+0x1dc/0x2a0 [] ? ttm_bo_man_get_node+0x62/0xb0 [] ? ttm_bo_mem_space+0x28e/0x340 [] ? ttm_bo_move_buffer+0xfc/0x170 [] ? kmem_cache_alloc+0xb2/0xc0 [] ? ttm_bo_validate+0x95/0x110 [] ? ttm_bo_init+0x2ec/0x3b0 [] ? radeon_bo_create+0x18a/0x200 [] ? radeon_bo_clear_va+0x40/0x40 [] ? radeon_gem_object_create+0x92/0x160 [] ? radeon_gem_create_ioctl+0x6c/0x150 [] ? radeon_gem_object_free+0x2f/0x40 [] ? drm_ioctl+0x420/0x4f0 [] ? radeon_gem_pwrite_ioctl+0x20/0x20 [] ? do_vfs_ioctl+0x2e4/0x4e0 [] ? vfs_read+0x118/0x160 [] ? sys_ioctl+0x4c/0xa0 [] ? sys_read+0x51/0xa0 [] ? system_call_fastpath+0x16/0x1b Signed-off-by: Maarten Lankhorst Reported-by: Markus Trippelsdorf Message-ID: <20121217182752.GA351@x4> Acked-by: Paul Menzel diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 0bf66f9..9f85418 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -579,7 +579,7 @@ static int ttm_bo_cleanup_refs_and_unlock(struct ttm_buffer_object *bo, * at this point the buffer should be dead, so * no new sync objects can be attached. */ - sync_obj = driver->sync_obj_ref(&bo->sync_obj); + sync_obj = driver->sync_obj_ref(bo->sync_obj); spin_unlock(&bdev->fence_lock); atomic_set(&bo->reserved, 0);