Re: [PATCH v2 13/13] nubus: Add support for the driver model

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux