From patchwork Fri Jun 14 22:13:16 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2725251 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 A5E2F9F967 for ; Fri, 14 Jun 2013 22:18:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B93E5201EF for ; Fri, 14 Jun 2013 22:18:15 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id DEC3C201B8 for ; Fri, 14 Jun 2013 22:18:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EBC0DE5FCE for ; Fri, 14 Jun 2013 15:18:14 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ea0-f181.google.com (mail-ea0-f181.google.com [209.85.215.181]) by gabe.freedesktop.org (Postfix) with ESMTP id 35326E5CDE for ; Fri, 14 Jun 2013 15:13:32 -0700 (PDT) Received: by mail-ea0-f181.google.com with SMTP id a15so632341eae.26 for ; Fri, 14 Jun 2013 15:13:31 -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:in-reply-to:references; bh=FFt+ZMNxc4ho6J2C6io1vZygf/C6aUi4n7fFre7XTFg=; b=J3j07wVxz8U2bBZnecTnxNgryAe8VRgFc9M8P6VNqnE8pyB0MN9BrQDAgnDANIKPi8 5U97VnS2eI+YCtrbC08kjL9fXSio5t78zLfa6YcNUpQ8rxnG/aLg9iXgISdKqRyxYnEN opO5ZZgeYo116Vgj+ypcmmdGnnUdd0caIygX4= 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:in-reply-to:references :x-gm-message-state; bh=FFt+ZMNxc4ho6J2C6io1vZygf/C6aUi4n7fFre7XTFg=; b=KSJAAds5PmYote+WYc4weKlA/RDT2/n8QavippG92/fW/ixrx4ija2ogXQi/GfZyQN LBcl8mfpzaVM6lvoGquo2PgEXIaXJGREg3MJIveji4l2OlI7uUZnQdZuDkLbrBU+fhD3 mRWKyV3k3K3cbGxmwHb/h2tcG7RObBGHwL22k4//9nk072x/7TPK5FHmdq3XXxYtz00Q 5NSQgvtnM5GZ2WuZG8HCJUXDvJB9HAED1QzOa/XevwRJS7Tkd+uVw3qRGjHix79pxS2m gj1REqsw+bVxtTkP14kkhysMN8WBw5xHrvN98sn3Gyj7XoTR/tf6Cu4j0iFRaGMuuCS1 dcYg== X-Received: by 10.15.101.2 with SMTP id bo2mr5060367eeb.57.1371248011223; Fri, 14 Jun 2013 15:13:31 -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 m1sm6258301eex.17.2013.06.14.15.13.29 for (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 14 Jun 2013 15:13:30 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 6/6] drm: fix fb leak in setcrtc Date: Sat, 15 Jun 2013 00:13:16 +0200 Message-Id: <1371247996-6052-7-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1371247996-6052-1-git-send-email-daniel.vetter@ffwll.ch> References: <1371247996-6052-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQmve9B2p128eBRyTlVsAJHpK7mb2sUK5RlJwDUOlMl/1iLI4CBpbFqr6U55mPXKYY+g8D4I Cc: Daniel Vetter , Russell King , stable@vger.kernel.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: , 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 Drivers are allowed (actually have to) disable unrelated crtcs in their ->set_config callback (when we steal all the connectors from that crtc). If they do that they'll clear crtc->fb to NULL. Which results in a refcount leak, since the drm core is keeping track of that reference. To fix this track the old fb of all crtcs and adjust references for all of them. Of course, since we only hold an additional reference for the fb for the current crtc we need to increase refcounts before we drop the old one. This approach has the benefit that it inches us a bit closer to an atomic modeset world, where we want to update the config of all crtcs in one step. This regression has been introduce in the framebuffer refcount conversion, specifically in commit b0d1232589df5575c5971224ac4cb30e7e525884 Author: Daniel Vetter Date: Tue Dec 11 01:07:12 2012 +0100 drm: refcounting for crtc framebuffers Reported-by: Russell King Cc: Russell King Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 22 ++++++++++++++++------ include/drm/drm_crtc.h | 4 ++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index bcee25a..3486a00 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1931,21 +1931,31 @@ out: int drm_mode_set_config_internal(struct drm_mode_set *set) { struct drm_crtc *crtc = set->crtc; - struct drm_framebuffer *fb, *old_fb; + struct drm_framebuffer *fb; + struct drm_crtc *tmp; int ret; - old_fb = crtc->fb; + /* + * NOTE: ->set_config can also disable other crtcs (if we steal all + * connectors from it), hence we need to refcount the fbs across all + * crtcs. Atomic modeset will have saner semantics ... + */ + list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) + tmp->old_fb = tmp->fb; + fb = set->fb; ret = crtc->funcs->set_config(set); if (ret == 0) { /* crtc->fb must be updated by ->set_config, enforces this. */ WARN_ON(fb != crtc->fb); + } - if (old_fb) - drm_framebuffer_unreference(old_fb); - if (fb) - drm_framebuffer_reference(fb); + list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) { + if (tmp->fb) + drm_framebuffer_reference(tmp->fb); + if (tmp->old_fb) + drm_framebuffer_unreference(tmp->old_fb); } return ret; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 53c33e2..07d1dbbb 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -409,6 +409,10 @@ struct drm_crtc { /* framebuffer the connector is currently bound to */ struct drm_framebuffer *fb; + /* Temporary tracking of the old fb while a modeset is ongoing. Used + * by drm_mode_set_config_internal to implement correct refcounting. */ + struct drm_framebuffer *old_fb; + bool enabled; /* Requested mode from modesetting. */