[PATCH] Correction to kmod.c control loop

[borsa77@libero.it]

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/