From patchwork Thu Aug 8 07:10:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2840804 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 606769F479 for ; Thu, 8 Aug 2013 07:25:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5D7E1202E5 for ; Thu, 8 Aug 2013 07:25:24 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 384EF202E9 for ; Thu, 8 Aug 2013 07:25:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2DEA8E6463 for ; Thu, 8 Aug 2013 00:25:23 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ee0-f51.google.com (mail-ee0-f51.google.com [74.125.83.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 111D4E754F for ; Thu, 8 Aug 2013 00:11:07 -0700 (PDT) Received: by mail-ee0-f51.google.com with SMTP id c1so1300131eek.38 for ; Thu, 08 Aug 2013 00:11:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=JFnF1C88YeTMt7Wgb5TRzP3KfNdI6SOvDIdprkqwh2g=; b=GP5tWuXVtfjT2iYVBTEyAA8HvnPMyVLyAwGaz83nnBz9fXLeA17CPmaZpKdaX/ouFX GATpkOLXse05DCpB9+cp8Te6jyVtWuH12t+nDpxyb3v6xBvt0qcXVlSpE8hiwHLI63DM W4g77mt2ioHbFOFDRT8fi/zCiPKt1z0s2WNy4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=JFnF1C88YeTMt7Wgb5TRzP3KfNdI6SOvDIdprkqwh2g=; b=lnmc5OIBUKdpE6QH/DFoJfG3wWZf0nuTHp/fERx5lzw2H4Aq4y0cWMdIxsT8z9vCwm ZLofH3v0lkv7+DIXtxUm237khJyz1ZyHB5oORF7NekVn0X1QDcqWHQanKU2XsgBvFu9f nJOZwK1xhE35zJ5i9QgGKBb2X53bYA42eQKtGNNf/7A5ZJU2vIlvvHWeVMR8A8mIy/RK QDDmmNtd/nKs1GEttMabaeMfJ/yftbke7A1aKYSGfRvIaYoq8bBmwhxu73thcSRn4ppC Wq00YCG1ypSF6AuCBilDNZsa8k0RP3TX1xYVFINDrMdRymedSNGC+Ca7NyhBiskzt0qm 01Dw== X-Gm-Message-State: ALoCoQlNE6p48sEk4SRiYw3saf8DZEv+CtEGJGA7N45q1n96xsKn5lj8SKELb7W+D2j2EVc7QkfV X-Received: by 10.14.102.72 with SMTP id c48mr6831136eeg.52.1375945867008; Thu, 08 Aug 2013 00:11:07 -0700 (PDT) Received: from aaron.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPSA id m54sm16647369eex.2.2013.08.08.00.11.05 for (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 08 Aug 2013 00:11:06 -0700 (PDT) From: Daniel Vetter To: Dave Airlie Subject: [PATCH 5/5] drm/prime: remove cargo-cult locking from map_sg helper Date: Thu, 8 Aug 2013 09:10:40 +0200 Message-Id: <1375945840-9784-6-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1375945840-9784-1-git-send-email-daniel.vetter@ffwll.ch> References: <1375945840-9784-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Laurent Pinchart , DRI Development 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 X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing of the underlying page array is anything we need to be concerned about, and either those pages are pinned independently, or we're screwed no matter what. And indeed, nouveau/radeon pin the backing storage in their attach/detach functions. Since I've created this patch cma prime support for dma_buf was added. drm_gem_cma_prime_get_sg_table only calls kzalloc and the creates&maps the sg table with dma_get_sgtable. It doesn't touch any gem object state otherwise. So the cma helpers also look safe. The only thing we might claim it does is prevent concurrent mapping of dma_buf attachments. But a) that's not allowed and b) the current code is racy already since it checks whether the sg mapping exists _before_ grabbing the lock. So the dev->struct_mutex locking here does absolutely nothing useful, but only distracts. Remove it. This should also help Maarten's work to eventually pin the backing storage more dynamically by preventing locking inversions around dev->struct_mutex. v2: Add analysis for recently added cma helper prime code. Cc: Laurent Pinchart Cc: Maarten Lankhorst Acked-by: Laurent Pinchart Acked-by: Maarten Lankhorst Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_prime.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index a35f206..f115962 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -167,8 +167,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, if (WARN_ON(prime_attach->dir != DMA_NONE)) return ERR_PTR(-EBUSY); - mutex_lock(&obj->dev->struct_mutex); - sgt = obj->dev->driver->gem_prime_get_sg_table(obj); if (!IS_ERR(sgt)) { @@ -182,7 +180,6 @@ static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, } } - mutex_unlock(&obj->dev->struct_mutex); return sgt; }