From patchwork Mon Mar 2 01:59:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 5909871 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 52FD7BF6C3 for ; Mon, 2 Mar 2015 02:00:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 63B9D2025B for ; Mon, 2 Mar 2015 02:00:52 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 456CF2012B for ; Mon, 2 Mar 2015 02:00:48 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 853936E35C; Sun, 1 Mar 2015 17:59:55 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ig0-f179.google.com (mail-ig0-f179.google.com [209.85.213.179]) by gabe.freedesktop.org (Postfix) with ESMTP id 331C36E332; Sun, 1 Mar 2015 17:59:54 -0800 (PST) Received: by igqa13 with SMTP id a13so13624837igq.0; Sun, 01 Mar 2015 17:59:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=PjHN6ndmdexL1s/UpSfSk8UaHFgljubjkno97txZprQ=; b=Bajof32CIcQKu3ORpBxP8cwAM8DVeODGdMAfZVCW71tYu7Yn58s3LSPqZbV+/lfe1f QAF9iUPo9hlSlMenWt54r6Ydc/DstbE+TFfvqyehENv7bM1l1ybn+1IZz8lTJGZha81t E5QxFqLKWg0SjFqaiV6r2CtdCBw4/40gq5rYsMbb8C5Ffc5DTvMkjLY9zEdXtMN9YpH6 SEVLIhUOCw/kEotBRdMk76xrwkbs8e9HkUOMVvAUiinGbylJjBYdMGTKjnEZ+kRA7bbY QHOJEVQsOrV821Mz8LO+LIY5bf8HE238/0DdlcRNPoTsxhpa2GZFH5bUi8cOe+Vz+fUV julw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=PjHN6ndmdexL1s/UpSfSk8UaHFgljubjkno97txZprQ=; b=Opqmyz6GFRttXb+4qu0XCqsmqSBYPElRuqZRBaaa8o/osNO0YE6JpkFQyR2iYBkdWV FSwGTnsavq1DitWBPUDqdLr2w6hX+s0CgEyu5WnsdbVk6dpKIV2LZjRHqDmaXxpSwpPv ZdNhZOihkZKDGiOGf97GzE0mCE6V45pyjRn4Q= MIME-Version: 1.0 X-Received: by 10.43.118.66 with SMTP id fp2mr26450469icc.45.1425261593905; Sun, 01 Mar 2015 17:59:53 -0800 (PST) Received: by 10.36.60.10 with HTTP; Sun, 1 Mar 2015 17:59:53 -0800 (PST) In-Reply-To: References: Date: Sun, 1 Mar 2015 17:59:53 -0800 X-Google-Sender-Auth: PD07ZdD1lErTZKFGmQ8MuXEzyiM Message-ID: From: Linus Torvalds To: Dave Airlie , Daniel Vetter , Jani Nikula , Matt Roper , Ander Conselvan de Oliveira Cc: intel-gfx , Linux Kernel Mailing List , DRI mailing list Subject: Re: [Intel-gfx] [git pull] drm fixes X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, 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 On Sun, Mar 1, 2015 at 1:00 PM, Linus Torvalds wrote: > > Back to the drawing board. Ok, many hours later, but I found it. The bisection was a disaster, having to work around other bugs in this area, but it ended up getting "close enough" that I figured out what went wrong. The "intel_plane_duplicate_state()" is horribly horribly buggy. It looks at the state->fb pointer, but it may have been free'd already. This workaround "works for me", but it's really still very questionable, because while the "kref_get_unless_zero()" works correctly when the last reference has been dropped, I'm not sure that there is any guarantee that the whole allocation even exists any more, so I think the *correct* thing to do would be to clear state->fb when dropping the kref. But this was the smallest working patch I could come up with. Somebody who actually knows the code should start looking at the places that do drm_framebuffer_unreference(), and actually clear that pointer instead. Added Matt Roper and Ander Conselvan de Oliveira to the discussion, since they are the ones git says are involved with the original broken intel_plane_duplicate_state(). Anyway, attached is (a) the patch with a big comment (b) the warnings I get on that machine that show where this problem triggers (and another warning earlier). Comments? I'm sure this probably only triggers with *old* X servers that don't do all the modern dri stuff. Linus From c182b15c3abee75cdc9d9564b6ab826403690f4e Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 28 Feb 2015 21:44:48 -0800 Subject: [PATCH] Workaround for drm bug Signed-off-by: Linus Torvalds --- drivers/gpu/drm/i915/intel_atomic_plane.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 9e6f727..72714d3 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -85,8 +85,23 @@ intel_plane_duplicate_state(struct drm_plane *plane) return NULL; state = &intel_state->base; - if (state->fb) - drm_framebuffer_reference(state->fb); + + /* + * We cannot do drm_framebuffer_reference(), because the reference + * may already have been dropped. + * + * So we do what drm_framebuffer_lookup() does, namely do a + * kref_get_unless_zero(). Even that is somewhat questionable, + * in that maybe the 'fb' already got free'd. So warn loudly + * about this. + * + * Maybe the base.fb should be cleared by whatever drops the + * reference? + */ + if (state->fb && !kref_get_unless_zero(&state->fb->refcount)) { + state->fb = NULL; + WARN_ONCE(1, "intel_plane_duplicate_state got plane with dead frame buffer"); + } return state; } -- 2.3.1.167.g7f4ba4b