Messages in this thread | | | Date | Tue, 25 Sep 2007 10:01:09 +0200 | From | Cornelia Huck <> | Subject | Re: [PATCH 1/4] module: implement module_inhibit_unload() |
| |
On Tue, 25 Sep 2007 14:38:38 +1000, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Have you tested that *this* path works? Let's take your first change as > an example: > > + mutex_lock(&gdev->reg_mutex); > + __ccwgroup_remove_symlinks(gdev); > + device_unregister(dev); > + mutex_unlock(&gdev->reg_mutex); > > Now, are you sure that calling cleanup_ccwgroup just after > device_unregister() works? > > static void __exit > cleanup_ccwgroup (void) > { > bus_unregister (&ccwgroup_bus_type); > } >
ccwgroup is a bit special. The ccwgroup drivers (say, qeth) will unregister their ccwgroup_driver in their exit function. ccwgroup will then unregister all devices bound to this driver (a ccwgroup device without a driver does not make sense, since they are artifically created by writing to a 'group' attribute provided by the driver). This makes sure that ccwgroup cannot be unloaded while there are still devices on the bus, so your example won't hit.
> > I think it's too much work for the > > users of the API and it will be easy to pass the wrong @owner and go > > unnoticed. > > But your shortcut insists that all module authors be aware that > functions can be running after exit() is called. That's a recipe for > instability and disaster.
There are already problems like this with ->release().
<And no, I still haven't gotten around to testing and reviewing all those patchsets, sorry> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |