Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751965AbdHEOr5 (ORCPT ); Sat, 5 Aug 2017 10:47:57 -0400 Received: from smtp.domeneshop.no ([194.63.252.55]:54145 "EHLO smtp.domeneshop.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751226AbdHEOrz (ORCPT ); Sat, 5 Aug 2017 10:47:55 -0400 Subject: Re: [PATCH v5 2/6] drm/bridge: Add a devm_ allocator for panel bridge. From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= To: Ilia Mirkin , Eric Anholt Cc: Daniel Vetter , Thierry Reding , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Laurent Pinchart References: <20170718210510.12229-1-eric@anholt.net> <20170718210510.12229-2-eric@anholt.net> <2023170.2CE3xBA9bq@avalon> <87zibfqd1u.fsf@eliezer.anholt.net> Message-ID: <6361373a-f865-26c8-0bf4-a4815a940c90@tronnes.org> Date: Sat, 5 Aug 2017 16:47:27 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2623 Lines: 78 Den 05.08.2017 12.59, skrev Noralf Trønnes: > (I had to switch to Daniel's Intel address to get this sent) > > Den 05.08.2017 00.19, skrev Ilia Mirkin: >> On Fri, Aug 4, 2017 at 4:43 PM, Eric Anholt wrote: >>> Laurent Pinchart writes: >>> >>>> Hi Eric, >>>> >>>> (CC'ing Daniel) >>>> >>>> Thank you for the patch. >>>> >>>> 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? > It seems plausible looking at the code: void device_initialize(struct device *dev) { [...] kobject_init(&dev->kobj, &device_ktype); [...] } static struct kobj_type device_ktype = { .release = device_release, }; /** * device_release - free device structure. * @kobj: device's kobject. * * This is called once the reference count for the object * reaches 0. We forward the call to the device's release * method, which should handle actually freeing the structure. */ static void device_release(struct kobject *kobj) { [...] devres_release_all(dev); [...] } Last put call chain: put_device() -> kobject_put() -> kref_put() -> kobject_release() -> kobject_cleanup() -> device_release() -> devres_release_all() But I haven't actually tried it, so I might be mistaken.