From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 799BEC04EB8 for ; Fri, 30 Nov 2018 09:41:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 33C422146D for ; Fri, 30 Nov 2018 09:41:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="H7+f+o74" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 33C422146D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726827AbeK3Ut4 (ORCPT ); Fri, 30 Nov 2018 15:49:56 -0500 Received: from mail-ed1-f68.google.com ([209.85.208.68]:37666 "EHLO mail-ed1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726469AbeK3Ut4 (ORCPT ); Fri, 30 Nov 2018 15:49:56 -0500 Received: by mail-ed1-f68.google.com with SMTP id h15so4309533edb.4 for ; Fri, 30 Nov 2018 01:41:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=sender:date:from:to:cc:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to:user-agent; bh=BlZOvymTYw44UyxMtxgGl5eBsS3i5N6wcCdoEYYmpvE=; b=H7+f+o74E2MSaZx410NGx5RICamNom9hKdqtsf3rc7SNVlM6jyYnh9Mi4qDAJbFZkg UA2dDfEDSN4t/O1v905cwX+CND7PcWZjzB/EjZ6OT3XU0OwaBkdQAClvfr/++t8vcYbQ eWF/Ve4aoH+WH4fTZ50d2C+16mXT58wA72B3M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to:user-agent; bh=BlZOvymTYw44UyxMtxgGl5eBsS3i5N6wcCdoEYYmpvE=; b=r4SIQn3SlTA5dS+cU/WB+ql8SGpqxVmjLSz9fMAvQf/bLXL0e337naTwy1plczNFn1 h0AWIbJaoalECnt83e4rGNlbv6q6ri5cM54l0c8zmW8BfHNOs1Gf+V9QxetHuG3pEg4B rXBvA62rkBrgoaBXXvMoH+DB71rl8yZYw6l9ebp14FD3jmMctAlrnx1yqsOFFk8Jy3tb tYNERrqgxZfDO7GcA9RYkHykL7ssIHT6YbuEO7Bt4fxnd3XDm68U+swGvea30OIIgtZm enzH9mo77A6qnLF5/zrfnV20LseEsDFV7cHX4J7YXOJNXQEAnUXjUuO5ZfbgnHVUMTXC 2Mvw== X-Gm-Message-State: AA+aEWYhH7Wnz687JXHwI4wwvA9INXRHVTYyEVXc1pJK3Rb4T/BwZ93x 3vtlBau+8y/D6/NLE4heid9IrA== X-Google-Smtp-Source: AFSGD/V9+bb+10oigL0NnoTZUlbax9DRldlg1lg174uJ5YTsVNIGNOpkEvTpg48yCyUoWoh9VLO43Q== X-Received: by 2002:aa7:c152:: with SMTP id r18mr4757340edp.258.1543570871268; Fri, 30 Nov 2018 01:41:11 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id c29sm1244493eda.75.2018.11.30.01.41.09 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 30 Nov 2018 01:41:10 -0800 (PST) Date: Fri, 30 Nov 2018 10:41:08 +0100 From: Daniel Vetter To: Sam Bobroff Cc: Daniel Vetter , "airlied@gmail.com" , Dave Airlie , Dave Airlie , Linux Kernel Mailing List , dri-devel Subject: Re: [PATCH 2/2] drm/ast: Fix connector leak during driver unload Message-ID: <20181130094108.GT21184@phenom.ffwll.local> Mail-Followup-To: Sam Bobroff , "airlied@gmail.com" , Dave Airlie , Dave Airlie , Linux Kernel Mailing List , dri-devel References: <3350c5dd5ea08c71e4769ea3801290e7f9238a6c.1541397462.git.sbobroff@linux.ibm.com> <20181129020034.GA10328@tungsten.ozlabs.ibm.com> <20181130001750.GA6776@tungsten.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181130001750.GA6776@tungsten.ozlabs.ibm.com> X-Operating-System: Linux phenom 4.18.0-2-amd64 User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Nov 30, 2018 at 11:17:51AM +1100, Sam Bobroff wrote: > On Thu, Nov 29, 2018 at 09:56:53AM +0100, Daniel Vetter wrote: > > On Thu, Nov 29, 2018 at 9:05 AM Sam Bobroff wrote: > > > > > > On Thu, Nov 29, 2018 at 09:40:53AM +1000, Dave Airlie wrote: > > > > On Mon, 5 Nov 2018 at 15:59, Sam Bobroff wrote: > > > > > > > > > > When unloading the ast driver, a warning message is printed by > > > > > drm_mode_config_cleanup() because a reference is still held to one of > > > > > the drm_connector structs. > > > > > > > > > > Correct this by calling drm_framebuffer_remove() in > > > > > ast_fbdev_destroy(). > > > > > > > > > > Signed-off-by: Sam Bobroff > > > > > --- > > > > > drivers/gpu/drm/ast/ast_fb.c | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c > > > > > index 0cd827e11fa2..655372ea81e9 100644 > > > > > --- a/drivers/gpu/drm/ast/ast_fb.c > > > > > +++ b/drivers/gpu/drm/ast/ast_fb.c > > > > > @@ -263,6 +263,10 @@ static void ast_fbdev_destroy(struct drm_device *dev, > > > > > { > > > > > struct ast_framebuffer *afb = &afbdev->afb; > > > > > > > > > > + /* drm_framebuffer_remove() expects us to hold a ref, which it > > > > > + * will drop, so take one: */ > > > > > + drm_framebuffer_get(&afb->base); > > > > > + drm_framebuffer_remove(&afb->base); > > > > > > > > This doesn't seem corret, no other driver does this pattern, and I > > > > can't believe ast is special here. > > > > > > > > The get just doesn't make sense. > > > > > > Thanks for having a look at this, as I said in the cover letter I was > > > concerned that it might not be a good fix. > > > > > > But the AST driver does seem to be special (or just old?) because it > > > embeds the drm_framebuffer directly into ast_fbdev and (almost all) > > > other drivers dynamically allocate and reference count theirs. > > > > > > The drm_framebuffer_get() certainly looks weird but it is there in order > > > to cause drm_framebuffer_remove() to call legacy_remove_fb(), which it > > > won't do unless the refcount is at least 2. (And because the > > > drm_framebuffer isn't dynamically allocated in this case we don't really > > > care about the reference count anyway.) > > > > > > An alternative might be to call legacy_remove_fb() directly, but it's > > > declared static. Do you think it would be better to expose it and call > > > it directly from the AST driver code? Or is there some other better way > > > to put the drm_connectors? > > > > Your problem isn't the dynamic fb vs. embedded fb for fbdev (you're > > already using drm_framebuffer_unregister_private to handle that). Your > > problem is you're not shutting down stuff on driver unload, which > > means the fb is still in use. drm_atomic_helper_shutdown() takes care > > of that for atomic drivers. > > > > No idea anymore what to do for legacy code, probably need to open code > > a shutdown sequence. Definitely not the above. > > -Daniel > > Well, it looks like drm_crtc_force_disable_all() would also do the job, > and from looking at nouveau_display_fini() it's used there as an > alternative to drm_atomic_helper_shutdown(). Ah right, I tried looking for that one but didn't find it with a quick scan. > Would it be reasonable to call that at the start of > ast_fbdev_destroy() instead? (Testing shows that it does allow the > drm_connector to be released. Is it enough/correct though?) Yes. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2] drm/ast: Fix connector leak during driver unload Date: Fri, 30 Nov 2018 10:41:08 +0100 Message-ID: <20181130094108.GT21184@phenom.ffwll.local> References: <3350c5dd5ea08c71e4769ea3801290e7f9238a6c.1541397462.git.sbobroff@linux.ibm.com> <20181129020034.GA10328@tungsten.ozlabs.ibm.com> <20181130001750.GA6776@tungsten.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-ed1-x544.google.com (mail-ed1-x544.google.com [IPv6:2a00:1450:4864:20::544]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8DB166E5DE for ; Fri, 30 Nov 2018 09:41:12 +0000 (UTC) Received: by mail-ed1-x544.google.com with SMTP id h50so4311757ede.5 for ; Fri, 30 Nov 2018 01:41:12 -0800 (PST) Content-Disposition: inline In-Reply-To: <20181130001750.GA6776@tungsten.ozlabs.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Sam Bobroff Cc: Dave Airlie , Linux Kernel Mailing List , dri-devel , Dave Airlie List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBOb3YgMzAsIDIwMTggYXQgMTE6MTc6NTFBTSArMTEwMCwgU2FtIEJvYnJvZmYgd3Jv dGU6Cj4gT24gVGh1LCBOb3YgMjksIDIwMTggYXQgMDk6NTY6NTNBTSArMDEwMCwgRGFuaWVsIFZl dHRlciB3cm90ZToKPiA+IE9uIFRodSwgTm92IDI5LCAyMDE4IGF0IDk6MDUgQU0gU2FtIEJvYnJv ZmYgPHNib2Jyb2ZmQGxpbnV4LmlibS5jb20+IHdyb3RlOgo+ID4gPgo+ID4gPiBPbiBUaHUsIE5v diAyOSwgMjAxOCBhdCAwOTo0MDo1M0FNICsxMDAwLCBEYXZlIEFpcmxpZSB3cm90ZToKPiA+ID4g PiBPbiBNb24sIDUgTm92IDIwMTggYXQgMTU6NTksIFNhbSBCb2Jyb2ZmIDxzYm9icm9mZkBsaW51 eC5pYm0uY29tPiB3cm90ZToKPiA+ID4gPiA+Cj4gPiA+ID4gPiBXaGVuIHVubG9hZGluZyB0aGUg YXN0IGRyaXZlciwgYSB3YXJuaW5nIG1lc3NhZ2UgaXMgcHJpbnRlZCBieQo+ID4gPiA+ID4gZHJt X21vZGVfY29uZmlnX2NsZWFudXAoKSBiZWNhdXNlIGEgcmVmZXJlbmNlIGlzIHN0aWxsIGhlbGQg dG8gb25lIG9mCj4gPiA+ID4gPiB0aGUgZHJtX2Nvbm5lY3RvciBzdHJ1Y3RzLgo+ID4gPiA+ID4K PiA+ID4gPiA+IENvcnJlY3QgdGhpcyBieSBjYWxsaW5nIGRybV9mcmFtZWJ1ZmZlcl9yZW1vdmUo KSBpbgo+ID4gPiA+ID4gYXN0X2ZiZGV2X2Rlc3Ryb3koKS4KPiA+ID4gPiA+Cj4gPiA+ID4gPiBT aWduZWQtb2ZmLWJ5OiBTYW0gQm9icm9mZiA8c2JvYnJvZmZAbGludXguaWJtLmNvbT4KPiA+ID4g PiA+IC0tLQo+ID4gPiA+ID4gIGRyaXZlcnMvZ3B1L2RybS9hc3QvYXN0X2ZiLmMgfCA0ICsrKysK PiA+ID4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNCBpbnNlcnRpb25zKCspCj4gPiA+ID4gPgo+ID4g PiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9hc3QvYXN0X2ZiLmMgYi9kcml2ZXJz L2dwdS9kcm0vYXN0L2FzdF9mYi5jCj4gPiA+ID4gPiBpbmRleCAwY2Q4MjdlMTFmYTIuLjY1NTM3 MmVhODFlOSAxMDA2NDQKPiA+ID4gPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9hc3QvYXN0X2Zi LmMKPiA+ID4gPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9hc3QvYXN0X2ZiLmMKPiA+ID4gPiA+ IEBAIC0yNjMsNiArMjYzLDEwIEBAIHN0YXRpYyB2b2lkIGFzdF9mYmRldl9kZXN0cm95KHN0cnVj dCBkcm1fZGV2aWNlICpkZXYsCj4gPiA+ID4gPiAgewo+ID4gPiA+ID4gICAgICAgICBzdHJ1Y3Qg YXN0X2ZyYW1lYnVmZmVyICphZmIgPSAmYWZiZGV2LT5hZmI7Cj4gPiA+ID4gPgo+ID4gPiA+ID4g KyAgICAgICAvKiBkcm1fZnJhbWVidWZmZXJfcmVtb3ZlKCkgZXhwZWN0cyB1cyB0byBob2xkIGEg cmVmLCB3aGljaCBpdAo+ID4gPiA+ID4gKyAgICAgICAgKiB3aWxsIGRyb3AsIHNvIHRha2Ugb25l OiAqLwo+ID4gPiA+ID4gKyAgICAgICBkcm1fZnJhbWVidWZmZXJfZ2V0KCZhZmItPmJhc2UpOwo+ ID4gPiA+ID4gKyAgICAgICBkcm1fZnJhbWVidWZmZXJfcmVtb3ZlKCZhZmItPmJhc2UpOwo+ID4g PiA+Cj4gPiA+ID4gVGhpcyBkb2Vzbid0IHNlZW0gY29ycmV0LCBubyBvdGhlciBkcml2ZXIgZG9l cyB0aGlzIHBhdHRlcm4sIGFuZCBJCj4gPiA+ID4gY2FuJ3QgYmVsaWV2ZSBhc3QgaXMgc3BlY2lh bCBoZXJlLgo+ID4gPiA+Cj4gPiA+ID4gVGhlIGdldCBqdXN0IGRvZXNuJ3QgbWFrZSBzZW5zZS4K PiA+ID4KPiA+ID4gVGhhbmtzIGZvciBoYXZpbmcgYSBsb29rIGF0IHRoaXMsIGFzIEkgc2FpZCBp biB0aGUgY292ZXIgbGV0dGVyIEkgd2FzCj4gPiA+IGNvbmNlcm5lZCB0aGF0IGl0IG1pZ2h0IG5v dCBiZSBhIGdvb2QgZml4Lgo+ID4gPgo+ID4gPiBCdXQgdGhlIEFTVCBkcml2ZXIgZG9lcyBzZWVt IHRvIGJlIHNwZWNpYWwgKG9yIGp1c3Qgb2xkPykgYmVjYXVzZSBpdAo+ID4gPiBlbWJlZHMgdGhl IGRybV9mcmFtZWJ1ZmZlciBkaXJlY3RseSBpbnRvIGFzdF9mYmRldiBhbmQgKGFsbW9zdCBhbGwp Cj4gPiA+IG90aGVyIGRyaXZlcnMgZHluYW1pY2FsbHkgYWxsb2NhdGUgYW5kIHJlZmVyZW5jZSBj b3VudCB0aGVpcnMuCj4gPiA+Cj4gPiA+IFRoZSBkcm1fZnJhbWVidWZmZXJfZ2V0KCkgY2VydGFp bmx5IGxvb2tzIHdlaXJkIGJ1dCBpdCBpcyB0aGVyZSBpbiBvcmRlcgo+ID4gPiB0byBjYXVzZSBk cm1fZnJhbWVidWZmZXJfcmVtb3ZlKCkgdG8gY2FsbCBsZWdhY3lfcmVtb3ZlX2ZiKCksIHdoaWNo IGl0Cj4gPiA+IHdvbid0IGRvIHVubGVzcyB0aGUgcmVmY291bnQgaXMgYXQgbGVhc3QgMi4gKEFu ZCBiZWNhdXNlIHRoZQo+ID4gPiBkcm1fZnJhbWVidWZmZXIgaXNuJ3QgZHluYW1pY2FsbHkgYWxs b2NhdGVkIGluIHRoaXMgY2FzZSB3ZSBkb24ndCByZWFsbHkKPiA+ID4gY2FyZSBhYm91dCB0aGUg cmVmZXJlbmNlIGNvdW50IGFueXdheS4pCj4gPiA+Cj4gPiA+IEFuIGFsdGVybmF0aXZlIG1pZ2h0 IGJlIHRvIGNhbGwgbGVnYWN5X3JlbW92ZV9mYigpIGRpcmVjdGx5LCBidXQgaXQncwo+ID4gPiBk ZWNsYXJlZCBzdGF0aWMuICBEbyB5b3UgdGhpbmsgaXQgd291bGQgYmUgYmV0dGVyIHRvIGV4cG9z ZSBpdCBhbmQgY2FsbAo+ID4gPiBpdCBkaXJlY3RseSBmcm9tIHRoZSBBU1QgZHJpdmVyIGNvZGU/ IE9yIGlzIHRoZXJlIHNvbWUgb3RoZXIgYmV0dGVyIHdheQo+ID4gPiB0byBwdXQgdGhlIGRybV9j b25uZWN0b3JzPwo+ID4gCj4gPiBZb3VyIHByb2JsZW0gaXNuJ3QgdGhlIGR5bmFtaWMgZmIgdnMu IGVtYmVkZGVkIGZiIGZvciBmYmRldiAoeW91J3JlCj4gPiBhbHJlYWR5IHVzaW5nIGRybV9mcmFt ZWJ1ZmZlcl91bnJlZ2lzdGVyX3ByaXZhdGUgdG8gaGFuZGxlIHRoYXQpLiBZb3VyCj4gPiBwcm9i bGVtIGlzIHlvdSdyZSBub3Qgc2h1dHRpbmcgZG93biBzdHVmZiBvbiBkcml2ZXIgdW5sb2FkLCB3 aGljaAo+ID4gbWVhbnMgdGhlIGZiIGlzIHN0aWxsIGluIHVzZS4gZHJtX2F0b21pY19oZWxwZXJf c2h1dGRvd24oKSB0YWtlcyBjYXJlCj4gPiBvZiB0aGF0IGZvciBhdG9taWMgZHJpdmVycy4KPiA+ IAo+ID4gTm8gaWRlYSBhbnltb3JlIHdoYXQgdG8gZG8gZm9yIGxlZ2FjeSBjb2RlLCBwcm9iYWJs eSBuZWVkIHRvIG9wZW4gY29kZQo+ID4gYSBzaHV0ZG93biBzZXF1ZW5jZS4gRGVmaW5pdGVseSBu b3QgdGhlIGFib3ZlLgo+ID4gLURhbmllbAo+IAo+IFdlbGwsIGl0IGxvb2tzIGxpa2UgZHJtX2Ny dGNfZm9yY2VfZGlzYWJsZV9hbGwoKSB3b3VsZCBhbHNvIGRvIHRoZSBqb2IsCj4gYW5kIGZyb20g bG9va2luZyBhdCBub3V2ZWF1X2Rpc3BsYXlfZmluaSgpIGl0J3MgdXNlZCB0aGVyZSBhcyBhbgo+ IGFsdGVybmF0aXZlIHRvIGRybV9hdG9taWNfaGVscGVyX3NodXRkb3duKCkuCgpBaCByaWdodCwg SSB0cmllZCBsb29raW5nIGZvciB0aGF0IG9uZSBidXQgZGlkbid0IGZpbmQgaXQgd2l0aCBhIHF1 aWNrCnNjYW4uCiAKPiBXb3VsZCBpdCBiZSByZWFzb25hYmxlIHRvIGNhbGwgdGhhdCBhdCB0aGUg c3RhcnQgb2YKPiBhc3RfZmJkZXZfZGVzdHJveSgpIGluc3RlYWQ/IChUZXN0aW5nIHNob3dzIHRo YXQgaXQgZG9lcyBhbGxvdyB0aGUKPiBkcm1fY29ubmVjdG9yIHRvIGJlIHJlbGVhc2VkLiBJcyBp dCBlbm91Z2gvY29ycmVjdCB0aG91Z2g/KQoKWWVzLgotRGFuaWVsCi0tIApEYW5pZWwgVmV0dGVy ClNvZnR3YXJlIEVuZ2luZWVyLCBJbnRlbCBDb3Jwb3JhdGlvbgpodHRwOi8vYmxvZy5mZndsbC5j aApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2 ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9s aXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK