[PATCH] Overzealous permenant mark removed

[PATCH] Overzealous permenant mark removed

[Rusty Russell]

Linus, please apply.

Name: Modules without init functions don't need exit functions
Author: Rusty Russell
Status: Trivial

D: If modules don't use module_exit(), they cannot be unloaded.  This
D: safety mechanism should not apply for modules which don't use
D: module_init() (implying they have nothing to clean up anyway).

diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.52/kernel/module.c working-2.5.52-noexit/kernel/module.c
--- linux-2.5.52/kernel/module.c	Tue Dec 17 08:11:03 2002
+++ working-2.5.52-noexit/kernel/module.c	Mon Dec 23 11:26:36 2002
@@ -405,7 +405,8 @@ sys_delete_module(const char *name_user,
 		}
 	}
 
-	if (!mod->exit || mod->unsafe) {
+	/* If it has an init func, it must have an exit func to unload */
+	if ((mod->init && !mod->exit) || mod->unsafe) {
 		forced = try_force(flags);
 		if (!forced) {
 			/* This module can't be removed */
@@ -473,7 +474,7 @@ static void print_unload_info(struct seq
 	if (mod->unsafe)
 		seq_printf(m, " [unsafe]");
 
-	if (!mod->exit)
+	if (mod->init && !mod->exit)
 		seq_printf(m, " [permanent]");
 
 	seq_printf(m, "\n");

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Overzealous permenant mark removed

[Dave Jones]

On Fri, Dec 27, 2002 at 07:44:41PM +1100, Rusty Russell wrote:

 > Name: Modules without init functions don't need exit functions
 > D: If modules don't use module_exit(), they cannot be unloaded.  This
 > D: safety mechanism should not apply for modules which don't use
 > D: module_init() (implying they have nothing to clean up anyway).

Just a heads up, as this bit me with agpgart which had a module_init()
but no module_exit() and then found itself un-unloadable[1].
I don't know if agpgart found itself in the unique position of doing
this (It only really used its _init function to clear some vars,
and printk a banner), but its something to keep an eye out for.

I fixed it by adding a null _exit function, (which later did a
sanity check just for good measure).

		Dave

[1] Coo, a new silly word.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] Overzealous permenant mark removed

[Rusty Russell]

In message <20021230114043.GD11633@suse.de> you write:
> On Fri, Dec 27, 2002 at 07:44:41PM +1100, Rusty Russell wrote:
> 
>  > Name: Modules without init functions don't need exit functions
>  > D: If modules don't use module_exit(), they cannot be unloaded.  This
>  > D: safety mechanism should not apply for modules which don't use
>  > D: module_init() (implying they have nothing to clean up anyway).
> 
> Just a heads up, as this bit me with agpgart which had a module_init()
> but no module_exit() and then found itself un-unloadable[1].
> I don't know if agpgart found itself in the unique position of doing
> this (It only really used its _init function to clear some vars,
> and printk a banner), but its something to keep an eye out for.

The banner, well, you know Linus' position on printing banners 8)

The rest seems superfluous:

int __init agp_init(void)
{
	static int already_initialised=0;

	if (already_initialised!=0)
		return 0;

	already_initialised = 1;

	memset(&agp_bridge, 0, sizeof(struct agp_bridge_data));
	agp_bridge.type = NOT_SUPPORTED;

	printk(KERN_INFO "Linux agpgart interface v%d.%d (c) Dave Jones\n",
	       AGPGART_VERSION_MAJOR, AGPGART_VERSION_MINOR);
	return 0;
}

Hope that helps,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] Overzealous permenant mark removed

[Linus Torvalds]

On Tue, 31 Dec 2002, Rusty Russell wrote:
> 
> The banner, well, you know Linus' position on printing banners 8)

Well, I don't much enjoy banners, but on the other hand I enjoy nasty
little made-up rules that make people jump through hoops to do what they
really want to do even less. 

I don't really think that having no exit function should mean that you
can't unload it ("unless you have no init function, in which case it's ok,
unless it's the third thursday in a month that starts with the letter
'M', in which case you can only unload it if you walk widdershins around 
the computer thrice").

In other words, the rule really should be: "if the usage count > 1, you
can't unload it".

And if somebody wants to create an un-unloadable driver, he should just 
increment the module count and be done with it. No magic rules (maybe you 
can make /proc/modules print out "<permanent>" if the count is over some 
number, and then people who want permanent modules just initialize the 
count past that).

		Linus

Re: [PATCH] Overzealous permenant mark removed

[Rusty Russell]

In message <Pine.LNX.4.44.0212302313370.2043-100000@home.transmeta.com> you wri
te:
> And if somebody wants to create an un-unloadable driver, he should just 
> increment the module count and be done with it. No magic rules (maybe you 
> can make /proc/modules print out "<permanent>" if the count is over some 
> number, and then people who want permanent modules just initialize the 
> count past that).

OTOH, your approach left us with stuff that wasn't modular at all
(like IPv4) because people felt it was somehow "wrong" to make it
non-unloadable.

I guess it depends on numbers.  If we see lots of drivers which
initialize things and don't really need to clean up, you're right.

If we see far more "I didn't implement unloading" drivers, it's
easiest to do the safe thing: require the author DO SOMETHING to make
the module unloadable, not vice versa.

But the change is trivial,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.