Re: [PATCH 1/4] module: implement module_inhibit_unload()

[Rusty Russell <rusty@rustcorp.com.au>]

On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
> Rusty Russell wrote:
> > As stated you cannot protect arbitrary code this way, as you are trying
> > to do.  I do not think you've broken any of the current code, but I
> > cannot tell.  You're certainly going to surprise unsuspecting future
> > authors.
> 
> Can you elaborate a bit?  Why can't it protect the code?

Because you don't know what that code does.  After all, it's assumed
that module code doesn't get called after exit and you're deliberately
violating that assumption.

> > Can you really not figure out the module owner of the sysfs entry to inc
> > its use count during this procedure?  (__module_get()).
> 
> I can but I don't think it's worth the effort.  It will involve passing
> @owner parameter down through kobject to sysfs but the path is pretty
> obscure and thus difficult to test.

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);
}

> 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.

Rusty.