[PATCH]rtl8187:fixed kernel panic on module unload

[PATCH]rtl8187:fixed kernel panic on module unload

[Тхай Кирилл]

From: Tkhai Kirill <tkhai@yandex.ru>

rtl8187: fix kernel panic on module unload (BUG in rtl8187_leds_exit())
	Signed-off-by: Tkhai Kirill <tkhai@yandex.ru>
---
The function rtl8187_leds_exit must not return until all LED's works
in device workqueue have completed or cancelled. Otherwise, we can get
the folowing:
	wq = create_singlethread_workqueue("phyX");

	INIT_DELAYED_WORK(&work, work_func);
	queue_delayed_work(wq, &work, HZ/20);

	destroy_workqueue(wq);

=======>kernel panic
On current vanila driver code (when running on SMP systems), panic happens
almost every time. My patch for version 2.6.31 (and 2.6.31.6 too):

--- a/drivers/net/wireless/rtl818x/rtl8187_leds.c.orig	2009-11-26 22:26:45.000000000 +0300
+++ b/drivers/net/wireless/rtl818x/rtl8187_leds.c	2009-11-28 18:22:26.659785533 +0300
@@ -36,9 +36,6 @@ static void led_turn_on(struct work_stru
 	if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
 		return ;
 
-	/* Skip if the LED is not registered. */
-	if (!led->dev)
-		return;
 	mutex_lock(&priv->conf_mutex);
 	switch (led->ledpin) {
 	case LED_PIN_GPIO0:
@@ -74,9 +71,6 @@ static void led_turn_off(struct work_str
 	if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
 		return ;
 
-	/* Skip if the LED is not registered. */
-	if (!led->dev)
-		return;
 	mutex_lock(&priv->conf_mutex);
 	switch (led->ledpin) {
 	case LED_PIN_GPIO0:
@@ -208,12 +202,12 @@ void rtl8187_leds_exit(struct ieee80211_
 {
 	struct rtl8187_priv *priv = dev->priv;
 
-	/* turn the LED off before exiting */
-	queue_delayed_work(dev->workqueue, &priv->led_off, 0);
-	cancel_delayed_work_sync(&priv->led_off);
-	cancel_delayed_work_sync(&priv->led_on);
 	rtl8187_unregister_led(&priv->led_rx);
 	rtl8187_unregister_led(&priv->led_tx);
+	cancel_delayed_work_sync(&priv->led_on);
+	/* turn the LED off before exiting */
+	queue_delayed_work(dev->workqueue, &priv->led_off, 0);
+	flush_workqueue(dev->workqueue);
 }
 #endif /* def CONFIG_RTL8187_LED */
 

Re: [PATCH]rtl8187:fixed kernel panic on module unload

[Larry Finger]

On 11/30/2009 11:21 AM, Тхай Кирилл wrote:
> From: Tkhai Kirill <tkhai@yandex.ru>
> 
> rtl8187: fix kernel panic on module unload (BUG in rtl8187_leds_exit())
> 	Signed-off-by: Tkhai Kirill <tkhai@yandex.ru>
> ---
> The function rtl8187_leds_exit must not return until all LED's works
> in device workqueue have completed or cancelled. Otherwise, we can get
> the folowing:
> 	wq = create_singlethread_workqueue("phyX");
> 
> 	INIT_DELAYED_WORK(&work, work_func);
> 	queue_delayed_work(wq, &work, HZ/20);
> 
> 	destroy_workqueue(wq);
> 
> =======>kernel panic
> On current vanila driver code (when running on SMP systems), panic happens
> almost every time. My patch for version 2.6.31 (and 2.6.31.6 too):
> 
> --- a/drivers/net/wireless/rtl818x/rtl8187_leds.c.orig	2009-11-26 22:26:45.000000000 +0300
> +++ b/drivers/net/wireless/rtl818x/rtl8187_leds.c	2009-11-28 18:22:26.659785533 +0300
> @@ -36,9 +36,6 @@ static void led_turn_on(struct work_stru
>  	if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
>  		return ;
>  
> -	/* Skip if the LED is not registered. */
> -	if (!led->dev)
> -		return;
>  	mutex_lock(&priv->conf_mutex);
>  	switch (led->ledpin) {
>  	case LED_PIN_GPIO0:
> @@ -74,9 +71,6 @@ static void led_turn_off(struct work_str
>  	if (priv->mode == NL80211_IFTYPE_UNSPECIFIED)
>  		return ;
>  
> -	/* Skip if the LED is not registered. */
> -	if (!led->dev)
> -		return;
>  	mutex_lock(&priv->conf_mutex);
>  	switch (led->ledpin) {
>  	case LED_PIN_GPIO0:
> @@ -208,12 +202,12 @@ void rtl8187_leds_exit(struct ieee80211_
>  {
>  	struct rtl8187_priv *priv = dev->priv;
>  
> -	/* turn the LED off before exiting */
> -	queue_delayed_work(dev->workqueue, &priv->led_off, 0);
> -	cancel_delayed_work_sync(&priv->led_off);
> -	cancel_delayed_work_sync(&priv->led_on);
>  	rtl8187_unregister_led(&priv->led_rx);
>  	rtl8187_unregister_led(&priv->led_tx);
> +	cancel_delayed_work_sync(&priv->led_on);
> +	/* turn the LED off before exiting */
> +	queue_delayed_work(dev->workqueue, &priv->led_off, 0);
> +	flush_workqueue(dev->workqueue);
>  }
>  #endif /* def CONFIG_RTL8187_LED */

Have you applied the fix in mainline commit
37b12dd2b07b4d7dc222a5f7f88b25cec532b2aa? It should have fixed this problem.

Larry

Re: [PATCH]rtl8187:fixed kernel panic on module unload

[Hin-Tak Leung]

--- On Mon, 30/11/09, Larry Finger <Larry.Finger@lwfinger.net> wrote:

> Have you applied the fix in mainline commit
> 37b12dd2b07b4d7dc222a5f7f88b25cec532b2aa? It should have
> fixed this problem.

37b12dd2b07b4d7dc222a5f7f88b25cec532b2aa enters mainline at v2.6.32-rc7.

I suppose it is a question for John - wireless-testing only have the vX.Y.Z-rc* and the vX.Y.Z tags... it would be useful to have the vX.Y.Z.? maintenance release tags as well (it is a matter of knowing where to git pull?) to know if a change has gone into one of the maintenance releases.