[RFC] kobject_init changes

[RFC] kobject_init changes

[Greg KH]

New thread time...

Here are two patches, one that adds a two new functions,
kobject_init_ng() and kobject_init_and_add() and then a patch that shows
how a number of different users are converted over to these new
functions.

I choose the name "kobject_init_ng()" for now, so that I can do a patch
series that converts the differnet usages in the kernel tree, and then
do a global rename all at once, making it easier to find any potential
problems that might happen.

Same goes for the _and_add() function, which is really just
kobject_register() but without the uevent call, and it has proper memory
cleanup.

Any objections to these changes?

My plans with this are:
	- clean up the rest of the kernel tree usages of kobject_init()
	- rename kobject_init_ng() to kobject_init().
	- remove all users of kobject_register() and use the new
	  functions and a uevent call.
	- fix up the new kobject_create_and_register() function to be
	  create_and_add() instead.

The last one will require some patch reworking in my tree, but I have
four days of meetings next week, and two plane rides, so I'll need
something real to do :)

I suppose if I really get bored, I can fix up the cdev stuff, but that's
going to take a lot more work...

thanks,

greg k-h

[RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

This is what the kobject_init function is going to become.  Add it to
the kernel and then we can convert over the current kobject_init() users
before renaming it.

Also add a kobject_init_and_add function which bundles up what a lot of
the current callers want to do all at once, and it properly handles the
memory usages, unlike kobject_register();

Cc: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 include/linux/kobject.h |    8 ++++
 lib/kobject.c           |   91 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -79,6 +79,14 @@ static inline const char * kobject_name(
 }
 
 extern void kobject_init(struct kobject *);
+extern int __must_check kobject_init_ng(struct kobject *kobj,
+					struct kobj_type *ktype,
+					struct kobject *parent,
+					const char *fmt, ...);
+extern int __must_check kobject_init_and_add(struct kobject *kobj,
+					     struct kobj_type *ktype,
+					     struct kobject *parent,
+					     const char *fmt, ...);
 extern void kobject_cleanup(struct kobject *);
 
 extern int __must_check kobject_add(struct kobject *);
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -347,6 +347,97 @@ int kobject_set_name(struct kobject *kob
 }
 EXPORT_SYMBOL(kobject_set_name);
 
+/*
+ * kobject_init_varg - the main kobject init function
+ *
+ * The kobject is initialized here with the needed fields.  After this
+ * function returns successfully, the kobject can not be freed directly,
+ * kobject_put must be called in order to clean it up properly.  If this
+ * function returns an error, the memory can be safely freed directly.
+ */
+static int kobject_init_varg(struct kobject *kobj, struct kobj_type *ktype,
+			     struct kobject *parent, const char *fmt,
+			     va_list vargs)
+{
+	va_list aq;
+	int retval;
+
+	if ((!kobj) || (!ktype))
+		return -EINVAL;
+
+	WARN_ON(atomic_read(&kobj->kref.refcount));
+	kref_init(&kobj->kref);
+	INIT_LIST_HEAD(&kobj->entry);
+	kobj->ktype = ktype;
+	kobj->parent = parent;
+
+	va_copy(aq, vargs);
+	retval = kobject_set_name_vargs(kobj, fmt, aq);
+	va_end(aq);
+
+	return retval;
+}
+
+/**
+ * kobject_init_ng - initialize a kobject structure
+ * @kobj: pointer to the kobject to initialize
+ * @ktype: pointer to the ktype for this kobject.
+ * @parent: pointer to the parent of 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_ng(struct kobject *kobj, struct kobj_type *ktype,
+		    struct kobject *parent, const char *fmt, ...)
+{
+	va_list args;
+	int retval;
+
+	va_start(args, fmt);
+	retval = kobject_init_varg(kobj, ktype, parent, fmt, args);
+	va_end(args);
+
+	return retval;
+}
+EXPORT_SYMBOL(kobject_init_ng);
+
+/**
+ * kobject_init_and_add - initialize a kobject structure and add it to the kobject hierarchy
+ * @kobj: pointer to the kobject to initialize
+ * @ktype: pointer to the ktype for this kobject.
+ * @parent: pointer to the parent of this kobject.
+ * @fmt: the name of the kobject.
+ *
+ * This function will properly initialize a kobject and then call
+ * kobject_add().
+ *
+ * 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_and_add(struct kobject *kobj, struct kobj_type *ktype,
+			 struct kobject *parent, const char *fmt, ...)
+{
+	va_list args;
+	int retval;
+
+	va_start(args, fmt);
+	retval = kobject_init_varg(kobj, ktype, parent, fmt, args);
+	va_end(args);
+	if (retval)
+		return retval;
+
+	retval = kobject_add(kobj);
+	if (retval)
+		kobject_put(kobj);
+
+	return retval;
+}
+EXPORT_SYMBOL(kobject_init_and_add);
+
 /**
  *	kobject_rename - change the name of an object
  *	@kobj:	object in question.

[RFC] kobject: convert some users of kobject_init to the new functions.

[Greg KH]

This converts the kernel/ directory and the char_dev directory to use
the new kobject_init functions as an example and test of them.

Cc: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 fs/char_dev.c   |   18 ++++++++++++++----
 kernel/module.c |   12 ++++--------
 kernel/params.c |    5 +----
 kernel/user.c   |    5 +----
 mm/slub.c       |    5 +----
 5 files changed, 21 insertions(+), 24 deletions(-)

--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -509,10 +509,15 @@ static struct kobj_type ktype_cdev_dynam
 struct cdev *cdev_alloc(void)
 {
 	struct cdev *p = kzalloc(sizeof(struct cdev), GFP_KERNEL);
+	int ret;
+
 	if (p) {
-		p->kobj.ktype = &ktype_cdev_dynamic;
 		INIT_LIST_HEAD(&p->list);
-		kobject_init(&p->kobj);
+		ret = kobject_init_ng(&p->kobj, &ktype_cdev_dynamic, NULL,
+				      "cdev_dynamic");
+		if (ret)
+			printk(KERN_ERR "cdev: something went wrong "
+			       "initializing the dynamic kobject!\n");
 	}
 	return p;
 }
@@ -527,10 +532,15 @@ struct cdev *cdev_alloc(void)
  */
 void cdev_init(struct cdev *cdev, const struct file_operations *fops)
 {
+	int ret;
+
 	memset(cdev, 0, sizeof *cdev);
 	INIT_LIST_HEAD(&cdev->list);
-	cdev->kobj.ktype = &ktype_cdev_default;
-	kobject_init(&cdev->kobj);
+	ret = kobject_init_ng(&cdev->kobj, &ktype_cdev_default, NULL,
+			      "cdev_static");
+	if (ret)
+		printk(KERN_ERR "cdev: something went very wrong initializing "
+		       "the static kobject!\n");
 	cdev->ops = fops;
 }
 
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1218,18 +1218,14 @@ int mod_sysfs_init(struct module *mod)
 		err = -EINVAL;
 		goto out;
 	}
-	memset(&mod->mkobj.kobj, 0, sizeof(mod->mkobj.kobj));
-	err = kobject_set_name(&mod->mkobj.kobj, "%s", mod->name);
-	if (err)
-		goto out;
-	mod->mkobj.kobj.kset = module_kset;
-	mod->mkobj.kobj.ktype = &module_ktype;
 	mod->mkobj.mod = mod;
 
-	kobject_init(&mod->mkobj.kobj);
+	memset(&mod->mkobj.kobj, 0, sizeof(mod->mkobj.kobj));
+	mod->mkobj.kobj.kset = module_kset;
+	err = kobject_init_and_add(&mod->mkobj.kobj, &module_ktype, NULL,
+				   "%s", mod->name);
 
 	/* delay uevent until full sysfs population */
-	err = kobject_add(&mod->mkobj.kobj);
 out:
 	return err;
 }
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -561,10 +561,7 @@ static void __init kernel_param_sysfs_se
 
 	mk->mod = THIS_MODULE;
 	mk->kobj.kset = module_kset;
-	mk->kobj.ktype = &module_ktype;
-	kobject_set_name(&mk->kobj, name);
-	kobject_init(&mk->kobj);
-	ret = kobject_add(&mk->kobj);
+	ret = kobject_init_and_add(&mk->kobj, &module_ktype, NULL, "%s", name);
 	if (ret) {
 		printk(KERN_ERR "Module '%s' failed to be added to sysfs, "
 		      "error number %d\n", name, ret);
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -181,11 +181,8 @@ static int uids_user_create(struct user_
 	int error;
 
 	memset(kobj, 0, sizeof(struct kobject));
-	kobj->ktype = &uids_ktype;
 	kobj->kset = uids_kset;
-	kobject_init(kobj);
-	kobject_set_name(&up->kobj, "%d", up->uid);
-	error = kobject_add(kobj);
+	error = kobject_init_and_add(kobj, &uids_ktype, NULL, "%d", up->uid);
 	if (error)
 		goto done;
 
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4021,11 +4021,8 @@ static int sysfs_slab_add(struct kmem_ca
 		name = create_unique_id(s);
 	}
 
-	kobject_set_name(&s->kobj, name);
 	s->kobj.kset = slab_kset;
-	s->kobj.ktype = &slab_ktype;
-	kobject_init(&s->kobj);
-	err = kobject_add(&s->kobj);
+	err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, name);
 	if (err)
 		return err;
 

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Alan Stern]

On Fri, 30 Nov 2007, Greg KH wrote:

> +/**
> + * kobject_init_and_add - initialize a kobject structure and add it to the kobject hierarchy
> + * @kobj: pointer to the kobject to initialize
> + * @ktype: pointer to the ktype for this kobject.
> + * @parent: pointer to the parent of this kobject.
> + * @fmt: the name of the kobject.
> + *
> + * This function will properly initialize a kobject and then call
> + * kobject_add().
> + *
> + * 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_and_add(struct kobject *kobj, struct kobj_type *ktype,
> +			 struct kobject *parent, const char *fmt, ...)
> +{
> +	va_list args;
> +	int retval;
> +
> +	va_start(args, fmt);
> +	retval = kobject_init_varg(kobj, ktype, parent, fmt, args);
> +	va_end(args);
> +	if (retval)
> +		return retval;
> +
> +	retval = kobject_add(kobj);
> +	if (retval)
> +		kobject_put(kobj);

No, no!

You have recreated the problem we have been discussing during the last
couple of days.  If the kobject_init_varg() routine gets an error then
the kobject will need to be deallocated manually.  If the kobject_add()
routine gets an error then the cleanup invoked by kobject_put() will do
the deallocation automatically.

But the caller can't tell in which subroutine an error occurred, so it
won't know what to do when kobject_init_and_add() returns an error.

The only way to resolve this problem is to have the _init routine 
consume no resources and never fail.  That way the only possible 
failure mode would be if the _add routine doesn't work, in which case 
either a kfree() or a kobject_put() would be acceptable.

In particular, this implies that the name should be set as part of the 
_add() call, not as part of _init().  This is more in line with the way 
the code tends to use kobjects anyhow.  Unless people want to name 
unregistered kobjects -- does this ever happen?  And it if does, can 
these kobjects simply be replaced by krefs?

My suggestion: Have kobject_init_ng() accept a ktype pointer but not a 
parent or name.  Instead, make kobject_add_ng() take the parent and 
name (possibly a kset also).  Then when kobject_init_and_add() 
encounters an error, it shouldn't do a _put() -- the caller can either 
do the _put() or just do a kfree().

Alan Stern

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 03:25:52PM -0500, Alan Stern wrote:
> On Fri, 30 Nov 2007, Greg KH wrote:
> 
> > +/**
> > + * kobject_init_and_add - initialize a kobject structure and add it to the kobject hierarchy
> > + * @kobj: pointer to the kobject to initialize
> > + * @ktype: pointer to the ktype for this kobject.
> > + * @parent: pointer to the parent of this kobject.
> > + * @fmt: the name of the kobject.
> > + *
> > + * This function will properly initialize a kobject and then call
> > + * kobject_add().
> > + *
> > + * 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_and_add(struct kobject *kobj, struct kobj_type *ktype,
> > +			 struct kobject *parent, const char *fmt, ...)
> > +{
> > +	va_list args;
> > +	int retval;
> > +
> > +	va_start(args, fmt);
> > +	retval = kobject_init_varg(kobj, ktype, parent, fmt, args);
> > +	va_end(args);
> > +	if (retval)
> > +		return retval;
> > +
> > +	retval = kobject_add(kobj);
> > +	if (retval)
> > +		kobject_put(kobj);
> 
> No, no!
> 
> You have recreated the problem we have been discussing during the last
> couple of days.  If the kobject_init_varg() routine gets an error then
> the kobject will need to be deallocated manually.  If the kobject_add()
> routine gets an error then the cleanup invoked by kobject_put() will do
> the deallocation automatically.
> 
> But the caller can't tell in which subroutine an error occurred, so it
> won't know what to do when kobject_init_and_add() returns an error.

Oh crap.  You're totally right.  I suck.

> The only way to resolve this problem is to have the _init routine 
> consume no resources and never fail.  That way the only possible 
> failure mode would be if the _add routine doesn't work, in which case 
> either a kfree() or a kobject_put() would be acceptable.
> 
> In particular, this implies that the name should be set as part of the 
> _add() call, not as part of _init().  This is more in line with the way 
> the code tends to use kobjects anyhow.  Unless people want to name 
> unregistered kobjects -- does this ever happen?  And it if does, can 
> these kobjects simply be replaced by krefs?

No, the only non-registered kobjects in the tree right now are never
named.  So this should be safe.

> My suggestion: Have kobject_init_ng() accept a ktype pointer but not a 
> parent or name.  Instead, make kobject_add_ng() take the parent and 
> name (possibly a kset also).  Then when kobject_init_and_add() 
> encounters an error, it shouldn't do a _put() -- the caller can either 
> do the _put() or just do a kfree().

Why not the parent for init()?  Isn't it always known at that time?
I'll dig to be sure.

Ok, second round of patches coming up...

thanks,

greg k-h

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 01:04:20PM -0800, Greg KH wrote:
> > My suggestion: Have kobject_init_ng() accept a ktype pointer but not a 
> > parent or name.  Instead, make kobject_add_ng() take the parent and 
> > name (possibly a kset also).  Then when kobject_init_and_add() 
> > encounters an error, it shouldn't do a _put() -- the caller can either 
> > do the _put() or just do a kfree().
> 
> Why not the parent for init()?  Isn't it always known at that time?
> I'll dig to be sure.

You are right again, to match up with the driver core methods, we don't
know the parent until add() time.

thanks,

greg k-h

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Alan Stern]

On Fri, 30 Nov 2007, Greg KH wrote:

> > My suggestion: Have kobject_init_ng() accept a ktype pointer but not a 
> > parent or name.  Instead, make kobject_add_ng() take the parent and 
> > name (possibly a kset also).  Then when kobject_init_and_add() 
> > encounters an error, it shouldn't do a _put() -- the caller can either 
> > do the _put() or just do a kfree().
> 
> Why not the parent for init()?  Isn't it always known at that time?
> I'll dig to be sure.

Specifying the parent during _add() is more logical, because a kobject
doesn't actually _do_ anything to the parent until it is registered in
the parent's directory.  Or to put it another way, an unregistered
kobject can't have a parent in any meaningful sense so there's no point
specifying the parent in the _init() call.

It's really just a matter of taste.

Alan Stern

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 04:19:53PM -0500, Alan Stern wrote:
> On Fri, 30 Nov 2007, Greg KH wrote:
> 
> > > My suggestion: Have kobject_init_ng() accept a ktype pointer but not a 
> > > parent or name.  Instead, make kobject_add_ng() take the parent and 
> > > name (possibly a kset also).  Then when kobject_init_and_add() 
> > > encounters an error, it shouldn't do a _put() -- the caller can either 
> > > do the _put() or just do a kfree().
> > 
> > Why not the parent for init()?  Isn't it always known at that time?
> > I'll dig to be sure.
> 
> Specifying the parent during _add() is more logical, because a kobject
> doesn't actually _do_ anything to the parent until it is registered in
> the parent's directory.  Or to put it another way, an unregistered
> kobject can't have a parent in any meaningful sense so there's no point
> specifying the parent in the _init() call.

Ok, how about this:
	void kobject_init(struct kobject *kobj, struct ktype *ktype);

and then:
	int kobject_add(struct kobject *kobj, struct kobject *parent, const char *fmt, ...);

After we call kobject_init() we HAVE to call kobject_put() to clean up
properly.  So, if kobject_add() fails, we still need to clean up with
kobject_put();

That means we _can_ create a:
	int kobject_init_and_add(struct kobject *kobj, struct ktype *ktype, struct kobject *parent, const char *fmt, ...);

and if that fails, then again, you have to call kobject_put() to clean
things up, right?

Does this look sane?

thanks,

greg k-h

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Alan Stern]

On Fri, 30 Nov 2007, Greg KH wrote:

> Ok, how about this:
> 	void kobject_init(struct kobject *kobj, struct ktype *ktype);
> 
> and then:
> 	int kobject_add(struct kobject *kobj, struct kobject *parent, const char *fmt, ...);
> 
> After we call kobject_init() we HAVE to call kobject_put() to clean up
> properly.  So, if kobject_add() fails, we still need to clean up with
> kobject_put();

You could put that a little less strongly.  After kobject_init() you
SHOULD call kobject_put() to clean up properly, and after kobject_add()
you MUST call kobject_del() and kobject_put().

However if kobject_add() is never called, or if it is called and it 
fails, then it's okay to use kfree().  It's not clear whether this 
distinction will matter in practice.  It's probably best to document 
this using your stronger description.

The same sort of rule should apply to other kernel objects, like struct 
device.  After intialization you have to do a final _put, before that 
you just do a kfree().  (And initialization cannot fail.)

> That means we _can_ create a:
> 	int kobject_init_and_add(struct kobject *kobj, struct ktype *ktype, struct kobject *parent, const char *fmt, ...);
> 
> and if that fails, then again, you have to call kobject_put() to clean
> things up, right?

Right.  Because you know that the failure must have occurred in the 
_add portion.

> Does this look sane?

Yes, I think that will all work correctly.

Alan Stern

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 05:10:33PM -0500, Alan Stern wrote:
> On Fri, 30 Nov 2007, Greg KH wrote:
> 
> > Ok, how about this:
> > 	void kobject_init(struct kobject *kobj, struct ktype *ktype);
> > 
> > and then:
> > 	int kobject_add(struct kobject *kobj, struct kobject *parent, const char *fmt, ...);
> > 
> > After we call kobject_init() we HAVE to call kobject_put() to clean up
> > properly.  So, if kobject_add() fails, we still need to clean up with
> > kobject_put();
> 
> You could put that a little less strongly.  After kobject_init() you
> SHOULD call kobject_put() to clean up properly, and after kobject_add()
> you MUST call kobject_del() and kobject_put().
> 
> However if kobject_add() is never called, or if it is called and it 
> fails, then it's okay to use kfree().  It's not clear whether this 
> distinction will matter in practice.  It's probably best to document 
> this using your stronger description.

No, if kobject_add() fails, kobject_put() still must be called in order
to free up the name pointer, unless you are somehow guessing that the
"kobject_set_name()" portion of kobject_add() somehow failed.  And you
can't know that, so you have to call kobject_put() in order to be safe
and clean up everything.

Now why did we not do the final kobject_put() in kobject_del() as well?
Doing two calls, always in order, seems a bit strange, anyone know why
it's this way?

> The same sort of rule should apply to other kernel objects, like struct 
> device.  After intialization you have to do a final _put, before that 
> you just do a kfree().  (And initialization cannot fail.)

Yes.

> > That means we _can_ create a:
> > 	int kobject_init_and_add(struct kobject *kobj, struct ktype *ktype, struct kobject *parent, const char *fmt, ...);
> > 
> > and if that fails, then again, you have to call kobject_put() to clean
> > things up, right?
> 
> Right.  Because you know that the failure must have occurred in the 
> _add portion.

Ok, good, I might get this right yet :)

thanks,

greg k-h

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 05:10:33PM -0500, Alan Stern wrote:
> On Fri, 30 Nov 2007, Greg KH wrote:
> 
> > Ok, how about this:
> > 	void kobject_init(struct kobject *kobj, struct ktype *ktype);
> > 
> > and then:
> > 	int kobject_add(struct kobject *kobj, struct kobject *parent, const char *fmt, ...);
> > 
> > After we call kobject_init() we HAVE to call kobject_put() to clean up
> > properly.  So, if kobject_add() fails, we still need to clean up with
> > kobject_put();
> 
> You could put that a little less strongly.  After kobject_init() you
> SHOULD call kobject_put() to clean up properly, and after kobject_add()
> you MUST call kobject_del() and kobject_put().

No, in looking at the code, you only need to call kobject_del() to clean
everything up properly, if kobject_add() succeeds.  No need to call
kobject_put() yet again.

Can someone else verify that this really is correct?

thanks,

greg k-h

p.s. I think it's time to start a "travel to .nz and kick a certain
ex-kernel-developer around a bit" fund...

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Alan Stern]

On Fri, 30 Nov 2007, Greg KH wrote:

> > However if kobject_add() is never called, or if it is called and it 
> > fails, then it's okay to use kfree().  It's not clear whether this 
> > distinction will matter in practice.  It's probably best to document 
> > this using your stronger description.
> 
> No, if kobject_add() fails, kobject_put() still must be called in order
> to free up the name pointer, unless you are somehow guessing that the
> "kobject_set_name()" portion of kobject_add() somehow failed.

Actually I imagined that if kobject_add() failed it would back out all 
the changes it made -- which means it would deallocate the name 
string.  But requiring people to call kobject_put() will do this just 
as well.

>  And you
> can't know that, so you have to call kobject_put() in order to be safe
> and clean up everything.
> 
> Now why did we not do the final kobject_put() in kobject_del() as well?
> Doing two calls, always in order, seems a bit strange, anyone know why
> it's this way?

To be symmetrical with kobject_init() and kobject_add().  Besides, 
isn't there kobject_unregister()?  Presumably it will go away along 
with kobject_register(), though.

> > You could put that a little less strongly.  After kobject_init() you
> > SHOULD call kobject_put() to clean up properly, and after kobject_add()
> > you MUST call kobject_del() and kobject_put().
>
> No, in looking at the code, you only need to call kobject_del() to clean
> everything up properly, if kobject_add() succeeds.  No need to call
> kobject_put() yet again.

Sorry, yes, that's what I meant.  After a successful call to 
kobject_add() you must call kobject_del() to undo the _add, and then
kobject_put() for the final cleanup.

Alan Stern

Re: [RFC] kobject: add kobject_init_ng and kobject_init_and_add functions

[Greg KH]

On Fri, Nov 30, 2007 at 06:22:37PM -0500, Alan Stern wrote:
> On Fri, 30 Nov 2007, Greg KH wrote:
> >  And you
> > can't know that, so you have to call kobject_put() in order to be safe
> > and clean up everything.
> > 
> > Now why did we not do the final kobject_put() in kobject_del() as well?
> > Doing two calls, always in order, seems a bit strange, anyone know why
> > it's this way?
> 
> To be symmetrical with kobject_init() and kobject_add().  Besides, 
> isn't there kobject_unregister()?  Presumably it will go away along 
> with kobject_register(), though.

Yes, it will go away too, once everyone gets converted.

> > > You could put that a little less strongly.  After kobject_init() you
> > > SHOULD call kobject_put() to clean up properly, and after kobject_add()
> > > you MUST call kobject_del() and kobject_put().
> >
> > No, in looking at the code, you only need to call kobject_del() to clean
> > everything up properly, if kobject_add() succeeds.  No need to call
> > kobject_put() yet again.
> 
> Sorry, yes, that's what I meant.  After a successful call to 
> kobject_add() you must call kobject_del() to undo the _add, and then
> kobject_put() for the final cleanup.

No, kobject_del() does the put for you[1].  All that is needed is a call
to kobject_del().

I'll post the updated patches in a minute, they look and seem to work
much better.

thanks,

greg k-h