Re: [Patch -mm 0/3] RFC: module unloading vs. release function
On Mon, 16 Apr 2007 17:02:46 -0400 (EDT), Alan Stern <[EMAIL PROTECTED]> wrote: > > No, only the core module has to stay. For example, every time you > > register an input device you pin input.ko as it is the module that > > provides ->release() method for input devices. You can freely unload > > psmouse, or hid or your favorite joystick but input has to stay until > > last reference to the input device is dropped. serio and gameport work > > the same way. > > > > I think the requirement that one is not able to unload a core > > subsystem module untill all users are dropped off is ok - you can't > > unload it anyway until you unload all drivers that reference its > > exported functions and once you unload all the drivers data objects > > will drop off pretty quickly. > > The problem is that sometimes devices are owned by modules that aren't > core subsystem modules. Or the code using a device doesn't depend on the > device's owner module and therefore doesn't pin it. Yes. For example, zfcp creates some devices itself that don't have any relation to the ccw bus. (Other s390 drivers need a simple device; they use a generic device creation routine that uses a release function that is always built into s390 kernels.) - 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 0/3] RFC: module unloading vs. release function
On Mon, 16 Apr 2007 15:38:52 -0400 (EDT), Alan Stern <[EMAIL PROTECTED]> wrote: > On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > > Unfortunately all this "wait for refcount in module's exit" schemas > > lead to the following deadlock: > > > > rmmod my_module < /path/to/some/file/incrementing/my/refcount > > (Note that this problem will be a lot harder to provoke once Tejun's > changes to sysfs are in place. But it will still be possible, unless we > make similar changes to all the other filesystems as well.) > > There are three possible approaches to this problem: > > 1. Ignore it, as we do now. If someone actually tries running your > example above, an oops will result when the kobject's release > method is called after my_module has been unloaded from memory. > > 2. Do what Cornelia suggested, and allow the example to deadlock. > > 3. Change the module code so that rmmod can return _before_ the > module is actually unloaded from memory (but after the module's > exit routine has completed). This will lead to more problems. > For example, what if someone tries to modprobe my_module back > again before it has finished unloading? > > My feeling is that either a deadlock or more complications with modprobe > would be preferable to an oops. Your opinion may differ. My current preference is 2. (obviously :)). I don't like 3. too much (too complicated code), but I think it would still be better than 1. (And I agree, this will be harder to trigger with Tejun's patches.) > > (Also, doing this might be a good way to expose a lot of hidden > refcounting bugs. They will become very obvious when rmmod hangs.) Good point. - 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 0/3] RFC: module unloading vs. release function
On Tue, 2007-04-17 at 00:44 +0400, Alexey Dobriyan wrote: > On Mon, Apr 16, 2007 at 03:38:52PM -0400, Alan Stern wrote: > > 3. Change the module code so that rmmod can return _before_ the > > module is actually unloaded from memory (but after the module's > > exit routine has completed). This will lead to more problems. > > For example, what if someone tries to modprobe my_module back > > again before it has finished unloading? > > This problem (or its absence) must be already in tree: module_mutex is > dropped for the duration of ->exit() function, so init_module(2) could > load new old module meanwhile. Only if you give it a different name when loading it the second time. 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 0/3] RFC: module unloading vs. release function
On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > > > What about 4: > > > > > > When registering an [k]object increment refcount of module that > > > provides ->release() function. > > > > > > That would normally require ->release function to be placed on > > > subsystem level to allow unloading individual devices. That's what Cornelia is trying to do. For all release methods, not just those on the subsystem level (although that is where most of them sit). > > But that would also mean that a lot of modules that want to be able to > > be released whenever they want to today, not be allowed to (network > > drivers, etc.) > > > > No, only the core module has to stay. For example, every time you > register an input device you pin input.ko as it is the module that > provides ->release() method for input devices. You can freely unload > psmouse, or hid or your favorite joystick but input has to stay until > last reference to the input device is dropped. serio and gameport work > the same way. > > I think the requirement that one is not able to unload a core > subsystem module untill all users are dropped off is ok - you can't > unload it anyway until you unload all drivers that reference its > exported functions and once you unload all the drivers data objects > will drop off pretty quickly. The problem is that sometimes devices are owned by modules that aren't core subsystem modules. Or the code using a device doesn't depend on the device's owner module and therefore doesn't pin it. 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 0/3] RFC: module unloading vs. release function
On Mon, Apr 16, 2007 at 03:38:52PM -0400, Alan Stern wrote: > 3. Change the module code so that rmmod can return _before_ the > module is actually unloaded from memory (but after the module's > exit routine has completed). This will lead to more problems. > For example, what if someone tries to modprobe my_module back > again before it has finished unloading? This problem (or its absence) must be already in tree: module_mutex is dropped for the duration of ->exit() function, so init_module(2) could load new old module meanwhile. - 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 0/3] RFC: module unloading vs. release function
On 4/16/07, Greg KH <[EMAIL PROTECTED]> wrote: On Mon, Apr 16, 2007 at 03:03:16PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Greg KH <[EMAIL PROTECTED]> wrote: > >On Mon, Apr 16, 2007 at 02:30:17PM -0400, Dmitry Torokhov wrote: > >> On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >> >Hi, > >> > > >> >based on the discussion in "How should an exit routine wait for > >> >release() callbacks?", I've cooked up some patches that make module > >> >unload wait until the last reference for a kobject has been dropped. > >> >This should plug the "release function in already deleted module" race; > >> >however, if the last kobject_put() from the module containing the > >> >release function is not in the module's exit function, there's still a > >> >small window (not sure if and how to plug this). > >> > >> Unfortunately all this "wait for refcount in module's exit" schemas > >> lead to the following deadlock: > >> > >>rmmod my_module < /path/to/some/file/incrementing/my/refcount > > > >No, it should just return "module in use" as the reference count it > >grabbed before rmmod is called. > > > > No, because it it were module's refcount we woudl not have problem > with ->release() to begin with. It is object's refcount. Yes, but with these patches, we are incrementing that reference count when the kobject is created, which will cause this to fail. Then you will never be able to unload the module, not with the current module tools. > >But either way, that's just foolish to try to prevent that from failing > >:) > > Why? It works now for most of teh subsystems. That's because it is buggy :) Depends on the subsystem. -- Dmitry - 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 0/3] RFC: module unloading vs. release function
On 4/16/07, Greg KH <[EMAIL PROTECTED]> wrote: On Mon, Apr 16, 2007 at 03:47:13PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Alan Stern <[EMAIL PROTECTED]> wrote: > >On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > > > >> On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >> > Hi, > >> > > >> > based on the discussion in "How should an exit routine wait for > >> > release() callbacks?", I've cooked up some patches that make module > >> > unload wait until the last reference for a kobject has been dropped. > >> > This should plug the "release function in already deleted module" race; > >> > however, if the last kobject_put() from the module containing the > >> > release function is not in the module's exit function, there's still a > >> > small window (not sure if and how to plug this). > >> > >> Unfortunately all this "wait for refcount in module's exit" schemas > >> lead to the following deadlock: > >> > >> rmmod my_module < /path/to/some/file/incrementing/my/refcount > > > >(Note that this problem will be a lot harder to provoke once Tejun's > >changes to sysfs are in place. But it will still be possible, unless we > >make similar changes to all the other filesystems as well.) > > > >There are three possible approaches to this problem: > > > >1. Ignore it, as we do now. If someone actually tries running your > > example above, an oops will result when the kobject's release > > method is called after my_module has been unloaded from memory. > > > >2. Do what Cornelia suggested, and allow the example to deadlock. > > > >3. Change the module code so that rmmod can return _before_ the > > module is actually unloaded from memory (but after the module's > > exit routine has completed). This will lead to more problems. > > For example, what if someone tries to modprobe my_module back > > again before it has finished unloading? > > > >My feeling is that either a deadlock or more complications with modprobe > >would be preferable to an oops. Your opinion may differ. > > > > What about 4: > > When registering an [k]object increment refcount of module that > provides ->release() function. > > That would normally require ->release function to be placed on > subsystem level to allow unloading individual devices. But that would also mean that a lot of modules that want to be able to be released whenever they want to today, not be allowed to (network drivers, etc.) No, only the core module has to stay. For example, every time you register an input device you pin input.ko as it is the module that provides ->release() method for input devices. You can freely unload psmouse, or hid or your favorite joystick but input has to stay until last reference to the input device is dropped. serio and gameport work the same way. I think the requirement that one is not able to unload a core subsystem module untill all users are dropped off is ok - you can't unload it anyway until you unload all drivers that reference its exported functions and once you unload all the drivers data objects will drop off pretty quickly. -- Dmitry - 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 0/3] RFC: module unloading vs. release function
On Mon, Apr 16, 2007 at 03:03:16PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Greg KH <[EMAIL PROTECTED]> wrote: > >On Mon, Apr 16, 2007 at 02:30:17PM -0400, Dmitry Torokhov wrote: > >> On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >> >Hi, > >> > > >> >based on the discussion in "How should an exit routine wait for > >> >release() callbacks?", I've cooked up some patches that make module > >> >unload wait until the last reference for a kobject has been dropped. > >> >This should plug the "release function in already deleted module" race; > >> >however, if the last kobject_put() from the module containing the > >> >release function is not in the module's exit function, there's still a > >> >small window (not sure if and how to plug this). > >> > >> Unfortunately all this "wait for refcount in module's exit" schemas > >> lead to the following deadlock: > >> > >>rmmod my_module < /path/to/some/file/incrementing/my/refcount > > > >No, it should just return "module in use" as the reference count it > >grabbed before rmmod is called. > > > > No, because it it were module's refcount we woudl not have problem > with ->release() to begin with. It is object's refcount. Yes, but with these patches, we are incrementing that reference count when the kobject is created, which will cause this to fail. > >But either way, that's just foolish to try to prevent that from failing > >:) > > Why? It works now for most of teh subsystems. That's because it is buggy :) 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 0/3] RFC: module unloading vs. release function
On Mon, Apr 16, 2007 at 03:47:13PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Alan Stern <[EMAIL PROTECTED]> wrote: > >On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > > > >> On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >> > Hi, > >> > > >> > based on the discussion in "How should an exit routine wait for > >> > release() callbacks?", I've cooked up some patches that make module > >> > unload wait until the last reference for a kobject has been dropped. > >> > This should plug the "release function in already deleted module" race; > >> > however, if the last kobject_put() from the module containing the > >> > release function is not in the module's exit function, there's still a > >> > small window (not sure if and how to plug this). > >> > >> Unfortunately all this "wait for refcount in module's exit" schemas > >> lead to the following deadlock: > >> > >> rmmod my_module < /path/to/some/file/incrementing/my/refcount > > > >(Note that this problem will be a lot harder to provoke once Tejun's > >changes to sysfs are in place. But it will still be possible, unless we > >make similar changes to all the other filesystems as well.) > > > >There are three possible approaches to this problem: > > > >1. Ignore it, as we do now. If someone actually tries running your > > example above, an oops will result when the kobject's release > > method is called after my_module has been unloaded from memory. > > > >2. Do what Cornelia suggested, and allow the example to deadlock. > > > >3. Change the module code so that rmmod can return _before_ the > > module is actually unloaded from memory (but after the module's > > exit routine has completed). This will lead to more problems. > > For example, what if someone tries to modprobe my_module back > > again before it has finished unloading? > > > >My feeling is that either a deadlock or more complications with modprobe > >would be preferable to an oops. Your opinion may differ. > > > > What about 4: > > When registering an [k]object increment refcount of module that > provides ->release() function. > > That would normally require ->release function to be placed on > subsystem level to allow unloading individual devices. But that would also mean that a lot of modules that want to be able to be released whenever they want to today, not be allowed to (network drivers, etc.) 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 0/3] RFC: module unloading vs. release function
On 4/16/07, Alan Stern <[EMAIL PROTECTED]> wrote: On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > > Hi, > > > > based on the discussion in "How should an exit routine wait for > > release() callbacks?", I've cooked up some patches that make module > > unload wait until the last reference for a kobject has been dropped. > > This should plug the "release function in already deleted module" race; > > however, if the last kobject_put() from the module containing the > > release function is not in the module's exit function, there's still a > > small window (not sure if and how to plug this). > > Unfortunately all this "wait for refcount in module's exit" schemas > lead to the following deadlock: > > rmmod my_module < /path/to/some/file/incrementing/my/refcount (Note that this problem will be a lot harder to provoke once Tejun's changes to sysfs are in place. But it will still be possible, unless we make similar changes to all the other filesystems as well.) There are three possible approaches to this problem: 1. Ignore it, as we do now. If someone actually tries running your example above, an oops will result when the kobject's release method is called after my_module has been unloaded from memory. 2. Do what Cornelia suggested, and allow the example to deadlock. 3. Change the module code so that rmmod can return _before_ the module is actually unloaded from memory (but after the module's exit routine has completed). This will lead to more problems. For example, what if someone tries to modprobe my_module back again before it has finished unloading? My feeling is that either a deadlock or more complications with modprobe would be preferable to an oops. Your opinion may differ. What about 4: When registering an [k]object increment refcount of module that provides ->release() function. That would normally require ->release function to be placed on subsystem level to allow unloading individual devices. -- Dmitry - 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 0/3] RFC: module unloading vs. release function
On Mon, 16 Apr 2007, Dmitry Torokhov wrote: > On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > > Hi, > > > > based on the discussion in "How should an exit routine wait for > > release() callbacks?", I've cooked up some patches that make module > > unload wait until the last reference for a kobject has been dropped. > > This should plug the "release function in already deleted module" race; > > however, if the last kobject_put() from the module containing the > > release function is not in the module's exit function, there's still a > > small window (not sure if and how to plug this). > > Unfortunately all this "wait for refcount in module's exit" schemas > lead to the following deadlock: > > rmmod my_module < /path/to/some/file/incrementing/my/refcount (Note that this problem will be a lot harder to provoke once Tejun's changes to sysfs are in place. But it will still be possible, unless we make similar changes to all the other filesystems as well.) There are three possible approaches to this problem: 1. Ignore it, as we do now. If someone actually tries running your example above, an oops will result when the kobject's release method is called after my_module has been unloaded from memory. 2. Do what Cornelia suggested, and allow the example to deadlock. 3. Change the module code so that rmmod can return _before_ the module is actually unloaded from memory (but after the module's exit routine has completed). This will lead to more problems. For example, what if someone tries to modprobe my_module back again before it has finished unloading? My feeling is that either a deadlock or more complications with modprobe would be preferable to an oops. Your opinion may differ. (Also, doing this might be a good way to expose a lot of hidden refcounting bugs. They will become very obvious when rmmod hangs.) 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 0/3] RFC: module unloading vs. release function
On 4/16/07, Greg KH <[EMAIL PROTECTED]> wrote: On Mon, Apr 16, 2007 at 02:30:17PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >Hi, > > > >based on the discussion in "How should an exit routine wait for > >release() callbacks?", I've cooked up some patches that make module > >unload wait until the last reference for a kobject has been dropped. > >This should plug the "release function in already deleted module" race; > >however, if the last kobject_put() from the module containing the > >release function is not in the module's exit function, there's still a > >small window (not sure if and how to plug this). > > Unfortunately all this "wait for refcount in module's exit" schemas > lead to the following deadlock: > >rmmod my_module < /path/to/some/file/incrementing/my/refcount No, it should just return "module in use" as the reference count it grabbed before rmmod is called. No, because it it were module's refcount we woudl not have problem with ->release() to begin with. It is object's refcount. But either way, that's just foolish to try to prevent that from failing :) Why? It works now for most of teh subsystems. -- Dmitry - 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 0/3] RFC: module unloading vs. release function
On Mon, Apr 16, 2007 at 07:36:19PM +0200, Cornelia Huck wrote: > Hi, > > based on the discussion in "How should an exit routine wait for > release() callbacks?" Wait, why is this needed anymore with the recent work in splitting sysfs away from the backing code? 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 0/3] RFC: module unloading vs. release function
On Mon, Apr 16, 2007 at 02:30:17PM -0400, Dmitry Torokhov wrote: > On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: > >Hi, > > > >based on the discussion in "How should an exit routine wait for > >release() callbacks?", I've cooked up some patches that make module > >unload wait until the last reference for a kobject has been dropped. > >This should plug the "release function in already deleted module" race; > >however, if the last kobject_put() from the module containing the > >release function is not in the module's exit function, there's still a > >small window (not sure if and how to plug this). > > Unfortunately all this "wait for refcount in module's exit" schemas > lead to the following deadlock: > >rmmod my_module < /path/to/some/file/incrementing/my/refcount No, it should just return "module in use" as the reference count it grabbed before rmmod is called. But either way, that's just foolish to try to prevent that from failing :) 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 0/3] RFC: module unloading vs. release function
On 4/16/07, Cornelia Huck <[EMAIL PROTECTED]> wrote: Hi, based on the discussion in "How should an exit routine wait for release() callbacks?", I've cooked up some patches that make module unload wait until the last reference for a kobject has been dropped. This should plug the "release function in already deleted module" race; however, if the last kobject_put() from the module containing the release function is not in the module's exit function, there's still a small window (not sure if and how to plug this). Unfortunately all this "wait for refcount in module's exit" schemas lead to the following deadlock: rmmod my_module < /path/to/some/file/incrementing/my/refcount -- Dmitry - 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 0/3] RFC: module unloading vs. release function
Hi, based on the discussion in "How should an exit routine wait for release() callbacks?", I've cooked up some patches that make module unload wait until the last reference for a kobject has been dropped. This should plug the "release function in already deleted module" race; however, if the last kobject_put() from the module containing the release function is not in the module's exit function, there's still a small window (not sure if and how to plug this). This new refcounting still needs to be exploited (i. e. a driver actually setting kobject->owner). Whether we need to set the owner for kobject or if for device would be sufficient is also still open to debate. [1/3] Use module->mkobj even if !CONFIG_SYSFS. [2/3] Expose module->mkobj reference count. [3/3] Introduce kobject->owner for refcounting. Patchset is only slightly tested, but comments are welcome :) - 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/
