lkml.org 
[lkml]   [2017]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 13/13] nubus: Add support for the driver model
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

\
 
 \ /
  Last update: 2017-11-24 00:40    [W:0.074 / U:3.860 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site