On Mon, Oct 07, 2024 at 11:33:09AM +0300, Heikki Krogerus wrote: > Hi, > > On Fri, Oct 04, 2024 at 11:17:01AM -0300, Thadeu Lima de Souza Cascardo wrote: > > On Fri, Oct 04, 2024 at 05:08:28PM +0300, Heikki Krogerus wrote: > > > On Fri, Oct 04, 2024 at 09:37:38AM -0300, Thadeu Lima de Souza Cascardo wrote: > > > > The altmode device release refers to its parent device, but without keeping > > > > a reference to it. > > > > > > > > When registering the altmode, get a reference to the parent and put it in > > > > the release function. > > > > > > > > Before this fix, when using CONFIG_DEBUG_KOBJECT_RELEASE, we see issues > > > > like this: > > > > > > Let me study what's going on in the drivers code. The children should > > > _not_ be cleaned first before the parent. I'll have to come back to > > > this on Monday. > > > > > > This really should not be necessary. > > > > > > > Well, they are likely not. But driver core API states that either way, you > > should keep such references. And one way to test it is using > > CONFIG_DEBUG_KOBJECT_RELEASE. That delays the actual release/cleanup of the > > struct device, so: > > What I want to understand is, what is the rationale for this behaviour > in the core. If this is something that the drivers can do, then why is > the core not doing it for everything? Why is the core decrementing the > parent reference count already in device_del() instead of > device_release()? > > I'm probable missing something, but I really want to understand this. > There are other drivers also need resources from their parent, so if > there is nothing preventing this from being handled in the core, then > that's where it really should be handled. > > thanks, > > -- > heikki > It has been like that for 20+ years, it seems. I noticed that you have changed the behavior in the case of kobject parent lifetime relatively recently (back in 2020). One potential challenge of changing this is that dev->parent is usually set even before device_initialize gets called. At least, the reference must be get there at device_initialize. A quick glance here shows that the parent may be set between device_initialize and device_add, including in device_create_groups_vargs. Around 315 calls would need to be inspected, I would guess more than 100 callsites would need to be "fixed" for this change in the API. And, then, how many really need to keep the reference to the parent, and wouldn't keep such a reference cause any problems in any of these cases? Cascardo.
