[PATCH] Correction to kmod.c control loop

From: borsa77
Date: Tue Dec 20 2005 - 15:41:30 EST


I tried this patch on my system Slackware 10.1 with the version kernel
2.4.29 with any problem, below it is in broken form to allow comment
to the source.

--- ./kmod.bak 2005-12-19 12:48:56.000000000 +0100
+++ ../kernel/kmod.c 2005-12-19 13:29:44.000000000 +0100
@@ -175,13 +175,11 @@
*/
int request_module(const char * module_name)
{
- pid_t pid;
- int waitpid_result;
+ pid_t pid, waitpid_result;
sigset_t tmpsig;
int i;
static atomic_t kmod_concurrent = ATOMIC_INIT(0);
-#define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary
value - KAO */
- static int kmod_loop_msg;
+ static int MAX_KMOD_CONCURRENT, kmod_loop_msg;

The man page for waitpid function tells the return type is pid_t.

/* Don't allow request_module() before the root fs is mounted! */
if ( ! current->fs->root ) {
@@ -192,7 +190,7 @@


/* If modprobe needs a service that is in a module, we get a
recursive
* loop. Limit the number of running kmod threads to
max_threads/2 or
- * MAX_KMOD_CONCURRENT, whichever is the smaller. A
cleaner method
+ * MAX_KMOD_CONCURRENT, whichever is the larger. A
cleaner method
* would be to run the parents of this process, counting how
many times
* kmod was invoked. That would mean accessing the internals
of the
* process tables to get the command line, proc_pid_cmdline is
static
@@ -200,7 +198,7 @@
* KAO.
*/
i = max_threads/2;
- if (i > MAX_KMOD_CONCURRENT)
+ if (i < MAX_KMOD_CONCURRENT)
i = MAX_KMOD_CONCURRENT;
atomic_inc(&kmod_concurrent);
if (atomic_read(&kmod_concurrent) > i) {
@@ -208,6 +206,7 @@
printk(KERN_ERR
"kmod: runaway modprobe loop assumed
and stopped\n");
atomic_dec(&kmod_concurrent);
+ MAX_KMOD_CONCURRENT =
2*MAX_KMOD_CONCURRENT+1;
return -ENOMEM;
}

Two advantages: (i) you do not worry about the choice of an arbitrary
value, (ii) you can reiterate modprobe command until the module is
loaded because MAX_KMOD_CONCURRENT grows with arithmetic
progression.

@@ -237,6 +236,7 @@
if (waitpid_result != pid) {
printk(KERN_ERR "request_module[%s]: waitpid(%d,...)
failed, errno %d\n",
module_name, pid, -waitpid_result);
+ return waitpid_result;
}
return 0;
}

I think here the exit point was omitted because originally the check was
before the unblock of the signals, now it is safe because it is at the end
so the errorcode should be handled.

If you believe these corrections are valid, please you will send me
feedback. Otherwise I am sorry for this lack of time.
Regards, Marco Borsari.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/