From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v5 2/6] drm/bridge: Add a devm_ allocator for panel bridge. Date: Mon, 07 Aug 2017 13:22:23 +0300 Message-ID: <30057693.2anM9zYF0F@avalon> References: <20170718210510.12229-1-eric@anholt.net> <20170807092507.b735ribfpjm6oejk@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id 290916E3DE for ; Mon, 7 Aug 2017 10:22:09 +0000 (UTC) In-Reply-To: <20170807092507.b735ribfpjm6oejk@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Thierry Reding , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org SGkgRGFuaWVsLAoKT24gTW9uZGF5IDA3IEF1ZyAyMDE3IDExOjI1OjA3IERhbmllbCBWZXR0ZXIg d3JvdGU6Cj4gT24gU2F0LCBBdWcgMDUsIDIwMTcgYXQgMTI6NTk6MDdQTSArMDIwMCwgTm9yYWxm IFRyw7hubmVzIHdyb3RlOgo+ID4gRGVuIDA1LjA4LjIwMTcgMDAuMTksIHNrcmV2IElsaWEgTWly a2luOgo+ID4+IE9uIEZyaSwgQXVnIDQsIDIwMTcgYXQgNDo0MyBQTSwgRXJpYyBBbmhvbHQgPGVy aWNAYW5ob2x0Lm5ldD4gd3JvdGU6Cj4gPj4+IExhdXJlbnQgUGluY2hhcnQgPGxhdXJlbnQucGlu Y2hhcnRAaWRlYXNvbmJvYXJkLmNvbT4gd3JpdGVzOgo+ID4+Pj4gT24gVHVlc2RheSAxOCBKdWwg MjAxNyAxNDowNTowNiBFcmljIEFuaG9sdCB3cm90ZToKPiA+Pj4+PiBUaGlzIHdpbGwgbGV0IGRy aXZlcnMgcmVkdWNlIHRoZSBlcnJvciBjbGVhbnVwIHRoZXkgbmVlZCwgaW4KPiA+Pj4+PiBwYXJ0 aWN1bGFyIHRoZSAiaXNfcGFuZWxfYnJpZGdlIiBmbGFnLgo+ID4+Pj4+IAo+ID4+Pj4+IHYyOiBT bGlnaHQgY2xlYW51cCBvZiByZW1vdmUgZnVuY3Rpb24gYnkgQW5kcnplago+ID4+Pj4gCj4gPj4+ PiBJIGp1c3Qgd2FudCB0byBwb2ludCBvdXQgdGhhdCwgaW4gdGhlIGNvbnRleHQgb2YgRGFuaWVs J3Mgd29yayBvbgo+ID4+Pj4gaG90LXVucGx1ZywgOTAlIG9mIHRoZSBkZXZtXyogYWxsb2NhdGlv bnMgYXJlIHdyb25nIGFuZCB3aWxsIGdldCBpbgo+ID4+Pj4gdGhlIHdheS4gQWxsIERSTSBjb3Jl IG9iamVjdHMgdGhhdCBhcmUgYWNjZXNzaWJsZSBvbmUgd2F5IG9yCj4gPj4+PiBhbm90aGVyIGZy b20gdXNlcnNwYWNlIHdpbGwgbmVlZCB0byBiZSBwcm9wZXJseSByZWZlcmVuY2UtY291bnRlZAo+ ID4+Pj4gYW5kIGZyZWVkIG9ubHkgd2hlbiB0aGUgbGFzdCByZWZlcmVuY2UgZGlzYXBwZWFycywg d2hpY2ggY291bGQgYmUKPiA+Pj4+IHdlbGwgYWZ0ZXIgdGhlIGNvcnJlc3BvbmRpbmcgZGV2aWNl IGlzIHJlbW92ZWQuIEkgYmVsaWV2ZSB0aGlzCj4gPj4+PiBjb3VsZCBiZSBvbmUgc3VjaCBvYmpl Y3RzIDotLwo+ID4+PiAKPiA+Pj4gU3VyZSwgaWYgeW91J3JlIGhvdHBsdWdnaW5nLCB5b3VyIGxp ZmUgaXMgcGFpbi4gIEZvciBub24taG90cGx1Z2dhYmxlCj4gPj4+IGRldmljZXMsIGxpa2Ugb3Vy IFNPQyBwbGF0Zm9ybSBkZXZpY2VzIChjdXJyZW50IHBhbmVsLWJyaWRnZQo+ID4+PiBjb25zdW1l cnMpLCB0aGlzIHN0aWxsIHNlZW1zIGxpa2UgYW4gZXhjZWxsZW50IHNpbXBsaWZpY2F0aW9uIG9m Cj4gPj4+IG1lbW9yeSBtYW5hZ2VtZW50Lgo+ID4+IAo+ID4+IEF0IHRoYXQgcG9pbnQgeW91IG1h eSBhcyB3ZWxsIG1ha2UgeW91ciBtb2R1bGUgbm9uLXVubG9hZGFibGUsIGFuZAo+ID4+IHJldHVy biBmYWlsdXJlIHdoZW4gdHJ5aW5nIHRvIHJlbW92ZSBhIGRldmljZSBmcm9tIG1hbmFnZW1lbnQg YnkgdGhlCj4gPj4gZHJpdmVyICh3aGF0ZXZlciB0aGUgb3Bwb3NpdGUgb2YgInByb2JlIiBpcywg SSBmb3JnZXQpLiBIb3RwbHVnZ2luZwo+ID4+IGRvZXNuJ3Qgb25seSBoYXBwZW4gd2hlbiBwaHlz aWNhbGx5IHJlbW92aW5nLCBpdCBjYW4gaGFwcGVuIGZvciBhbGwKPiA+PiBraW5kcyBvZiByZWFz b25zLi4uIGFuZCB1c2Vyc3BhY2UgbWF5IHN0aWxsIGhvbGQgcmVmZXJlbmNlcyBpbiBzb21lIG9m Cj4gPj4gdGhvc2UgY2FzZXMuCj4gPiAKPiA+IElmIGRybV9vcGVuKCkgZ2V0cyBhIHJlZiBvbiBk ZXYtPmRldiBhbmQgcHV0cyBpdCBpbiBkcm1fcmVsZWFzZSgpLAo+ID4gd29uJ3QgdGhhdCBkZWxh eSBkZXZtXyogY2xlYW51cCB1bnRpbCB1c2Vyc3BhY2UgaXMgZG9uZT8KPiAKPiBOby4gZHJtX2Rl dmljZSBpcyB0aGUgdGhpbmcgdGhhdCBpcyByZWZjb3VudGVkIGZvciB1c2Vyc3BhY2UgcmVmZXJl bmNlcwo+IGxpa2Ugb3BlbiBGRCAod2UncmUgbm90IHBlcmZlY3QgYWJvdXQgaXQsIGUuZy4gc3lz ZnMgYW5kIGRtYS1idWYvZmVuY2UKPiBkb24ndCkuCj4gCj4gZGV2bV8gb3RvaCBpcyB0aWVkIHRv IHRoZSBsaWZldGltZSBvZiB0aGUgdW5kZXJseWluZyBkZXZpY2UsIGFuZCB0aGF0IG9uZQo+IGNh biBnZXQgb3V0bGl2ZWQgYnkgZHJtX2RldmljZS4gT3IgYXQgbGVhc3QgYWZhaXVpLCBkZXZtXyBz dHVmZiBpcyBudWtlZAo+IG9uIHVucGx1ZywgYW5kIG5vdCB3aGVuIHRoZSBmaW5hbCBzdyByZWZl cmVuY2Ugb2YgdGhlIHN0cnVjdCBkZXZpY2UKPiBkaXNhcHBlYXJzLgo+IAo+IE5vdCBzdXJlIHRv dWdoLCBpdCdzIGNvbXBsaWNhdGVkLgoKSXQncyBjb21wbGljYXRlZCB3aGVuIHlvdSBoYXZlIHRv IHVuZGVyc3RhbmQgdGhlIGJlaGF2aW91ciBieSByZWFkaW5nIHRoZSAKY29kZSwgYnV0IHRoZSBi ZWhhdmlvdXIgaXNuJ3QgdGhhdCBjb21wbGV4LiBkZXZtIHJlc291cmNlcyBhcmUgcmVsZWFzZWQg Ym90aAoKMS4gcmlnaHQgYWZ0ZXIgdGhlIGRyaXZlcidzIC5yZW1vdmUoKSBvcGVyYXRpb24gcmV0 dXJucwoyLiB3aGVuIHRoZSBkZXZpY2UgaXMgZGVsZXRlZCAobGFzdCByZWZlcmVuY2UgcmVsZWFz ZWQpCgpJdCdzIHRoZSBmaXJzdCBvbmUgdGhhdCBtYWtlcyBkZXZtXyogYWxsb2NhdGlvbiB1bnN1 aXRhYmxlIGZvciBhbnkgc3RydWN0dXJlIAp0aGF0IGlzIGFjY2Vzc2libGUgZnJvbSB1c2Vyc3Bh Y2UgKHN1Y2ggYXMgaW4gZmlsZSBvcGVyYXRpb24gaGFuZGxlcnMpLgoKLS0gClJlZ2FyZHMsCgpM YXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752853AbdHGKWK (ORCPT ); Mon, 7 Aug 2017 06:22:10 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:41043 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752645AbdHGKWJ (ORCPT ); Mon, 7 Aug 2017 06:22:09 -0400 From: Laurent Pinchart To: Daniel Vetter Cc: Noralf =?ISO-8859-1?Q?Tr=F8nnes?= , Ilia Mirkin , Eric Anholt , Daniel Vetter , Thierry Reding , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" Subject: Re: [PATCH v5 2/6] drm/bridge: Add a devm_ allocator for panel bridge. Date: Mon, 07 Aug 2017 13:22:23 +0300 Message-ID: <30057693.2anM9zYF0F@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.34-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <20170807092507.b735ribfpjm6oejk@phenom.ffwll.local> References: <20170718210510.12229-1-eric@anholt.net> <20170807092507.b735ribfpjm6oejk@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v77AMFUt027419 Hi Daniel, On Monday 07 Aug 2017 11:25:07 Daniel Vetter wrote: > On Sat, Aug 05, 2017 at 12:59:07PM +0200, Noralf Trønnes wrote: > > Den 05.08.2017 00.19, skrev Ilia Mirkin: > >> On Fri, Aug 4, 2017 at 4:43 PM, Eric Anholt wrote: > >>> Laurent Pinchart writes: > >>>> On Tuesday 18 Jul 2017 14:05:06 Eric Anholt wrote: > >>>>> This will let drivers reduce the error cleanup they need, in > >>>>> particular the "is_panel_bridge" flag. > >>>>> > >>>>> v2: Slight cleanup of remove function by Andrzej > >>>> > >>>> I just want to point out that, in the context of Daniel's work on > >>>> hot-unplug, 90% of the devm_* allocations are wrong and will get in > >>>> the way. All DRM core objects that are accessible one way or > >>>> another from userspace will need to be properly reference-counted > >>>> and freed only when the last reference disappears, which could be > >>>> well after the corresponding device is removed. I believe this > >>>> could be one such objects :-/ > >>> > >>> Sure, if you're hotplugging, your life is pain. For non-hotpluggable > >>> devices, like our SOC platform devices (current panel-bridge > >>> consumers), this still seems like an excellent simplification of > >>> memory management. > >> > >> At that point you may as well make your module non-unloadable, and > >> return failure when trying to remove a device from management by the > >> driver (whatever the opposite of "probe" is, I forget). Hotplugging > >> doesn't only happen when physically removing, it can happen for all > >> kinds of reasons... and userspace may still hold references in some of > >> those cases. > > > > If drm_open() gets a ref on dev->dev and puts it in drm_release(), > > won't that delay devm_* cleanup until userspace is done? > > No. drm_device is the thing that is refcounted for userspace references > like open FD (we're not perfect about it, e.g. sysfs and dma-buf/fence > don't). > > devm_ otoh is tied to the lifetime of the underlying device, and that one > can get outlived by drm_device. Or at least afaiui, devm_ stuff is nuked > on unplug, and not when the final sw reference of the struct device > disappears. > > Not sure tough, it's complicated. It's complicated when you have to understand the behaviour by reading the code, but the behaviour isn't that complex. devm resources are released both 1. right after the driver's .remove() operation returns 2. when the device is deleted (last reference released) It's the first one that makes devm_* allocation unsuitable for any structure that is accessible from userspace (such as in file operation handlers). -- Regards, Laurent Pinchart