Re: [PATCH] b43: Fix rfkill callback deadlock
On Tuesday 30 October 2007 02:56:51 David Ellingsworth wrote: If that is the case, then a proper fix would be to use two locks to protect access to the status. One for allowing read access when no one is writing, and another for allowing exclusive write access. In such a configuration you could allow multiple readers while having only one writer. The above fix is not proper since the lock obtained by another section of code could be released before or during the status check. We don't care for that case, as the status is rechecked under lock later. This really is not a problem. We only care for the case where status!=INITIALIZED. Because then we mustn't aquire the lock and return silently. And that's what we do. -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
RE: [PATCH] b43: Fix rfkill callback deadlock
From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: [PATCH] b43: Fix
rfkill callback deadlock Date: Sun, 28 Oct 2007 17:27:10 +0100 CC: [EMAIL
PROTECTED]; bcm43xx-dev@lists.berlios.de; [EMAIL PROTECTED] wl-mutex
might already be locked on initialization. Signed-off-by: Michael Buesch
[EMAIL PROTECTED] Index: wireless-2.6/drivers/net/wireless/b43/rfkill.c
=== ---
wireless-2.6.orig/drivers/net/wireless/b43/rfkill.c 2007-10-27
13:28:16.0 +0200 +++ wireless-2.6/drivers/net/wireless/b43/rfkill.c
2007-10-28 17:13:55.0 +0100 @@ -61,15 +61,22 @@ static void
b43_rfkill_poll(struct input mutex_unlock(wl-mutex); } -/* Called when
the RFKILL toggled in software. - * This is called without locking. */ +/*
Called when the RFKILL toggled in software. */ static int
b43_rfkill_soft_toggle(void *data, enum rfkill_state state) { struct
b43_wldev *dev = data; struct b43_wl *wl = dev-wl; int err = 0; -
mutex_lock(wl-mutex); + /* When RFKILL is registered, it will call back
into this callback. + * wl-mutex will already be locked when this happens.
+ * So first trylock. On contention check if we are in initialization. + *
Silently return if that happens to avoid a deadlock. */ + if
(mutex_trylock(wl-mutex) == 0) { + if (b43_status(dev)
B43_STAT_INITIALIZED) + return 0; + mutex_lock(wl-mutex); + } if
(b43_status(dev) B43_STAT_INITIALIZED) goto out_unlock;
Why not replace the above lines up to and including - mutex_loc(wl-mutext);
with:
if(b43_status(dev) B43_STAT_INITIALIZED)
return 0;
mutex_lock(wl-mutex) @@ -89,7 +96,6 @@ static int
b43_rfkill_soft_toggle(void * b43_radio_turn_off(dev, 0); break; } -
out_unlock: mutex_unlock(wl-mutex);
___ Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
_
Boo! Scare away worms, viruses and so much more! Try Windows Live OneCare!
http://onecare.live.com/standard/en-us/purchase/trial.aspx?s_cid=wl_hotmailnews___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43: Fix rfkill callback deadlock
On Sunday 28 October 2007 23:37:10 David Ellingsworth wrote:
From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: [PATCH] b43: Fix
rfkill callback deadlock Date: Sun, 28 Oct 2007 17:27:10 +0100 CC: [EMAIL
PROTECTED]; bcm43xx-dev@lists.berlios.de; [EMAIL PROTECTED] wl-mutex
might already be locked on initialization. Signed-off-by: Michael Buesch
[EMAIL PROTECTED] Index:
wireless-2.6/drivers/net/wireless/b43/rfkill.c
=== ---
wireless-2.6.orig/drivers/net/wireless/b43/rfkill.c 2007-10-27
13:28:16.0 +0200 +++
wireless-2.6/drivers/net/wireless/b43/rfkill.c 2007-10-28
17:13:55.0 +0100 @@ -61,15 +61,22 @@ static void
b43_rfkill_poll(struct input mutex_unlock(wl-mutex); } -/* Called
when the RFKILL toggled in software. - * This is called without locking.
*/ +/* Called when the RFKILL toggled in software. */ static int
b43_rfkill_soft_toggle(void *data, enum rfkill_state state) { struct
b43_wldev *dev = data; struct b43_wl *wl = dev-wl; int err = 0; -
mutex_lock(wl-mutex); + /* When RFKILL is registered, it will call back
into this callback. + * wl-mutex will already be locked when this
happens. + * So first trylock. On contention check if we are in
initialization. + * Silently return if that happens to avoid a deadlock.
*/ + if (mutex_trylock(wl-mutex) == 0) { + if (b43_status(dev)
B43_STAT_INITIALIZED) + return 0; + mutex_lock(wl-mutex); + } if
(b43_status(dev) B43_STAT_INITIALIZED) goto out_unlock;
Why not replace the above lines up to and including -
mutex_loc(wl-mutext); with:
if(b43_status(dev) B43_STAT_INITIALIZED)
return 0;
mutex_lock(wl-mutex) @@ -89,7 +96,6 @@ static int
b43_rfkill_soft_toggle(void * b43_radio_turn_off(dev, 0); break; } -
out_unlock: mutex_unlock(wl-mutex);
___ Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
_
Boo! Scare away worms, viruses and so much more! Try Windows Live OneCare!
http://onecare.live.com/standard/en-us/purchase/trial.aspx?s_cid=wl_hotmailnews
I'm sorry. Your mailer completely fucked up the context
so I'm not really able to see what you want to tell me.
--
Greetings Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
RE: [PATCH] b43: Fix rfkill callback deadlock
Sorry, hopefully its readable this time.
From: [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Subject: [PATCH] b43: Fix rfkill callback deadlock
Date: Sun, 28 Oct 2007 17:27:10 +0100
CC: [EMAIL PROTECTED]; bcm43xx-dev@lists.berlios.de; [EMAIL PROTECTED]
wl-mutex might already be locked on initialization.
Signed-off-by: Michael Buesch
Index: wireless-2.6/drivers/net/wireless/b43/rfkill.c
===
--- wireless-2.6.orig/drivers/net/wireless/b43/rfkill.c 2007-10-27
13:28:16.0 +0200
+++ wireless-2.6/drivers/net/wireless/b43/rfkill.c 2007-10-28
17:13:55.0 +0100
@@ -61,15 +61,22 @@ static void b43_rfkill_poll(struct input
mutex_unlock(wl-mutex);
}
-/* Called when the RFKILL toggled in software.
- * This is called without locking. */
+/* Called when the RFKILL toggled in software. */
static int b43_rfkill_soft_toggle(void *data, enum rfkill_state state)
{
struct b43_wldev *dev = data;
struct b43_wl *wl = dev-wl;
int err = 0;
- mutex_lock(wl-mutex);
+ /* When RFKILL is registered, it will call back into this callback.
+ * wl-mutex will already be locked when this happens.
+ * So first trylock. On contention check if we are in initialization.
+ * Silently return if that happens to avoid a deadlock. */
+ if (mutex_trylock(wl-mutex) == 0) {
+ if (b43_status(dev) B43_STAT_INITIALIZED)
+ return 0;
+ mutex_lock(wl-mutex);
+ }
if (b43_status(dev) B43_STAT_INITIALIZED)
goto out_unlock;
Why not replace everything above up to and including the -
mutex_lock(wl-mutex); with:
if(b43_status(dev) B43_STAT_INITIALIZED)
return 0;
mutex_lock(wl-mutex);
@@ -89,7 +96,6 @@ static int b43_rfkill_soft_toggle(void *
b43_radio_turn_off(dev, 0);
break;
}
-
out_unlock:
mutex_unlock(wl-mutex);
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
_
Peek-a-boo FREE Tricks Treats for You!
http://www.reallivemoms.com?ocid=TXT_TAGHMloc=us
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH] b43: Fix rfkill callback deadlock
On Monday 29 October 2007 01:06:51 David Ellingsworth wrote:
- mutex_lock(wl-mutex);
+ /* When RFKILL is registered, it will call back into this callback.
+ * wl-mutex will already be locked when this happens.
+ * So first trylock. On contention check if we are in initialization.
+ * Silently return if that happens to avoid a deadlock. */
+ if (mutex_trylock(wl-mutex) == 0) {
+ if (b43_status(dev) B43_STAT_INITIALIZED)
+ return 0;
+ mutex_lock(wl-mutex);
+ }
if (b43_status(dev) B43_STAT_INITIALIZED)
goto out_unlock;
Why not replace everything above up to and including the -
mutex_lock(wl-mutex); with:
if(b43_status(dev) B43_STAT_INITIALIZED)
return 0;
mutex_lock(wl-mutex);
because the status should be queried under lock, of course.
--
Greetings Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
