Skip to contentSkip to conversation

Bump module ref during init.

PATCH In review Rusty Russell 2 participants 3 messages May 9, 2003 – May 12, 2003

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Bump module ref during init.

At the moment, __module_get is technically allowed during module init, because the module is being held in by its non-live state. But doing so triggers the BUG(), so bump the refcount during init.

Also cleans up unload code a little, making it clearer.

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

Name: Hold Initial Reference To Module
Author: Rusty Russell
Status: Tested on 2.5.69-bk3

D: __module_get is theoretically allowed on module inside init, since
D: we already hold an implicit reference. Currently this BUG()s: make
D: the reference count explicit, which also simplifies delete path.
D: Also cleans up unload path, such that it only drops semaphore when
D: it's actually sleeping for rmmod --wait.

kernel/module.c +31 -34
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .7932-linux-2.5.69/kernel/module.c .7932-linux-2.5.69.updated/kernel/module.c
@@ -214,6 +214,8 @@ static void module_unload_init(struct mo
 	INIT_LIST_HEAD(&mod->modules_which_use_me);
 	for (i = 0; i < NR_CPUS; i++)
 		atomic_set(&mod->ref[i].count, 0);
+	/* Hold reference count during initialization. */
+	atomic_set(&mod->ref[smp_processor_id()].count, 1);
 	/* Backwards compatibility macros put refcount during init. */
 	mod->waiter = current;
 }
@@ -462,6 +464,21 @@ void cleanup_module(void)
 }
 EXPORT_SYMBOL(cleanup_module);
 
+static void wait_for_zero_refcount(struct module *mod)
+{
+	/* Since we might sleep for some time, drop the semaphore first */
+	up(&module_mutex);
+	for (;;) {
+		DEBUGP("Looking at refcount...\n");
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		if (module_refcount(mod) == 0)
+			break;
+		schedule();
+	}
+	current->state = TASK_RUNNING;
+	down(&module_mutex);
+}
+
 asmlinkage long
 sys_delete_module(const char __user *name_user, unsigned int flags)
 {
@@ -500,16 +517,6 @@ sys_delete_module(const char __user *nam
 		goto out;
 	}
 
-	/* Coming up?  Allow force on stuck modules. */
-	if (mod->state == MODULE_STATE_COMING) {
-		forced = try_force(flags);
-		if (!forced) {
-			/* This module can't be removed */
-			ret = -EBUSY;
-			goto out;
-		}
-	}
-
 	/* If it has an init func, it must have an exit func to unload */
 	if ((mod->init != init_module && mod->exit == cleanup_module)
 	    || mod->unsafe) {
@@ -529,35 +536,22 @@ sys_delete_module(const char __user *nam
 	/* If it's not unused, quit unless we are told to block. */
 	if ((flags & O_NONBLOCK) && module_refcount(mod) != 0) {
 		forced = try_force(flags);
-		if (!forced)
+		if (!forced) {
 			ret = -EWOULDBLOCK;
-	} else {
-		mod->waiter = current;
-		mod->state = MODULE_STATE_GOING;
+			restart_refcounts();
+			goto out;
+		}
 	}
-	restart_refcounts();
-
-	if (ret != 0)
-		goto out;
-
-	if (forced)
-		goto destroy;
 
-	/* Since we might sleep for some time, drop the semaphore first */
-	up(&module_mutex);
-	for (;;) {
-		DEBUGP("Looking at refcount...\n");
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (module_refcount(mod) == 0)
-			break;
-		schedule();
-	}
-	current->state = TASK_RUNNING;
+	/* Mark it as dying. */
+	mod->waiter = current;
+	mod->state = MODULE_STATE_GOING;
+	restart_refcounts();
 
-	DEBUGP("Regrabbing mutex...\n");
-	down(&module_mutex);
+	/* Never wait if forced. */
+	if (!forced && module_refcount(mod) != 0)
+		wait_for_zero_refcount(mod);
 
- destroy:
 	/* Final destruction now noone is using it. */
 	mod->exit();
 	free_module(mod);
@@ -1448,6 +1442,7 @@ sys_init_module(void __user *umod,
 			printk(KERN_ERR "%s: module is now stuck!\n",
 			       mod->name);
 		else {
+			module_put(mod);
 			down(&module_mutex);
 			free_module(mod);
 			up(&module_mutex);
@@ -1458,6 +1453,8 @@ sys_init_module(void __user *umod,
 	/* Now it's a first class citizen! */
 	down(&module_mutex);
 	mod->state = MODULE_STATE_LIVE;
+	/* Drop initial reference. */
+	module_put(mod);
 	module_free(mod, mod->module_init);
 	mod->module_init = NULL;
 	mod->init_size = 0;
#1 lore raw

kernel/module.c void cleanup_module(void) · line 476

Quoted 11 lines · Rusty Russell
kernel/module.c +10 -0
+static void wait_for_zero_refcount(struct module *mod)
+{
+	/* Since we might sleep for some time, drop the semaphore first */
+	up(&module_mutex);
+	for (;;) {
+		DEBUGP("Looking at refcount...\n");
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		if (module_refcount(mod) == 0)
+			break;
+		schedule();

What wakes the task up again?

#2 lore raw

In message <[email protected]> you write:

Quoted 5 lines
Quoted 2 lines · Rusty Russell

+static void wait_for_zero_refcount(struct module *mod)

What wakes the task up again?

module_put(), as before.

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