Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Wed, 2007-04-18 at 11:20 -0400, Alan Stern wrote: > On Wed, 18 Apr 2007, Rusty Russell wrote: > > > Hi Alan, > > > > Your assertion is correct. I haven't studied the driver core, so I > > might be off-base here, but you'll note that if the module references > > the core kmalloc'ed object rather than the other way around it can be > > done safely. The core can also reference the module, but it must be > > able to live without it once it's gone (eg. by returning -ENOENT). > > "Live without it once it's gone..." Do you mean once the object is gone > or once the module is gone? The core in general has no way to know when > the module is gone; all it knows about is the object. The trouble arises > when the module is gone (whether the core knows it or not) but the object > is still present. Hi Alan, I meant that the module is gone: it has told the object (via unregister_xxx) that it's gone. > > A really poor example is below: ... > The example is fine as far as it goes, but it assumes that all > interactions with the underlying r->foo object can be done under a > spinlock. Of course this isn't true in general. There are certainly other ways of doing it, such as a mutex, a refcnt & completion (for function pointers), or disabling preemption across the access and using stop_machine(). Of course, these add complexity. This is the reason that I've always disliked module removal. We have a lot of code to deal with it and it has awkward semantics (unless --wait is used). OTOH, I'm not a fan of the network approach, either: I feel that bringing up an interface should bump the refcnt of the module which implements that interface. Currently taking out e1000 will just kill my eth0. Cheers, Rusty. > > Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Wed, 18 Apr 2007, Rusty Russell wrote:
> Hi Alan,
>
> Your assertion is correct. I haven't studied the driver core, so I
> might be off-base here, but you'll note that if the module references
> the core kmalloc'ed object rather than the other way around it can be
> done safely. The core can also reference the module, but it must be
> able to live without it once it's gone (eg. by returning -ENOENT).
"Live without it once it's gone..." Do you mean once the object is gone
or once the module is gone? The core in general has no way to know when
the module is gone; all it knows about is the object. The trouble arises
when the module is gone (whether the core knows it or not) but the object
is still present.
> A really poor example is below:
>
> int register_foo(struct foo *foo)
> {
> struct registered_foo *r = kmalloc(...);
> if (!r)
> return -ENOMEM;
> r->foo = foo;
> atomic_set(>refcnt, 1);
> spin_lock(_lock);
> list_add(, >list);
> spin_unlock(_lock);
> return 0;
> }
>
> void unregister_foo(struct foo *foo)
> {
> struct registered_foo *i, *found = NULL;
> spin_lock(_lock);
> list_for_each_entry(i, , list) {
> if (i->foo == foo) {
> i->foo = NULL;
> if (atomic_dec_and_test(>refcnt))
> kfree(i);
> break;
> }
> }
> spin_unlock(_lock);
> }
>
> int get_foo_value(struct registered_foo *r)
> {
> u32 val = -ENOENT;
> spin_lock(_lock);
> if (r->foo)
> val = r->foo->val;
> spin_unlock(_lock);
> return val;
> }
The example is fine as far as it goes, but it assumes that all
interactions with the underlying r->foo object can be done under a
spinlock. Of course this isn't true in general.
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Wed, 18 Apr 2007, Rusty Russell wrote:
Hi Alan,
Your assertion is correct. I haven't studied the driver core, so I
might be off-base here, but you'll note that if the module references
the core kmalloc'ed object rather than the other way around it can be
done safely. The core can also reference the module, but it must be
able to live without it once it's gone (eg. by returning -ENOENT).
Live without it once it's gone... Do you mean once the object is gone
or once the module is gone? The core in general has no way to know when
the module is gone; all it knows about is the object. The trouble arises
when the module is gone (whether the core knows it or not) but the object
is still present.
A really poor example is below:
int register_foo(struct foo *foo)
{
struct registered_foo *r = kmalloc(...);
if (!r)
return -ENOMEM;
r-foo = foo;
atomic_set(r-refcnt, 1);
spin_lock(foo_lock);
list_add(foos, r-list);
spin_unlock(foo_lock);
return 0;
}
void unregister_foo(struct foo *foo)
{
struct registered_foo *i, *found = NULL;
spin_lock(foo_lock);
list_for_each_entry(i, foos, list) {
if (i-foo == foo) {
i-foo = NULL;
if (atomic_dec_and_test(i-refcnt))
kfree(i);
break;
}
}
spin_unlock(foo_lock);
}
int get_foo_value(struct registered_foo *r)
{
u32 val = -ENOENT;
spin_lock(foo_lock);
if (r-foo)
val = r-foo-val;
spin_unlock(foo_lock);
return val;
}
The example is fine as far as it goes, but it assumes that all
interactions with the underlying r-foo object can be done under a
spinlock. Of course this isn't true in general.
Alan Stern
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Wed, 2007-04-18 at 11:20 -0400, Alan Stern wrote: On Wed, 18 Apr 2007, Rusty Russell wrote: Hi Alan, Your assertion is correct. I haven't studied the driver core, so I might be off-base here, but you'll note that if the module references the core kmalloc'ed object rather than the other way around it can be done safely. The core can also reference the module, but it must be able to live without it once it's gone (eg. by returning -ENOENT). Live without it once it's gone... Do you mean once the object is gone or once the module is gone? The core in general has no way to know when the module is gone; all it knows about is the object. The trouble arises when the module is gone (whether the core knows it or not) but the object is still present. Hi Alan, I meant that the module is gone: it has told the object (via unregister_xxx) that it's gone. A really poor example is below: ... The example is fine as far as it goes, but it assumes that all interactions with the underlying r-foo object can be done under a spinlock. Of course this isn't true in general. There are certainly other ways of doing it, such as a mutex, a refcnt completion (for function pointers), or disabling preemption across the access and using stop_machine(). Of course, these add complexity. This is the reason that I've always disliked module removal. We have a lot of code to deal with it and it has awkward semantics (unless --wait is used). OTOH, I'm not a fan of the network approach, either: I feel that bringing up an interface should bump the refcnt of the module which implements that interface. Currently taking out e1000 will just kill my eth0. Cheers, Rusty. Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Tue, 2007-04-17 at 12:08 -0400, Alan Stern wrote:
> More specifically, there _is_ no way in general to ensure that a reference
> will go away when the module's cleanup routine is called, unless you are
> very careful not to pass that reference on to _anybody_. The driver core
> certainly can't do this; it passes a device reference to anyone who
> creates a child of that device (the parent pointer) -- and it can't
> guarantee that every one of its clients will drop their references when
> their exit routines run.
Hi Alan,
Your assertion is correct. I haven't studied the driver core, so I
might be off-base here, but you'll note that if the module references
the core kmalloc'ed object rather than the other way around it can be
done safely. The core can also reference the module, but it must be
able to live without it once it's gone (eg. by returning -ENOENT).
A really poor example is below:
int register_foo(struct foo *foo)
{
struct registered_foo *r = kmalloc(...);
if (!r)
return -ENOMEM;
r->foo = foo;
atomic_set(>refcnt, 1);
spin_lock(_lock);
list_add(, >list);
spin_unlock(_lock);
return 0;
}
void unregister_foo(struct foo *foo)
{
struct registered_foo *i, *found = NULL;
spin_lock(_lock);
list_for_each_entry(i, , list) {
if (i->foo == foo) {
i->foo = NULL;
if (atomic_dec_and_test(>refcnt))
kfree(i);
break;
}
}
spin_unlock(_lock);
}
int get_foo_value(struct registered_foo *r)
{
u32 val = -ENOENT;
spin_lock(_lock);
if (r->foo)
val = r->foo->val;
spin_unlock(_lock);
return val;
}
> I'm not so sure I fully believe that network drivers can do what Rusty
> says, at least not in every circumstance. There simply are too many
> possible ways for references to linger on after you thought they were all
> gone. For example, what happens when the user has mounted a filesystem on
> a USB drive running via USB-over-TCP though a network interface?
That's actually fine, it's when packets linger in the system holding
references to the interface they came from that things get tricky.
Hence the networking code tries not to do that.
Cheers,
Rusty.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Tue, 17 Apr 2007, Cornelia Huck wrote: > On Tue, 17 Apr 2007 12:53:10 +1000, > Rusty Russell <[EMAIL PROTECTED]> wrote: > > > On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: > > > The fundamental rule is that whenever you hand out a pointer to a routine > > > living in a module, the receiver has to increment the module's refcount. > > > But the driver core violates this rule all over the place. > > > > Hi Alan, > > > > Your rule is overly simplistic, unfortunately. You have two choices: > > take a reference count, *or* ensure that the reference will go away when > > the module's cleanup routine is called. Network drivers are a classic > > example of the latter. > > Hm, but that's exactly the problem we face here. There is no race-free > way (at least, nobody could think about one yet) to make sure all > references are freed at exit time and simultaneously to make sure that > the release functions have finished before the module has been deleted. More specifically, there _is_ no way in general to ensure that a reference will go away when the module's cleanup routine is called, unless you are very careful not to pass that reference on to _anybody_. The driver core certainly can't do this; it passes a device reference to anyone who creates a child of that device (the parent pointer) -- and it can't guarantee that every one of its clients will drop their references when their exit routines run. I'm not so sure I fully believe that network drivers can do what Rusty says, at least not in every circumstance. There simply are too many possible ways for references to linger on after you thought they were all gone. For example, what happens when the user has mounted a filesystem on a USB drive running via USB-over-TCP though a network interface? Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Tue, 17 Apr 2007 12:53:10 +1000, Rusty Russell <[EMAIL PROTECTED]> wrote: > On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: > > The fundamental rule is that whenever you hand out a pointer to a routine > > living in a module, the receiver has to increment the module's refcount. > > But the driver core violates this rule all over the place. > > Hi Alan, > > Your rule is overly simplistic, unfortunately. You have two choices: > take a reference count, *or* ensure that the reference will go away when > the module's cleanup routine is called. Network drivers are a classic > example of the latter. Hm, but that's exactly the problem we face here. There is no race-free way (at least, nobody could think about one yet) to make sure all references are freed at exit time and simultaneously to make sure that the release functions have finished before the module has been deleted. > > Note that you cannot do both: if the cleanup routine calls something > which drops a reference count, it implies that the cleanup routine needs > to be called with non-zero reference count, and it won't be (ignoring > --force). That's where that second reference count (the kref in the embedded kobject in the module) comes into play, that doesn't prevent rmmod from being run. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Tue, 17 Apr 2007 12:53:10 +1000, Rusty Russell [EMAIL PROTECTED] wrote: On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: The fundamental rule is that whenever you hand out a pointer to a routine living in a module, the receiver has to increment the module's refcount. But the driver core violates this rule all over the place. Hi Alan, Your rule is overly simplistic, unfortunately. You have two choices: take a reference count, *or* ensure that the reference will go away when the module's cleanup routine is called. Network drivers are a classic example of the latter. Hm, but that's exactly the problem we face here. There is no race-free way (at least, nobody could think about one yet) to make sure all references are freed at exit time and simultaneously to make sure that the release functions have finished before the module has been deleted. Note that you cannot do both: if the cleanup routine calls something which drops a reference count, it implies that the cleanup routine needs to be called with non-zero reference count, and it won't be (ignoring --force). That's where that second reference count (the kref in the embedded kobject in the module) comes into play, that doesn't prevent rmmod from being run. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Tue, 17 Apr 2007, Cornelia Huck wrote: On Tue, 17 Apr 2007 12:53:10 +1000, Rusty Russell [EMAIL PROTECTED] wrote: On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: The fundamental rule is that whenever you hand out a pointer to a routine living in a module, the receiver has to increment the module's refcount. But the driver core violates this rule all over the place. Hi Alan, Your rule is overly simplistic, unfortunately. You have two choices: take a reference count, *or* ensure that the reference will go away when the module's cleanup routine is called. Network drivers are a classic example of the latter. Hm, but that's exactly the problem we face here. There is no race-free way (at least, nobody could think about one yet) to make sure all references are freed at exit time and simultaneously to make sure that the release functions have finished before the module has been deleted. More specifically, there _is_ no way in general to ensure that a reference will go away when the module's cleanup routine is called, unless you are very careful not to pass that reference on to _anybody_. The driver core certainly can't do this; it passes a device reference to anyone who creates a child of that device (the parent pointer) -- and it can't guarantee that every one of its clients will drop their references when their exit routines run. I'm not so sure I fully believe that network drivers can do what Rusty says, at least not in every circumstance. There simply are too many possible ways for references to linger on after you thought they were all gone. For example, what happens when the user has mounted a filesystem on a USB drive running via USB-over-TCP though a network interface? Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Tue, 2007-04-17 at 12:08 -0400, Alan Stern wrote:
More specifically, there _is_ no way in general to ensure that a reference
will go away when the module's cleanup routine is called, unless you are
very careful not to pass that reference on to _anybody_. The driver core
certainly can't do this; it passes a device reference to anyone who
creates a child of that device (the parent pointer) -- and it can't
guarantee that every one of its clients will drop their references when
their exit routines run.
Hi Alan,
Your assertion is correct. I haven't studied the driver core, so I
might be off-base here, but you'll note that if the module references
the core kmalloc'ed object rather than the other way around it can be
done safely. The core can also reference the module, but it must be
able to live without it once it's gone (eg. by returning -ENOENT).
A really poor example is below:
int register_foo(struct foo *foo)
{
struct registered_foo *r = kmalloc(...);
if (!r)
return -ENOMEM;
r-foo = foo;
atomic_set(r-refcnt, 1);
spin_lock(foo_lock);
list_add(foos, r-list);
spin_unlock(foo_lock);
return 0;
}
void unregister_foo(struct foo *foo)
{
struct registered_foo *i, *found = NULL;
spin_lock(foo_lock);
list_for_each_entry(i, foos, list) {
if (i-foo == foo) {
i-foo = NULL;
if (atomic_dec_and_test(i-refcnt))
kfree(i);
break;
}
}
spin_unlock(foo_lock);
}
int get_foo_value(struct registered_foo *r)
{
u32 val = -ENOENT;
spin_lock(foo_lock);
if (r-foo)
val = r-foo-val;
spin_unlock(foo_lock);
return val;
}
I'm not so sure I fully believe that network drivers can do what Rusty
says, at least not in every circumstance. There simply are too many
possible ways for references to linger on after you thought they were all
gone. For example, what happens when the user has mounted a filesystem on
a USB drive running via USB-over-TCP though a network interface?
That's actually fine, it's when packets linger in the system holding
references to the interface they came from that things get tricky.
Hence the networking code tries not to do that.
Cheers,
Rusty.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: > The fundamental rule is that whenever you hand out a pointer to a routine > living in a module, the receiver has to increment the module's refcount. > But the driver core violates this rule all over the place. Hi Alan, Your rule is overly simplistic, unfortunately. You have two choices: take a reference count, *or* ensure that the reference will go away when the module's cleanup routine is called. Network drivers are a classic example of the latter. Note that you cannot do both: if the cleanup routine calls something which drops a reference count, it implies that the cleanup routine needs to be called with non-zero reference count, and it won't be (ignoring --force). I hope that clarifies? Rusty. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Mon, 16 Apr 2007, Greg KH wrote: > On Mon, Apr 16, 2007 at 07:36:27PM +0200, Cornelia Huck wrote: > > Grab and release the module kobject refcount if kobj->owner is set. > > This prevents calls to the release function after the module has > > been unloaded. > > But doesn't this cause reference counts to be grabbed on modules that > don't want them? (like network drivers)? The refcounts we are talking about will not prevent modules from being unloaded. That is, they won't cause rmmod to fail -- they will simply cause it to wait until the refcount drops to 0. This should happen naturally as a result of the unregistration calls made by the module's exit routine. (That was the intention, anyway. I haven't yet seen all the parts of Cornelia's patch set, but that's how we discussed it.) > I don't understand what exactly you are trying to protect from here. > Especially as Tejun just ripped the lifetime rules between sysfs and > kobjects apart. There are other ways for userspace to acquire references to kernel objects. Think of procfs or debugfs, for example. Or opening a device file. Consider this example: A user has a pcmcia card containing a USB host controller. He plugs a USB flash drive into the card and mounts a filesystem stored on the flash drive. The mount call increments the module refcount for usb-storage (the owner of the block device containing the filesystem), but it doesn't do anything to the module refcount for the pcmcia driver (I forget its name -- yenta or something like that). So what happens if the pcmcia driver is unloaded while the filesystem is mounted? What happens when the filesystem is finally unmounted and a release method belonging to the pcmcia driver is called? The fundamental rule is that whenever you hand out a pointer to a routine living in a module, the receiver has to increment the module's refcount. But the driver core violates this rule all over the place. The most pervasive offender is struct device -- it contains a release() method pointer which often points into a module, but the driver core doesn't do anything like try_module_get() in device_initialize() or module_put() after calling dev->release(). Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
On Mon, Apr 16, 2007 at 07:36:27PM +0200, Cornelia Huck wrote: > Grab and release the module kobject refcount if kobj->owner is set. > This prevents calls to the release function after the module has > been unloaded. But doesn't this cause reference counts to be grabbed on modules that don't want them? (like network drivers)? I don't understand what exactly you are trying to protect from here. Especially as Tejun just ripped the lifetime rules between sysfs and kobjects apart. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[Patch -mm 3/3] RFC: Introduce kobject->owner for refcounting.
Grab and release the module kobject refcount if kobj->owner is set.
This prevents calls to the release function after the module has
been unloaded.
Signed-off-by: Cornelia Huck <[EMAIL PROTECTED]>
---
include/linux/kobject.h |1 +
lib/kobject.c |6 +-
2 files changed, 6 insertions(+), 1 deletion(-)
--- linux-2.6.orig/include/linux/kobject.h
+++ linux-2.6/include/linux/kobject.h
@@ -51,6 +51,7 @@ enum kobject_action {
};
struct kobject {
+ struct module * owner;
const char * k_name;
charname[KOBJ_NAME_LEN];
struct kref kref;
--- linux-2.6.orig/lib/kobject.c
+++ linux-2.6/lib/kobject.c
@@ -177,6 +177,8 @@ void kobject_init(struct kobject * kobj)
INIT_LIST_HEAD(>entry);
init_waitqueue_head(>poll);
kobj->kset = kset_get(kobj->kset);
+ /* Attempt to grab reference of owning module's kobject. */
+ mod_kobject_get(kobj->owner);
}
@@ -254,6 +256,7 @@ int kobject_shadow_add(struct kobject *
printk("kobject_add failed for %s (%d)\n",
kobject_name(kobj), error);
dump_stack();
+mod_kobject_put(kobj->owner);
}
return error;
@@ -491,6 +494,7 @@ void kobject_cleanup(struct kobject * ko
struct kobj_type * t = get_ktype(kobj);
struct kset * s = kobj->kset;
struct kobject * parent = kobj->parent;
+ struct module * owner = kobj->owner;
pr_debug("kobject %s: cleaning up\n",kobject_name(kobj));
if (kobj->k_name != kobj->name)
@@ -503,7 +507,7 @@ void kobject_cleanup(struct kobject * ko
"if this is not a directory kobject, it is broken "
"and must be fixed.\n",
kobj->name);
-
+ mod_kobject_put(owner);
if (s)
kset_put(s);
kobject_put(parent);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
Grab and release the module kobject refcount if kobj-owner is set.
This prevents calls to the release function after the module has
been unloaded.
Signed-off-by: Cornelia Huck [EMAIL PROTECTED]
---
include/linux/kobject.h |1 +
lib/kobject.c |6 +-
2 files changed, 6 insertions(+), 1 deletion(-)
--- linux-2.6.orig/include/linux/kobject.h
+++ linux-2.6/include/linux/kobject.h
@@ -51,6 +51,7 @@ enum kobject_action {
};
struct kobject {
+ struct module * owner;
const char * k_name;
charname[KOBJ_NAME_LEN];
struct kref kref;
--- linux-2.6.orig/lib/kobject.c
+++ linux-2.6/lib/kobject.c
@@ -177,6 +177,8 @@ void kobject_init(struct kobject * kobj)
INIT_LIST_HEAD(kobj-entry);
init_waitqueue_head(kobj-poll);
kobj-kset = kset_get(kobj-kset);
+ /* Attempt to grab reference of owning module's kobject. */
+ mod_kobject_get(kobj-owner);
}
@@ -254,6 +256,7 @@ int kobject_shadow_add(struct kobject *
printk(kobject_add failed for %s (%d)\n,
kobject_name(kobj), error);
dump_stack();
+mod_kobject_put(kobj-owner);
}
return error;
@@ -491,6 +494,7 @@ void kobject_cleanup(struct kobject * ko
struct kobj_type * t = get_ktype(kobj);
struct kset * s = kobj-kset;
struct kobject * parent = kobj-parent;
+ struct module * owner = kobj-owner;
pr_debug(kobject %s: cleaning up\n,kobject_name(kobj));
if (kobj-k_name != kobj-name)
@@ -503,7 +507,7 @@ void kobject_cleanup(struct kobject * ko
if this is not a directory kobject, it is broken
and must be fixed.\n,
kobj-name);
-
+ mod_kobject_put(owner);
if (s)
kset_put(s);
kobject_put(parent);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Mon, Apr 16, 2007 at 07:36:27PM +0200, Cornelia Huck wrote: Grab and release the module kobject refcount if kobj-owner is set. This prevents calls to the release function after the module has been unloaded. But doesn't this cause reference counts to be grabbed on modules that don't want them? (like network drivers)? I don't understand what exactly you are trying to protect from here. Especially as Tejun just ripped the lifetime rules between sysfs and kobjects apart. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Mon, 16 Apr 2007, Greg KH wrote: On Mon, Apr 16, 2007 at 07:36:27PM +0200, Cornelia Huck wrote: Grab and release the module kobject refcount if kobj-owner is set. This prevents calls to the release function after the module has been unloaded. But doesn't this cause reference counts to be grabbed on modules that don't want them? (like network drivers)? The refcounts we are talking about will not prevent modules from being unloaded. That is, they won't cause rmmod to fail -- they will simply cause it to wait until the refcount drops to 0. This should happen naturally as a result of the unregistration calls made by the module's exit routine. (That was the intention, anyway. I haven't yet seen all the parts of Cornelia's patch set, but that's how we discussed it.) I don't understand what exactly you are trying to protect from here. Especially as Tejun just ripped the lifetime rules between sysfs and kobjects apart. There are other ways for userspace to acquire references to kernel objects. Think of procfs or debugfs, for example. Or opening a device file. Consider this example: A user has a pcmcia card containing a USB host controller. He plugs a USB flash drive into the card and mounts a filesystem stored on the flash drive. The mount call increments the module refcount for usb-storage (the owner of the block device containing the filesystem), but it doesn't do anything to the module refcount for the pcmcia driver (I forget its name -- yenta or something like that). So what happens if the pcmcia driver is unloaded while the filesystem is mounted? What happens when the filesystem is finally unmounted and a release method belonging to the pcmcia driver is called? The fundamental rule is that whenever you hand out a pointer to a routine living in a module, the receiver has to increment the module's refcount. But the driver core violates this rule all over the place. The most pervasive offender is struct device -- it contains a release() method pointer which often points into a module, but the driver core doesn't do anything like try_module_get() in device_initialize() or module_put() after calling dev-release(). Alan Stern - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch -mm 3/3] RFC: Introduce kobject-owner for refcounting.
On Mon, 2007-04-16 at 15:53 -0400, Alan Stern wrote: The fundamental rule is that whenever you hand out a pointer to a routine living in a module, the receiver has to increment the module's refcount. But the driver core violates this rule all over the place. Hi Alan, Your rule is overly simplistic, unfortunately. You have two choices: take a reference count, *or* ensure that the reference will go away when the module's cleanup routine is called. Network drivers are a classic example of the latter. Note that you cannot do both: if the cleanup routine calls something which drops a reference count, it implies that the cleanup routine needs to be called with non-zero reference count, and it won't be (ignoring --force). I hope that clarifies? Rusty. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
