Re: [ofa-general] IB kernel modules and the kobject release() method

Re: [ofa-general] IB kernel modules and the kobject release() method

[Bart Van Assche]

On Thu, Aug 6, 2009 at 9:58 PM, Roland Dreier<rdreier@cisco.com> wrote:
>
>  > Are you sure that this indicates a shortcoming in the kobject
>  > debugging code ? The most recent messages related to the message "does
>  > not have a release() function, it is broken and must be fixed" I could
>  > find on the LKML date from July 16, 2009
>  > (http://lkml.org/lkml/2009/7/16/306 and
>  > http://lkml.org/lkml/2009/7/16/391). As you can see Greg KH
>  > acknowledges that if this message is logged that this indicates a
>  > problem that should be fixed.
>
> I'm not sure -- I just assume that the core module unloading code is
> working OK, since it is so heavily tested.  If there were really a "must
> be fixed" problem with module unloading then someone would surely have
> hit more than a warning message.

(added Greg KH and the LKML in CC)

I tried to look up more information about kobjects. The comment of
commit 7a6a41615bfb2f03ce797bc24104c50b42c935e5 suggests that in the
past the function kobject_cleanup() did not free the memory allocated
for static kobject names but that this was the responsibility of the
release() function. This should have been fixed in the current version
of kobject_cleanup(). So I'm wondering whether the message that
kobjects that do not have a release() function are broken still makes
sense ?

See also
* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7a6a41615bfb2f03ce797bc24104c50b42c935e5.
* http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.30.y.git;a=blob;f=lib/kobject.c

Bart.

Re: [ofa-general] IB kernel modules and the kobject release() method

[Greg KH]

On Fri, Aug 07, 2009 at 09:26:33AM +0200, Bart Van Assche wrote:
> On Thu, Aug 6, 2009 at 9:58 PM, Roland Dreier<rdreier@cisco.com> wrote:
> >
> >  > Are you sure that this indicates a shortcoming in the kobject
> >  > debugging code ? The most recent messages related to the message "does
> >  > not have a release() function, it is broken and must be fixed" I could
> >  > find on the LKML date from July 16, 2009
> >  > (http://lkml.org/lkml/2009/7/16/306 and
> >  > http://lkml.org/lkml/2009/7/16/391). As you can see Greg KH
> >  > acknowledges that if this message is logged that this indicates a
> >  > problem that should be fixed.
> >
> > I'm not sure -- I just assume that the core module unloading code is
> > working OK, since it is so heavily tested.  If there were really a "must
> > be fixed" problem with module unloading then someone would surely have
> > hit more than a warning message.
> 
> (added Greg KH and the LKML in CC)
> 
> I tried to look up more information about kobjects. The comment of
> commit 7a6a41615bfb2f03ce797bc24104c50b42c935e5 suggests that in the
> past the function kobject_cleanup() did not free the memory allocated
> for static kobject names but that this was the responsibility of the
> release() function. This should have been fixed in the current version
> of kobject_cleanup(). So I'm wondering whether the message that
> kobjects that do not have a release() function are broken still makes
> sense ?

No, it still makes sense :)

thanks,

greg k-h

Re: [ofa-general] IB kernel modules and the kobject release() method

[Roland Dreier]

 > No, it still makes sense :)

So what's the fix for this?  If even you have trouble understanding
kobject lifetimes and the requirement for a release function, is there
hope for anyone else?

 - R.

Re: [ofa-general] IB kernel modules and the kobject release() method

[Kay Sievers]

On Mon, Aug 10, 2009 at 22:33, Roland Dreier<rdreier@cisco.com> wrote:
>
>  > No, it still makes sense :)
>
> So what's the fix for this?

The fix is not to embed any kobject/struct device into an object whose
lifetime is not controlled by the kobject. The kobject should just be
a pointer then, and not inlined in the same memory the object uses,
which needs the kobject. The entire kobject embedding/release logic
works only properly if the kobject can own/control the lifetime of the
enclosing object - if not, it should just not be embedded.

> If even you have trouble understanding
> kobject lifetimes

I don't think we have trouble understanding that, but we are
hopelessly lost running after people who "optimize" the use of
kobjects/struct device to save an allocation or a separate dynamic
object.

> and the requirement for a release function, is there
> hope for anyone else?

Almost all uses where kobjects/struct device are statically allocated,
an object embeds more than a single kobject, or the embedded kobject
does not control the lifetime of the enclosing object, is just a
mis-use of kobjects.

Not sure, maybe it needs a simpler API to use non-embedded kobjects,
to make it easier to reach the originating object from the kobject.
That might not be obvious how to do that in some cases.

Kay