From patchwork Wed Jul 10 11:54:33 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2825631 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 D26C79F9CF for ; Wed, 10 Jul 2013 11:54:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2D90C2015F for ; Wed, 10 Jul 2013 11:54:53 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id EC1D820198 for ; Wed, 10 Jul 2013 11:54:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CE15DE6019 for ; Wed, 10 Jul 2013 04:54:47 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 09FAFE5D47 for ; Wed, 10 Jul 2013 04:54:35 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id l10so4711941eei.16 for ; Wed, 10 Jul 2013 04:54:35 -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:x-mailer; bh=FfVwT1Viuz2LqOO2Xgfs/Rxp11Q+FbrYIaD6aV5B0ag=; b=YMn7qIHAqh3q+5ks37ToXfqIXts0uZ4h1HdolwfiXc2zen/Mcuv+fCtnq2GKgf2U9X IIRtakpmcey7YoJg9JWOkz5qru2TUR8hJHI8DalxCHKoUhJcK747Rs436rCCDhfd7uey EtAVTEMkRWiRx1FWB4GTfitcMdewbNxkDHSmk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=FfVwT1Viuz2LqOO2Xgfs/Rxp11Q+FbrYIaD6aV5B0ag=; b=o5UfVlE/HGF1Gsjv5dJC21J71DfxkhwwPFkLfg2EgGPjkkxAflcCkWegb4pde/LJ7o Svl6vBFmQlJksOga8OjeFNjKgFJCBCR5nxoVttpRWHtHghSG92PZfbMvnbjLNrXIuBPC EZYOthQpGzZrfnzx7qI7V/mC7TN7Z1ZJFyFQq9Bs1NOGhg80B5qRz2T/KN2dVhkzrPHA LFpUAseWWM6ct3F9yS6BGFmu86TrcXRyP0jURNYc6ImgWvJRzz7joQe9HT32vzNwQUrP 1SLLffgRMB/ZcZujGFnBQiywLFlHPgSCyQ2dm0Z37YPBL+unqydKAPJe4OL4fk4pqbtC r7hA== X-Received: by 10.14.202.70 with SMTP id c46mr36131622eeo.28.1373457275153; Wed, 10 Jul 2013 04:54:35 -0700 (PDT) Received: from phenom.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPSA id o5sm59403335eef.5.2013.07.10.04.54.33 for (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 10 Jul 2013 04:54:34 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper Date: Wed, 10 Jul 2013 13:54:33 +0200 Message-Id: <1373457273-5800-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.3.2 X-Gm-Message-State: ALoCoQmPj2+SRuY66syoMJZeDfL/CTBd6Z5AfRQ5hNZSYglKGVBiEY/NAtmxDEWfXlhKg1w08NkI Cc: Daniel Vetter 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.4 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. 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. Cc: 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 85e450e..64a99b3 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; }