diff mbox

race condition with activate_module?

Message ID 201002121906.46804.lindner_marek@yahoo.de (mailing list archive)
State RFC, archived
Headers

Commit Message

Marek Lindner Feb. 12, 2010, 11:06 a.m. UTC 
  On Thursday 11 February 2010 17:20:16 Marek Lindner wrote:
> Better than introducing some locking code which would need to halt the
>  whole  module we should make sure that batman-adv does not process packets
>  before its initialization is complete.

I made a proof-of-concept patch (no testing yet) but it should illustrate what 
I was talking about. Please try it.

Cheers,
Marek

Comments

Simon Wunderlich Feb. 14, 2010, 4:58 p.m. UTC | #1 
Works fine and really seems to make a difference when trying with the ssleep()
stuff from linus (which also crashes in my qemu testbed). After applying your
patch it works fine. I have committed it along with some other sanity checks 
in r1573.

On Fri, Feb 12, 2010 at 07:06:46PM +0800, Marek Lindner wrote:
> On Thursday 11 February 2010 17:20:16 Marek Lindner wrote:
> > Better than introducing some locking code which would need to halt the
> >  whole  module we should make sure that batman-adv does not process packets
> >  before its initialization is complete.
> 
> I made a proof-of-concept patch (no testing yet) but it should illustrate what 
> I was talking about. Please try it.
> 
> Cheers,
> Marek

> diff --git a/batman-adv-kernelland/proc.c b/batman-adv-kernelland/proc.c
> index 3f5744d..76dd4d2 100644
> --- a/batman-adv-kernelland/proc.c
> +++ b/batman-adv-kernelland/proc.c
> @@ -115,10 +115,6 @@ static ssize_t proc_interfaces_write(struct file *instance,
>  
>  	hardif_add_interface(if_string, if_num);
>  
> -	if ((atomic_read(&module_state) == MODULE_INACTIVE) &&
> -	    (hardif_get_active_if_num() > 0))
> -		activate_module();
> -
>  	rcu_read_lock();
>  	if (list_empty(&if_list)) {
>  		rcu_read_unlock();
> @@ -127,6 +123,11 @@ static ssize_t proc_interfaces_write(struct file *instance,
>  	rcu_read_unlock();
>  
>  	num_ifs = if_num + 1;
> +
> +	if ((atomic_read(&module_state) == MODULE_INACTIVE) &&
> +	    (hardif_get_active_if_num() > 0))
> +		activate_module();
> +
>  	return count;
>  
>  end:
> @@ -453,7 +454,7 @@ static ssize_t proc_aggr_write(struct file *file, const char __user *buffer,
>  		       atomic_read(&aggregation_enabled),
>  		       (aggregation_enabled_tmp == 1 ? "enabled" : "disabled"),
>  		       aggregation_enabled_tmp);
> -		atomic_set(&aggregation_enabled, 
> +		atomic_set(&aggregation_enabled,
>  						(unsigned)aggregation_enabled_tmp);
>  	}
>
diff mbox

Patch

diff --git a/batman-adv-kernelland/proc.c b/batman-adv-kernelland/proc.c
index 3f5744d..76dd4d2 100644
--- a/batman-adv-kernelland/proc.c
+++ b/batman-adv-kernelland/proc.c
@@ -115,10 +115,6 @@  static ssize_t proc_interfaces_write(struct file *instance,
 
 	hardif_add_interface(if_string, if_num);
 
-	if ((atomic_read(&module_state) == MODULE_INACTIVE) &&
-	    (hardif_get_active_if_num() > 0))
-		activate_module();
-
 	rcu_read_lock();
 	if (list_empty(&if_list)) {
 		rcu_read_unlock();
@@ -127,6 +123,11 @@  static ssize_t proc_interfaces_write(struct file *instance,
 	rcu_read_unlock();
 
 	num_ifs = if_num + 1;
+
+	if ((atomic_read(&module_state) == MODULE_INACTIVE) &&
+	    (hardif_get_active_if_num() > 0))
+		activate_module();
+
 	return count;
 
 end:
@@ -453,7 +454,7 @@  static ssize_t proc_aggr_write(struct file *file, const char __user *buffer,
 		       atomic_read(&aggregation_enabled),
 		       (aggregation_enabled_tmp == 1 ? "enabled" : "disabled"),
 		       aggregation_enabled_tmp);
-		atomic_set(&aggregation_enabled, 
+		atomic_set(&aggregation_enabled,
 						(unsigned)aggregation_enabled_tmp);
 	}