Correction to kmod.c control loop

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.

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

2.4.29 is a pretty old kernel.

If you want to work on 2.4.x, then work against the latest version - 2.4.32 currently.

Even better is to work on 2.6.x instead (or both 2.6 & 2.4), and if you do you should generally send patches against latest Linus kernel - currently that's 2.6.15-rc5-git1

Working against an old kernel like 2.4.29 is often a waste of time since whatever you are trying to fix may very well already be fixed in a newer kernel (something you should at the very least check), so by working against the latest kernel you save both your own and everyone elses time.

Anyway, a few comments below.

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html