From patchwork Tue Nov 27 18:07:06 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jerome Glisse X-Patchwork-Id: 1812991 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 4CE8CDF254 for ; Tue, 27 Nov 2012 23:10:42 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 324BAE632A for ; Tue, 27 Nov 2012 15:10:42 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pa0-f49.google.com (mail-pa0-f49.google.com [209.85.220.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 899EFE62DF for ; Tue, 27 Nov 2012 15:08:57 -0800 (PST) Received: by mail-pa0-f49.google.com with SMTP id bi1so5681441pad.36 for ; Tue, 27 Nov 2012 15:08:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=uua5MmKME/eA7+F8oy3lclIINd91jt/SyjdWy40AlpE=; b=qI1Cu/2dQJBgia3wtZE5jb2oJQa47HOjZdKCR9oapZ2x/BcChkJGwisDA5jMGOU84e E640A0iKkF38m+VfDgPqiXJUcOs4Z14z+UWKDRFaQGShicF1wXWj2M3wxfPPuMr1bLDc a0HlvlpPGzQpV+i0X4uVRqjU9SIC7vA03zS61chTbJOcO2NNBW7Z1b2GLOeGd1wu4wXD 0Zmw3REudslwGgl33bm6hT+DvT/I/iatm3gR/u5j/F0x1f3OFFR0o0uFEyczRVAum+4a dB9LTkegBtc2IczVZqS30RCYMdrQ2+UoIFiM+50jC0ThYgmrGlFqlpFbQ7eZrLM08m07 Y6fw== Received: by 10.66.80.65 with SMTP id p1mr46780387pax.20.1354057737410; Tue, 27 Nov 2012 15:08:57 -0800 (PST) Received: from homer.localdomain.com ([66.187.233.206]) by mx.google.com with ESMTPS id zv10sm11304326pbc.76.2012.11.27.15.08.55 (version=SSLv3 cipher=OTHER); Tue, 27 Nov 2012 15:08:56 -0800 (PST) From: j.glisse@gmail.com To: dri-devel@lists.freedesktop.org Subject: [PATCH 2/2] drm/radeon: fix deadlock when bo is associated to different handle Date: Tue, 27 Nov 2012 13:07:06 -0500 Message-Id: <1354039626-19920-2-git-send-email-j.glisse@gmail.com> X-Mailer: git-send-email 1.7.11.7 In-Reply-To: <1354039626-19920-1-git-send-email-j.glisse@gmail.com> References: <1354039626-19920-1-git-send-email-j.glisse@gmail.com> Cc: Jerome Glisse 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: , MIME-Version: 1.0 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 From: Jerome Glisse There is a rare case, that seems to only happen accross suspend/resume cycle, where a bo is associated with several different handle. This lead to a deadlock in ttm buffer reservation path. This could only happen with flinked(globaly exported) object. Userspace should not reopen multiple time a globaly exported object. However the kernel should handle gracefully this corner case and not keep rejecting the userspace command stream. This is the object of this patch. Fix suspend/resume issue where user see following message : [drm:radeon_cs_ioctl] *ERROR* Failed to parse relocation -35! Signed-off-by: Jerome Glisse --- drivers/gpu/drm/radeon/radeon_cs.c | 53 ++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index 41672cc..064e64d 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -54,39 +54,48 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p) return -ENOMEM; } for (i = 0; i < p->nrelocs; i++) { - struct drm_radeon_cs_reloc *r; - + struct drm_radeon_cs_reloc *reloc; + + /* One bo could be associated with several different handle. + * Only happen for flinked bo that are open several time. + * + * FIXME: + * Maybe we should consider an alternative to idr for gem + * object to insure a 1:1 uniq mapping btw handle and gem + * object. + */ duplicate = false; - r = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4]; + reloc = (struct drm_radeon_cs_reloc *)&chunk->kdata[i*4]; + p->relocs[i].handle = 0; + p->relocs[i].flags = reloc->flags; + p->relocs[i].gobj = drm_gem_object_lookup(ddev, + p->filp, + reloc->handle); + if (p->relocs[i].gobj == NULL) { + DRM_ERROR("gem object lookup failed 0x%x\n", + reloc->handle); + return -ENOENT; + } + p->relocs[i].robj = gem_to_radeon_bo(p->relocs[i].gobj); + p->relocs[i].lobj.bo = p->relocs[i].robj; + p->relocs[i].lobj.wdomain = reloc->write_domain; + p->relocs[i].lobj.rdomain = reloc->read_domains; + p->relocs[i].lobj.tv.bo = &p->relocs[i].robj->tbo; + for (j = 0; j < i; j++) { - if (r->handle == p->relocs[j].handle) { + if (p->relocs[i].lobj.bo == p->relocs[j].lobj.bo) { p->relocs_ptr[i] = &p->relocs[j]; duplicate = true; break; } } + if (!duplicate) { - p->relocs[i].gobj = drm_gem_object_lookup(ddev, - p->filp, - r->handle); - if (p->relocs[i].gobj == NULL) { - DRM_ERROR("gem object lookup failed 0x%x\n", - r->handle); - return -ENOENT; - } p->relocs_ptr[i] = &p->relocs[i]; - p->relocs[i].robj = gem_to_radeon_bo(p->relocs[i].gobj); - p->relocs[i].lobj.bo = p->relocs[i].robj; - p->relocs[i].lobj.wdomain = r->write_domain; - p->relocs[i].lobj.rdomain = r->read_domains; - p->relocs[i].lobj.tv.bo = &p->relocs[i].robj->tbo; - p->relocs[i].handle = r->handle; - p->relocs[i].flags = r->flags; + p->relocs[i].handle = reloc->handle; radeon_bo_list_add_object(&p->relocs[i].lobj, &p->validated); - - } else - p->relocs[i].handle = 0; + } } return radeon_bo_list_validate(&p->validated); }