Right now I have about 80+ patches reworking the kset/ktype mess in the

-mm tree, cleaning things up and hopefully making it all easier for

people to both use, and understand.
So, while it is all relativly fresh in my mind, I thought it would be

good to also document the whole mess, and provide some solid example

code for others to use in the future.
Jonathan, I used your old lwn.net article about kobjects as the basis

for this document, I hope you don't mind (if you do, I'll be glad to

start over).  I've updated it to what is going on in the -mm tree, and

added new information.
This file should replace the existing Documentation/kobject.txt which is

woefully out of date and obsolete now.
I also have two example kernel modules showing how to use a simple

kobject and attributes, as well as a more complex kset/ktype/kobject

interaction.  I'll reply to this message with them as well, and I am

going to place them in the samples/kobject/ directory unless someone

really objects.
Any review comments that people might have on both the document, and the

two sample modules would be greatly appreciated.
thanks,
greg k-h
-----------------------------
Everything you never wanted to know about kobjects, ksets, and ktypes
Greg Kroah-Hartman <gregkh@suse.de>
Based on an original article by Jon Corbet for lwn.net written October 1,

2003 and located at http://lwn.net/Articles/51437/
Last updated November 27, 2008
Part of the difficulty in understanding the driver model - and the kobject

abstraction upon which it is built - is that there is no obvious starting

place. Dealing with kobjects requires understanding a few different types,

all of which make reference to each other. In an attempt to make things

easier, we'll take a multi-pass approach, starting with vague terms and

adding detail as we go. To that end, here are some quick definitions of

some terms we will be working with.
 - A kobject is an object of type struct kobject.  Kobjects have a name

   and a r...
[ message continues ]

As Cornelia said, it would be worthwhile mentioning krefs in this
kobject_init() isn't mandatory if you use kobject_register().  But then 
In fact kset, ktype, and parent are optional, right?  You might mention

at this point that not all those fields are needed, and explain later
It's worth mentioning here (and perhaps elsewhere too) that all of the

function calls described here can sleep and hence must be made in

process context, with the exception of the *_get() routines.  It's

possible to call *_put() in atomic context; the SCSI core does this

(with device_put, not kobject_put) and has to jump through hoops to run

the corresponding release routine in a waitqueue task.  In general,
Not to mention that doing this will leak memory.  Unless the kobject
Actually the current code doesn't seem to check whether kobj->ktype is
That's where a kobject shows up if its parent field isn't set when

kobject_add() is called.  But if the parent field _is_ set, does
There's no need to mention this here.  Not only is it wrong, more
If there is no containing kset, the parent remains NULL.  What happens
There are no checks for this!  At least, not until the cleanup occurs.
Here's a question for you: The code in kobject.c is full of odd things

like this (from the start of kobject_add):
	if (!(kobj = kobject_get(kobj)))

		return -ENOENT;
What's the point of the assignment?  We know that kobject_get() always

returns its argument.  This sort of thing happens all over the place.
Alan Stern
-

Is this really needed?  If anyone calls them from non-process context,
They are reference counted.  Other portions of the kernel can grab them

and think they are safe to use.  If you do this with a static object,

what happens when the code goes away?
Most of the nasty race conditions that require this are now cleaned up

with Tejun's great sysfs work, so you will probably not see problems if
heh.
I think your other questions are already answered, right?
thanks,
greg k-h

-

You don't need to set kset, do you?  Or parent -- especially if kset is

set instead.  Certainly kobjects without one or the other of those are 
IMO it's better to let people know up front.  Otherwise they might get

some misconception fixed in their brain, misdesign their drivers, and

have to change a large hunk of code when they discover it doesn't work

at runtime.
That's the whole point of documentation in the first place, right?  To
Let's see.  The error scenario you propose is that a loadable module

contains a static kobject and the kobject's refcount is still positive

when the module is unloaded.  Then the holder of the remaining

reference tries to access the kobject and crashes.  Yes, that's bad.
But turn it around: Suppose the kobject were allocated dynamically

instead.  It would remain in memory when the owning module was

unloaded, but its release method would go away along with the rest of

the module.  When the last remaining reference is dropped there would

be a crash anyway.
Evidently the bug here is allowing the module to be unloaded while one

of its kobjects still has a positive refcount.  It doesn't matter 
What about static kobjects in built-in (non-modular) kernel code?

Especially if the kobject in question is never released?
Alan Stern
-

Yeah, it's a kind of convenience that causes far more problems than is

solves. Kobjects are low-level and we depend entirely on proper events
Ksets are optional, yes. Ktypes are required for releasing the object,
It all depends on the parent pointer, and I think we have far more kobjects

no showing up at the kset like all devices belong to the devices_kset but

only one or two show up directly in the kset directory, all the other are
"If the kobject belonging to a kset has no parent kobject set, it will

be added to the kset's directory. Not all members of a kset do

necessarily live in the kset directory. If an explicit parent kobject is

assigned before the kobject is added, the kobject is registered with the
It does not matter anymore to assign the values before kobject_init(),
Right, that looks not too useful.
Kay
-

No -- we should but we don't.  Look at the code for kobject_init() and

kobject_add() in Greg's tree and you'll see.  Neither of them checks
Yes, but what if neither kobj->parent nor kobj->kset is set?
Alan Stern
-

Yeah, it was another "magic" that was built into the core, the code even

tried to find sysfs_ops for completely untyped kobjects. That is gone
It will show up in the root of sysfs, yes.
Kay
-

This patch (as1020) adds a check to kobject_init() to insure that the

ktype field is not NULL.  This is just for safety's sake; as far as I

know there are no remaining places where the field is left unset.  But

ironically, kset_init() did fail to set it!  The patch fixes that and

removes some redundant initialization in kset_createa().
The patch also fixes up elevator_init(), where ktype was set after

calling kobject_init() instead of before.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

CC: Kay Sievers <kay.sievers@vrfy.org>
---
Here is the check we discussed.  I'm a little behind on Kay's most

recent updates; I hope this doesn't clash with them.
There may be some other places where ktype gets set after

kobject_init() rather than before.  My testing can't be termendously

thorough.  But they should all be easy to fix (like elevator.c was).
Index: usb-2.6/lib/kobject.c

===================================================================

--- usb-2.6.orig/lib/kobject.c

+++ usb-2.6/lib/kobject.c

@@ -188,6 +188,7 @@ void kobject_init(struct kobject * kobj)

 		return;

 	WARN_ON(atomic_read(&kobj->kref.refcount));

 	verify_dynamic_kobject_allocation(kobj);

+	WARN_ON(!kobj->ktype);

 	kref_init(&kobj->kref);

 	INIT_LIST_HEAD(&kobj->entry);

 	kobj->kset = kset_get(kobj->kset);

@@ -624,18 +625,6 @@ struct kobject *kobject_create_and_regis

 }

 EXPORT_SYMBOL_GPL(kobject_create_and_register);
-/**

- *	kset_init - initialize a kset for use

- *	@k:	kset

- */

-

-void kset_init(struct kset * k)

-{

-	kobject_init(&k->kobj);

-	INIT_LIST_HEAD(&k->list);

-	spin_lock_init(&k->list_lock);

-}

-

 /* default kobject attribute operations */

 static ssize_t kobj_attr_show(struct kobject *kobj, struct attribute *attr,

 			      char *buf)

@@ -751,6 +740,19 @@ static struct kobj_type kset_ktype = {

 };
 /**

+ *	kset_init - initialize a kset for use

+ *	@k:	kset

+ */

+

+void kset_init(struct kset * k)...
[ message continues ]

No, it's safe to set ktype after kobject_init, it was just not safe to

set the kset.  As Kay pointed out, I just added a patch to my tree to

resolve this issue, and I'll go back and update the documentation now.
I do like the "check for a ktype" warning, I'll go add that to my local

tree and see what breaks.  Odds are, all the static kobjects will :(
thanks,
greg k-h

-

You have to be careful.  The ktype check I wrote lives in

kobject_init() -- that's why I had to make elevator_init() assign the

ktype before calling kobject_init().  If you put the check into

kobject_add() instead then you won't end up checking objects that get

initialized but not added.
Yes, nobody would deliberately use a kobject without adding it, but it

could happen as the result of an failure between the _init and _add

calls.
In any case, the change to kset_init() in my patch fixes a real bug.

It never did set the ktype at all.
Alan Stern
-

On Wed, 28 Nov 2007 17:00:57 -0500 (EST),
And if someone calls kobject_put() after kobject_init() to clean up,

their release function will not be called if they didn't set the ktype.

So the check really belongs into kobject_init() IMO.

-

Hmm, will one expect that the whole object will also be free'd when we

suggest to call kobject_put() to cleanup? That might be pretty

unexpected, right?
Kay
-

Right.  And even though cleaning up no longer needs to drop a reference

to the kset, it still might need to free the kobject's name.  So for

example, either of these sequences:
	kobject_init();			kobject_set_name();

	kobject_set_name();		kobject_init();

	...				...

	kobject_free();			kobject_free();
would leak memory.
In fact, if we were designing the kobject API from scratch, I'd suggest

making the ktype value an argument to kobject_init() so that it 
I don't understand the question.  People _already_ expect the cleanup

routine to free the kobject when the last reference is dropped.  Why

should there be any confusion over this?
Alan Stern
-

Sounds fine, maybe we should also pass the name along, so it will be

obvious what happens here:
Oh, if you want to rewind on error and have an initialized but still

unregistered kobject, and just want to free the allocated name by

calling kobject_cleanup() or kobject_put() you might not expect, that

your whole object that embeds the kobject will be gone. Just something

we need to document ...
Kay
-

I don't know...  Normally *_init() routines can't fail, but this could.

Then things like device_register() would run into trouble: The caller

wouldn't know whether a failure occurred before or after the

kobject_init() call, so it wouldn't know what sort of cleanup action 
When that sort of thing happens, the unwinding should be done by the

code responsible for whole object.  For example, if device_add() fails

then the caller should go on to call device_put() rather than

kfree(dev).
That's how you would expect things to work in most cases.  There aren't

many bare kobjects in the kernel.
I agree that documenting this behavior would be good.
Alan Stern
-

But wouldn't device_register() do the kobject cleanup for you when it

fails? Why would a caller of device_register() care about the state of
Ok, fine. Hope we will collect all that information in the end. :)
Kay
-

Let's say device_register() calls device_init(), which calls

kobject_init(), which fails.  Then there's no cleanup to do --

device_register() returns -ENOMEM or some such code and the caller has

to do the kfree().
Now let's say device_register() calls device_init(), which succeeds,

and then calls device_add(), which fails.  To recover properly,

somebody then has to call device_put().  That "somebody" can't be the

original caller -- according to the previous paragraph the original

caller won't do anything but kfree().  So the "somebody" has to be

device_register() itself.
But the device_put() will call kobject_put(), which will invoke the

device's cleanup routine, which will deallocate the structure.  Now the

original caller gets an error code (perhaps -ENOMEM again) but must

_not_ call kfree().
So what should the original caller do when an error occurs?
Alan Stern
-

Right, and that is not covered today, the current code just leaks the

allocated name.
Your error scenario confirmed my initial concern about suggesting

kobject_put() to clean up an initialized kobject.
We should probably make kobject_cleanup() free only the resources taken

by kobject_init(), and use kobject_cleanup() instead of kobject_put()?
Kay
-

My conclusion is different.  We should make kobject_init() not consume

any resources at all; just initialize various fields.  That way it

would be okay to call either kfree() or kobject_put() on an initialized

kobject.  And then when something like device_register() fails, the

caller would know the proper thing to do would be to call the put()

routine, always.
Of course, once the name has been assigned, only kobject_put() should

be used.
There's another good reason for not assigning the name in

kobject_init(): Code that uses kobjects (like the driver core) doesn't

set the name until later.
Alan Stern
-

Now we just move the exactly the same problem from _init() to

_set_name(). To free the name of an unregistered we would need to call
That can be done at any stage, I guess. We will rip out the name in the

struct device anyway.
Kay
-

I don't see that as a problem and it's not clear why you do.
It doesn't matter whether a kobject has been registered or not; once

it has been initialized you _should_ call kobject_put().  (Although

it's okay to call kfree() if the name hasn't been set yet.)
The same is true of larger objects.  Once you have called

device_initialize(), you _should_ call device_put() (although it's okay

to call kfree()).  Provided init routines don't consume resources, this

will work.
The only remaining problem is that somebody might set the name first

and then decide to abandon the object before calling kobject_init().  
Are you also going to change all the places in the kernel where the

device name (.bus_id) isn't set until after device_initialize() has

been called?
Alan Stern
-

I just say, it's exactly the same problem and it does not really make a

difference that kobject_init() does not do anything, if we require

another function to be called before we can call kobject_add(). A

kobject without a name will not be valid, and we will need a way for an
What is the whole point of kobject_init() then? You can just do the same

stuff in kobject_add() if you require not to allocate anything there. 
None of the initialized fields can be used before we have called
Yes, I already have a patch that does that for all stuff that was needed

on my boxes.
Kay
-

To be exact, bus_id will be removed from the device, and the device name

will be set and retrieved directly from the kobject.
Kay
-

On Thu, 29 Nov 2007 21:26:40 +0100,
I guess you'll have accessor functions for the bus id for use by higher

level code?

-

On Thu, 29 Nov 2007 17:04:40 +0100,
Agreed. Better don't hide too much.

-

Ok, how about this function.  If it errors out, it is free to just call

kfree() on the kobject.  Seems simple enough to me, any objections?  If

not, I'll go through and fix up the whole tree...
thanks,
greg k-h
-----------------
/**

 * kobject_init - initialize a kobject structure

 * @kobj: pointer to the kobject to initialize

 * @ktype: pointer to the ktype for this kobject.

 * @fmt: the name of the kobject

 *

 * This function will properly initialize a kobject such that it can then

 * be passed to the kobject_add() call.

 *

 * If the function returns an error, the memory allocated by the kobject

 * can be safely freed, no other functions need to be called.

 */

int kobject_init(struct kobject *kobj, struct kobj_type *ktype, const char *fmt, ...)

{

	va_list args;

	int retval;
	if (!kobj)

		return -EINVAL;
	if (!ktype)

		return -EINVAL;
	WARN_ON(atomic_read(&kobj->kref.refcount));

	kref_init(&kobj->kref);

	INIT_LIST_HEAD(&kobj->entry);

	kobj->ktype = ktype;
	va_start(args, fmt);

	retval = kobject_set_name_vargs(kobj, fmt, args);

	va_end(args);
	return retval;

}
-

Looks good, _provided_ you also get rid of kobject_register().  Which

you pretty much would have to do anyway, since it doesn't accept the

ktype or the name as an argument.
Alan Stern
-

Yes, I would do that at the same time.
Well, not at the _exact_ same time, it might take a few patches to get

there :)
thanks for the review,
greg k-h

-

If the function does not return an error, the only way the kobject's

memory can be properly freed is with a call to kobject_put().  kfree()

should never be called on a kobject after this function has been called,

-

On Thu, 29 Nov 2007 13:54:55 -0800,
Yes, this looks fine.

-

On Thu, 29 Nov 2007 11:59:06 +0100,
I'd expect the kobject to be freed if I called kobject_put() on my last

reference (but that may be because I'm familiar with the code :)).

OTOH, if getting the reference on the kset is moved from kobject_init()

to kobject_add(), we aren't forced to use kobject_put() but may use

kfree().

-

No, people do deliberately do this.  Right now, after a few cleanup

patches in my tree, there are only two users who create kobjects but do

not register them, struct cdev and the kobj_map stuff.
The cdev code I'll work on cleaning up, there's just some historical

usages there that I've never felt all that comfortable with, and the

kobj_map code I'd like to convert to just use a list and a kref and not

a kobject at all so that I stop getting emails about it over time :)
thanks,
greg k-h

-

It should not harm, only the kset was needed before _init(), but even

that should be history with Greg's latest patch:
I think it's fine and elevator does not need to be changed, and we can

remove the requirement now from the documentation, as _init() now does

not depend on any pre-assigned values, right?
Kay
-

On Tue, 27 Nov 2007 15:02:52 -0800,
Perhaps better wording:
A kset is also represented via a subdirectory in sysfs, under which the
Maybe also mention kset_get()/kset_put() here? kset_unregister() will
It may be helpful to mention which uevents are by default created by
There is no field called "refcount"; the embedded struct kref kref is
Do we also want to mention kobject_rename() and kobject_move(), or are

those functions so esoteric that most people don't want to know about

them?

-

They can be found in the kerneldoc api reference if they are needed :)
thanks,
greg k-h

-

On Wed, 28 Nov 2007 22:04:58 -0800,
If we go forward with Kay's suggestion of removing kobject_register()
OK.

-

This draws a misleading picture. A member of a kset shows up where the

"parent" pointer points to. Like /sys/block is a kset, the kset contains

disks and partitions, but partitions do not live at the kset, and tons

of other kset directories where this is the case.
"If the kobject belonging to a kset has no parent kobject set, it will

be added to the kset's directory. Not all members of a kset do

necessarily live in the kset directory. If an explicit parent kobject is

assigned before the kobject is added, the kobject is registered with the
It's by far not "almost all cases". Can we please drop that? It's very
I think, we should remove all these default events from the kobject

core. We will not be able to manage the timing issues and "raw" kobject

users should request the events on their own, when they are finished

adding stuff to the kobject. I see currently no way to solve the

"attributes created after the event" problem. The new

*_create_and_register functions do not allow default attributes to be

created, which will just lead to serious trouble when someone wants to

use udev to set defaults and such things. We may just want to require an

explicit call to send the event?
Thanks,

Kay
-

Nice, thanks, I've added this :)
greg k-h

-

On Wed, 28 Nov 2007 13:23:02 +0100,
There will always be attributes that will show up later (for example,

after a device is activated). Probably the best approach is to keep the

default uevents, but have the attribute-adder send another uevent when

they are done?

-

Uh, that's more an exception where we can't give guarantees because of

very specific hardware setups, and it would be an additional "change"

event. There are valid cases for this, but only a _very_ few.
There is absolutely no reason not to do it right with the "add" event,

just because we are too lazy to solve it proper the current code. It's

just so broken by design, what we are doing today. :)
Kay
-

On Wed, 28 Nov 2007 16:57:48 +0100,
I'm worrying a bit about changes that impact the whole code tree in

lots of places. I'd be fine with the device layer doing its uevent

manually in device_add() at the very end, though. (This would allow

drivers to add attributes in their probe function before the uevent,

for example.)

-

The driver core does use the split already in most places, I did that

long ago. There are not too many (~20) users of kobject_register(), and

it's a pretty straight-forward change to change that to _init, _add,

_uevent, and get rid of that totally useless "convenience api".
I think there is no longer any excuse to keep that broken code around,

and even require to document that it's broken. The whole purpose of the

uevent is userspace consumption, which just doesn't work correctly with

the code we offer. The fix is trivial, and should be done now, and we no

longer need to fiddle around timing issues, just because we are too

lazy.
I propose the removal of _all_ funtions that have *register* in their

name, and always require the following sequence:

  _init()

  _add()

  _uevent(_ADD)
  _uevent(_REMOVE)

  _del()

  _put()
The _create_and_register() functions would become  _create_ and_add()

and will need an additional _uevent() call after they populated the

object.
Thanks,

Kay
-

On Wed, 28 Nov 2007 17:36:29 +0100,
I'm absolutely fine with doing that at the kobject level (after all,

it's a quite contained change, and the uevent function explicitely

works on a kobject).
For the other _register()/_unregister() functions, it's a different

piece of cake. They are:

- distributed through lot of different code

- at a higher level than kobjects, and kobject_uevent() acts on the

kobject

- usually encapsulating a sequence that wants to be used by almost all

callers, and that includes a uevent
I don't think we want people registering a higher level object and then

wondering why udev doesn't seem to take notice of it.

-

I think I still remember what I did 2.5 years ago :)

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commi...
Oh, I'm just talking about lib/kobject.c. And the new kobj/kset stuff we

added which is currently in the -mm tree.
It suffers from the same old problem, and even gets documentend as

"broken" now. I really think that should be fixed proper instead, and

it's the right time to do it now.
Thanks,

Kay
-

Accept that the kobject API is that low-level, that we can't have a

proper convenience API - it just does the wrong thing.

Do the same for all other in-kernel users what we did 2.5 years ago for

the driver core, and no longer use the kobject/kset_register()

functions, and let the caller send the events manually when the object

is ready, or removed.
We have ~20 callers of kobject_(un)register(), convert them to

kobject_init() + kobject_add() + kobject_uevent(), and just delete the

broken kobject_(un)register() functions from the kobject code.
Rename the new kobject/kset_create_and_register() to _create_and_add()

and also require the uevent to be sent manually when the caller is ready

with the object.
That way we get rid of a broken API which causes far more problems than

it solves. And we will no longer need to carry this in the example code:

/*

 * Note, these files will be created _after_ the kobject above

 * created.  This can cause userspace to be looking around in sysfs

 * for these files before they are really created.  If you are

 * worried about something like this, perhaps you really need to

 * create your own kset and have a default attribute group for your

 * kobject.

 */
We just get one single API, and we document that the caller needs to

send an event when it has finished populating the object, or deletes the

object.

If we are worrying about users who might forget to send events - I

really prefer missing uevents, which we can simply add, over ones with

the broken timing the current API causes, and failing userspace where

nobody understands what's going wrong at event time. :)
Thanks,

Kay
-

On Thu, 29 Nov 2007 08:50:31 +0100,
I find a bit more callers in my tree, but it still isn't too many.
This may also be a good time to evaluate which of those kobjects really

need a uevent. For example, I can imagine that you don't need a uevent

for a directory being created, but only for the objects created in that

directory.

-

True, most plain directories don't need an event (and usually don't have

one today), because the kset does not match, like the "holders/slaves"

directories, or the "glue directories" between bus and class devices. 
But for example the bus/class directories, there we really want an

uevent (and have one today) to set bus specific defaults when the

bus/class gets created.
Thanks,

Kay
-

On Tue, 27 Nov 2007 15:02:52 -0800,
Hm, hierarchy and representation in sysfs are useful things by
Unless they don't register their kobject. (But they should always set a

name anyway to avoid funny debug messages, so it is probably a good
Eh, no. We'll always return !NULL if the kobject is !NULL to start

with. If the reference count is already 0, the code will moan, but the
This is no longer true?
The major problem is that registered kobjects can be looked-up by other
We should perhaps add a bit fat warning here:
Note that once you registered your kobject via kobject_add(), you must

never use kfree() to free it directly. The only safe way is to use

kobject_put(). It is good practice to always use kobject_put() after
Which is especially hurting if you use kobjects in modules. (Which

reminds me: Must dig up the patchset that fixes the module unload vs.
Attention span ran out, will comment on the rest of this document later.

-

Good point, this was the way things used to work a long time ago, I'll remove
Now added, thanks.
thanks for the review, I really appreciate it.
greg k-h

-

Slightly ambiguous.  Instead just say:
If you have initialized your kobject via kobject_init() or

kobject_register(), you must not deallocate the kobject anywhere other

than its release() method (which is invoked during the final
In theory modules shouldn't present a problem -- especially if Greg

merges the "Kobjects: drop child->parent ref at unregistration" patch.
When a module is unloaded, it has to unregister all its kobjects, which

should force all their children to be unregistered too.  At that time

the children's drivers should drop all their references to the parent

kobject, leaving only references held by the module being unloaded.

Presumably it can arrange to drop its own references before its exit()

routine returns.
The only problem arises when a child's driver retains a reference to

the parent kobject.  If things are done properly, this reference should

involve incrementing the module count -- which would prevent the module

from being unloaded in the first place.
Alan Stern
-

On Wed, 28 Nov 2007 14:18:18 -0500 (EST),
This still leaves the possibility that random code may grab a reference

once the kobject is present in the tree and lookupable. You gave up the

control of the number of references to your object once you made it

public.

-

I don't agree with this argument.  Code should never grab random

references without insuring that the owner of the referenced object is

pinned.  This rule applies to everything, not just kobjects.
Unfortunately kobjects don't have an owner field.  In practice this

means that it isn't possible to pin the owner of some random kobject --

you have to know where the kobject came from or what it's embedded in.

All users of kobjects need to work this way.
Alan Stern
-

On Thu, 29 Nov 2007 10:47:19 -0500 (EST),
Of course. You don't necessarily need prevent module unloading, but pin
Yeah, that is what the-patchset-I-have-to-dig-around-for does:

Introduce an owner field in the kobject for pinning the module. (IIRC,

you even did some of the patches?)

-

Are these two patches what you mean?  They are a little out-of-date

now, but the main idea hasn't changed.
Alan Stern
Index: usb-2.6/include/linux/kobject.h

===================================================================

--- usb-2.6.orig/include/linux/kobject.h

+++ usb-2.6/include/linux/kobject.h

@@ -71,7 +71,8 @@ static inline const char * kobject_name(

 	return kobj->k_name;

 }
-extern void kobject_init(struct kobject *);

+extern void kobject_init_owner(struct kobject *, struct module *owner);

+#define kobject_init(kobj)	kobject_init_owner(kobj, THIS_MODULE)

 extern void kobject_cleanup(struct kobject *);
 extern int __must_check kobject_add(struct kobject *);

@@ -84,7 +85,9 @@ extern int __must_check kobject_shadow_r

 						const char *new_name);

 extern int __must_check kobject_move(struct kobject *, struct kobject *);
-extern int __must_check kobject_register(struct kobject *);

+extern int __must_check kobject_register_owner(struct kobject *,

+		struct module *owner);

+#define kobject_register(kobj)	kobject_register_owner(kobj, THIS_MODULE)

 extern void kobject_unregister(struct kobject *);
 extern struct kobject * kobject_get(struct kobject *);

Index: usb-2.6/lib/kobject.c

===================================================================

--- usb-2.6.orig/lib/kobject.c

+++ usb-2.6/lib/kobject.c

@@ -164,10 +164,11 @@ static void verify_dynamic_kobject_alloc

 #endif
 /**

- *	kobject_init - initialize object.

+ *	kobject_init_onwer - initialize object.

  *	@kobj:	object in question.

+ *	@owner: module owning @kobj.

  */

-void kobject_init(struct kobject * kobj)

+void kobject_init_owner(struct kobject * kobj, struct module *owner)

 {

 	if (!kobj)

 		return;

@@ -178,6 +179,7 @@ void kobject_init(struct kobject * kobj)

 	init_waitqueue_head(&kobj->poll);

 	kobj->kset = kset_get(kobj->kset);

 	/* Attempt to grab reference of owning module's kobject. */

+	kobj->owner = owner;

 	mod_kobject_get(kobj->owner);

 }
@@ ...
[ message continues ]

On Thu, 29 Nov 2007 11:55:43 -0500 (EST),
Yes, thanks. I had also a module-reference-count part that I need to

resurrect.

-

Yeah, we should require it. And kobject_cleanup() needs to be called to

free the allocated name, if the object is not already added and needs to

be deleted (common in rewinding on error).
Btw, do you have a good example for unregistered/unnamed kobjects?
Thanks,

Kay
-

On Wed, 28 Nov 2007 13:35:54 +0100,
Unfortunately not (I can only think of devices that might never be

registered).

-

Ok, so we can always require a kobject_put() after a kobject_init(), to
This is good, we should just get rid of that use case, it seems it

doesn't make any sense to use kobjects which will never be registered.
Thanks,

Kay
-

On Wed, 28 Nov 2007 17:03:07 +0100,
That's a sensible requirement, I think.

-

Yes, there are a few usages of unregistered kobjects in the kernel

today, but I think that was people just not realizing they could use a

simple kref instead.  I'm working on cleaning them up right now...
thanks,
greg k-h

-

Certainly I have no objections, I'm glad it was useful.
You don't keep this promise - bet you thought we wouldn't notice...

Actually I guess you do, in the "creating simple kobjects" section.

When you get to that point, you should mention that this is a situation

where standalone kobjects make sense.
Given that there are quite a few standalone kobjects created by this

patch set (kernel_kobj, security_kobj, s390_kobj, etc.), the "(even
Is setting the name mandatory now, or are there still places where

kobjects (which do not appear in sysfs) do have - and do not need - a
So just how should severely should we mock kobject maintainers who can't
Can we try that one again?
 - A kset can provide a set of default attributes for all kobjects which
Presumably the latter way is not recommended; I would either say so or

not mention this possibility at all.
jon

-

Sorry, yes, that is where I tried to explain it.  I'll flush it out some
Any kobject that is registered needs to have a name.  If someone tries

to call kobject_register() or kobject_add() without a name set they will

find out that it is not allowed :)
And yes, there are a few places in the kernel with kobjects that are
Ah, yes, now removed.
Thanks for the review, I really appreciate it.
greg k-h

-

^^^

Wow, that's impressive: both kobjects de-mystified and time travel made possible!
;-)

-

The future is now! ;-)
cheers,

	Kyle

-

/me goes off to recharge the flux generator...
Heh, thanks, I'll go fix that.
greg k-h

-

/*

 * Sample kset and ktype implementation

 *

 * Copyright (C) 2004-2007 Greg Kroah-Hartman <greg@kroah.com>

 * Copyright (C) 2007 Novell Inc.

 *

 * Released under the GPL version 2 only.

 *

 */

#include <linux/kobject.h>

#include <linux/string.h>

#include <linux/sysfs.h>

#include <linux/module.h>

#include <linux/init.h>
/*

 * This module shows how to create a kset in sysfs called

 * /sys/kernel/kset-example

 * Then tree kobjects are created and assigned to this kset, "foo", "baz",

 * and "bar".  In those kobjects, attributes of the same name are also

 * created and if an integer is written to these files, it can be later

 * read out of it.

 */
/*

 * This is our "object" that we will create a few of and register them with

 * sysfs.

 */

struct foo_obj {

	struct kobject kobj;

	int foo;

	int baz;

	int bar;

};

#define to_foo_obj(x) container_of(x, struct foo_obj, kobj)
/* a custom attribute that works just for a struct foo_obj. */

struct foo_attribute {

	struct attribute attr;

	ssize_t (*show)(struct foo_obj *foo, struct foo_attribute *attr, char *buf);

	ssize_t (*store)(struct foo_obj *foo, struct foo_attribute *attr, const char *buf, size_t count);

};

#define to_foo_attr(x) container_of(x, struct foo_attribute, attr)
/*

 * The default show function that must be passed to sysfs.  This will be

 * called by sysfs for whenever a show function is called by the user on a

 * sysfs file associated with the kobjects we have registered.  We need to

 * transpose back from a "default" kobject to our custom struct foo_obj and

 * then call the show function for that specific object.

 */

static ssize_t foo_attr_show(struct kobject *kobj,

			     struct attribute *attr,

			     char *buf)

{

	struct foo_attribute *attribute;

	struct foo_obj *foo;
	attribute = to_foo_attr(attr);

	foo = to_foo_obj(kobj);
	if (!attribute->show)

		return -EIO;
	return attribute->show(foo, attribute, buf);

}
/*

 * Just like the default...
[ message continues ]

On Tue, 27 Nov 2007 15:04:06 -0800,
kobject_put(foo) is needed since it gets you through kobject_cleanup()

-

No, kobject_register() should have handled that for us, right?
thanks,
greg k-h

-

On Wed, 28 Nov 2007 22:11:39 -0800,
kobject_register() doesn't do a kobject_put() if kobject_add() failed.

-

The rule is simple enough.  After calling kobject_register() you should

always use kobject_put() -- even if kobject_register() failed.
In fact, after calling kobject_init() you should use kobject_put().

The first rule follows from this one, since kobject_register() calls

kobject_init() internally.
Alan Stern
-

Yes, that makes sense, time to write it all down :)
thanks,
greg k-h

-

Hi,

The behavior is not very clear here, the root problem is that :
1. Should we call kobject_put so cleanup work can be done by refcount

touch zero or call kfree every time after kobject_register failed?
2. If kobject_put calling is true, should this be done in

kobject_register error handling codes or by hand after

kobject_register failed?
Regards

-

Call kobject_put so that the cleanup work will be done when the 
It should be done by hand after kobject_register fails.  Otherwise

kobject_register would end up dropping a reference that it never

acquired.
Alan Stern
-

IMO,I'd rather select kobject_put due to the kobj name should also be released.

After searching for kobject_register, I found one leaks as this issue in pktcdvd.
Signed-off-by: Dave Young <hidave.darkstar@gmail.com> 
---

drivers/block/pktcdvd.c |    4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)
diff -upr linux/drivers/block/pktcdvd.c linux.new/drivers/block/pktcdvd.c

--- linux/drivers/block/pktcdvd.c	2007-11-30 13:13:44.000000000 +0800

+++ linux.new/drivers/block/pktcdvd.c	2007-11-30 13:24:08.000000000 +0800

@@ -117,8 +117,10 @@ static struct pktcdvd_kobj* pkt_kobj_cre

 	p->kobj.parent = parent;

 	p->kobj.ktype = ktype;

 	p->pd = pd;

-	if (kobject_register(&p->kobj) != 0)

+	if (kobject_register(&p->kobj) != 0) {

+		kobject_put(&p->kobj);

 		return NULL;

+	}

 	return p;

 }

-

/*

 * Sample kobject implementation

 *

 * Copyright (C) 2004-2007 Greg Kroah-Hartman <greg@kroah.com>

 * Copyright (C) 2007 Novell Inc.

 *

 * Released under the GPL version 2 only.

 *

 */

#include <linux/kobject.h>

#include <linux/string.h>

#include <linux/sysfs.h>

#include <linux/module.h>

#include <linux/init.h>
/*

 * This module shows how to create a simple subdirectory in sysfs called

 * /sys/kernel/kobject-example  In that directory, 3 files are created:

 * "foo", "baz", and "bar".  If an integer is written to these files, it can be

 * later read out of it.

 */
static int foo;

static int baz;

static int bar;
/*

 * The "foo" file where a static variable is read from and written to.

 */

static ssize_t foo_show(struct kobject *kobj, struct kobj_attribute *attr,

			char *buf)

{

	return sprintf(buf, "%d\n", foo);

}
static ssize_t foo_store(struct kobject *kobj, struct kobj_attribute *attr,

			 const char *buf, size_t count)

{

	sscanf(buf, "%du", &foo);

	return count;

}
static struct kobj_attribute foo_attribute =

	__ATTR(foo, 0666, foo_show, foo_store);
/*

 * More complex function where we determine which varible is being accessed by

 * looking at the attribute for the "baz" and "bar" files.

 */

static ssize_t b_show(struct kobject *kobj, struct kobj_attribute *attr,

		      char *buf)

{

	int var;
	if (strcmp(attr->attr.name, "baz") == 0)

		var = baz;

	else

		var = bar;

	return sprintf(buf, "%d\n", var);

}
static ssize_t b_store(struct kobject *kobj, struct kobj_attribute *attr,

		       const char *buf, size_t count)

{

	int var;
	sscanf(buf, "%du", &var);

	if (strcmp(attr->attr.name, "baz") == 0)

		baz = var;

	else

		bar = var;

	return count;

}
static struct kobj_attribute baz_attribute =

	__ATTR(baz, 0666, b_show, b_store);

static struct kobj_attribute bar_attribute =

	__ATTR(bar, 0666, b_show, b_store);
/*

 * Create a group of attributes so that we can create and dest...
[ message continues ]