On Thu, 23 Nov 2017, Greg Kroah-Hartman wrote: > On Thu, Nov 23, 2017 at 11:24:38AM +1100, Finn Thain wrote: > > On Mon, 20 Nov 2017, I wrote: > > > > > > You need to free up the memory allocated, and I don't see that > > > > happening here ... The kernel should yell at you ... > > > > > > > > WARN(1, KERN_ERR "Device '%s' does not have a release() " > > > "function, it is broken and must be fixed.\n", > > > dev_name(dev)); > > > > > > This won't fire unless device_del() is called, right? > > > > Sorry, I should have written, "This won't fire unless > > device_unregister() is called, right?" -- though I guess it could be > > any call to put_device(). > > > > If need be I can add code to cleanly tear down the bus devices and the > > associated linked lists and procfs structures, just prior to kernel > > termination, as a kernel exitcall. But I don't see this pattern in > > use. > > When the kernel shuts down, no, the devices are not removed. > > But what happens when the bus code is unloaded if it is built as a > module? The devices will be removed then. Or they should be. >
This bus driver is not a module. > So please implement the remove device code path, OK. > just because some other busses are buggy that way does not mean you need > to duplicate their incorrect behavior. > Actually, I think the bug is in porting.txt, when it says "Optionally, the bus driver may set the device's name and release fields." > > > > It's not clear to me that the extra complexity is worth it. This may > > explain the other devices which never get unregistered (e.g. > > rtc_device, rtc_efi_dev, etc.) > > > > I've read Documentation/driver-model/ and watched your presentations > > on this topic but it's unclear to me whether you are saying in this > > thread that calling device_unregister() is mandatory. > > > > It sounds like you are saying that a non-NULL device.release method is > > mandatory (which is easily solved with an empty function). But > > Documentation/driver-model/porting.txt says the release method is > > optional. > > If you provide a non-NULL empty release function, you get to be made fun > of, as per the in-kernel kobject documentation. The kernel is trying to > save you from yourself, that warning is not there just to try to work > around. > That warning never shows up at all, because it would only ever appear at device_unregister() time, rather than at device_register() time. Anyway, I will read the in-kernel comments in the kobject code. Thanks for the tip. -- > thanks, > > greg k-h
