linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ipm: fix mutex use
@ 2010-05-14 16:56 Tomas Henzl
  2010-05-14 17:25 ` Corey Minyard
  0 siblings, 1 reply; 2+ messages in thread
From: Tomas Henzl @ 2010-05-14 16:56 UTC (permalink / raw)
  To: 'linux-kernel@vger.kernel.org', 'akpm@osdl.org'
  Cc: minyard, mjg, openipmi-developer

It looks like there is an unbalance with the mutexes after the latest 
IPMI patchset applied. For example in 
"static __devinit int init_ipmi_si(void)"
....
	list_for_each_entry(e, &smi_infos, link) {
		if (!e->irq && (!type || e->addr_source == type)) {
			if (!try_smi_init(e)) {
				type = e->addr_source;
			}
		}
	}
	mutex_unlock(&smi_infos_lock);

we are calling mutex_unlock twice, because the mutex_unlock(&smi_infos_lock)
is also called from try_smi_init.

If the lock in try_smi_init is not needed this can be then solved
by removing the mutex_unlock(&smi_infos_lock) from try_smi_init.

Signed-off-by: Tomas Henzl <thenzl@redhat.com>

---
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 8d7b879..c6af8e0 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3060,8 +3060,6 @@ static int try_smi_init(struct smi_info *new_smi)
 		goto out_err_stop_timer;
 	}
 
-	mutex_unlock(&smi_infos_lock);
-
 	printk(KERN_INFO "IPMI %s interface initialized\n",
 	       si_to_str[new_smi->si_type]);
 
@@ -3111,8 +3109,6 @@ static int try_smi_init(struct smi_info *new_smi)
 		new_smi->dev_registered = 0;
 	}
 
-	mutex_unlock(&smi_infos_lock);
-
 	return rv;
 }
 



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ipm: fix mutex use
  2010-05-14 16:56 [PATCH] ipm: fix mutex use Tomas Henzl
@ 2010-05-14 17:25 ` Corey Minyard
  0 siblings, 0 replies; 2+ messages in thread
From: Corey Minyard @ 2010-05-14 17:25 UTC (permalink / raw)
  To: Tomas Henzl, 'akpm@osdl.org'
  Cc: 'linux-kernel@vger.kernel.org', mjg, openipmi-developer

Yes, you are correct, we need this patch.

Acked-by: Corey Minyard <cminyard@mvista.com>

Thanks.

Tomas Henzl wrote:
> It looks like there is an unbalance with the mutexes after the latest 
> IPMI patchset applied. For example in 
> "static __devinit int init_ipmi_si(void)"
> ....
> 	list_for_each_entry(e, &smi_infos, link) {
> 		if (!e->irq && (!type || e->addr_source == type)) {
> 			if (!try_smi_init(e)) {
> 				type = e->addr_source;
> 			}
> 		}
> 	}
> 	mutex_unlock(&smi_infos_lock);
>
> we are calling mutex_unlock twice, because the mutex_unlock(&smi_infos_lock)
> is also called from try_smi_init.
>
> If the lock in try_smi_init is not needed this can be then solved
> by removing the mutex_unlock(&smi_infos_lock) from try_smi_init.
>
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>
> ---
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 8d7b879..c6af8e0 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3060,8 +3060,6 @@ static int try_smi_init(struct smi_info *new_smi)
>  		goto out_err_stop_timer;
>  	}
>  
> -	mutex_unlock(&smi_infos_lock);
> -
>  	printk(KERN_INFO "IPMI %s interface initialized\n",
>  	       si_to_str[new_smi->si_type]);
>  
> @@ -3111,8 +3109,6 @@ static int try_smi_init(struct smi_info *new_smi)
>  		new_smi->dev_registered = 0;
>  	}
>  
> -	mutex_unlock(&smi_infos_lock);
> -
>  	return rv;
>  }
>  
>
>
>   


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-05-14 17:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-14 16:56 [PATCH] ipm: fix mutex use Tomas Henzl
2010-05-14 17:25 ` Corey Minyard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).