[PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Larry Finger]

When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
may oops when the module is unloaded as the workqueue for led_on was
not being cancelled.

This patch fixes the problem reported in
http://marc.info/?l=linux-wireless&m=124742957615781&w=2.

Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
---

V2 - Do not create a new workqueue.

John,

This patch is 2.6.31 material. To the best of my knowledge, a formal bug
report was never filed; however, it was reported in the reference given above.

Larry
---

Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
+++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
@@ -208,11 +208,12 @@ void rtl8187_leds_exit(struct ieee80211_
 {
 	struct rtl8187_priv *priv = dev->priv;
 
-	rtl8187_unregister_led(&priv->led_tx);
 	/* 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);
 }
 #endif /* def CONFIG_RTL8187_LED */
 

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Richard Farina]

Larry Finger wrote:
> When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
> may oops when the module is unloaded as the workqueue for led_on was
> not being cancelled.
>
> This patch fixes the problem reported in
> http://marc.info/?l=linux-wireless&m=124742957615781&w=2.
>
> Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
> ---
>
> V2 - Do not create a new workqueue.
>
> John,
>
> This patch is 2.6.31 material. To the best of my knowledge, a formal bug
> report was never filed; however, it was reported in the reference given above.
>
>   
Anyone know what happened here? This bug still seems very much alive in 
compat-wireless-2.6.31-rc7.  I know the window is closed and this really
isn't "earthshattering" but a kernel panick is kind of "a big deal".  I 
seems like it was tested doing a proper modprobe -r but not if you just 
unplug the usb card.
When I unplug the usb I get instadeath, very uncool.  If someone can 
teach me how to get the kernel output from a non-functional system I am 
happy to provide whatever.

thanks,
Rick Farina
> Larry
> ---
>
> Index: wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/rtl818x/rtl8187_leds.c
> +++ wireless-testing/drivers/net/wireless/rtl818x/rtl8187_leds.c
> @@ -208,11 +208,12 @@ void rtl8187_leds_exit(struct ieee80211_
>  {
>  	struct rtl8187_priv *priv = dev->priv;
>  
> -	rtl8187_unregister_led(&priv->led_tx);
>  	/* 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);
>  }
>  #endif /* def CONFIG_RTL8187_LED */
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Larry Finger]

Richard Farina wrote:
> Larry Finger wrote:
>> When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
>> may oops when the module is unloaded as the workqueue for led_on was
>> not being cancelled.
>>
>> This patch fixes the problem reported in
>> http://marc.info/?l=linux-wireless&m=124742957615781&w=2.
>>
>> Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
>> ---
>>
>> V2 - Do not create a new workqueue.
>>
>> John,
>>
>> This patch is 2.6.31 material. To the best of my knowledge, a formal bug
>> report was never filed; however, it was reported in the reference
>> given above.
>>
>>   
> Anyone know what happened here? This bug still seems very much alive in
> compat-wireless-2.6.31-rc7.  I know the window is closed and this really
> isn't "earthshattering" but a kernel panick is kind of "a big deal".  I
> seems like it was tested doing a proper modprobe -r but not if you just
> unplug the usb card.

This kind of accusation is not the best way to get your problem solved.

> When I unplug the usb I get instadeath, very uncool.  If someone can
> teach me how to get the kernel output from a non-functional system I am
> happy to provide whatever.

This patch has been in wireless testing since August 3. I have no idea
why it wouldn't be in compat-wireless since then. In addition, I have
unplugged/plugged my RTL8187B device many times with no problem.

I just downloaded compat-wireless-2009-08-26 - it has the patch
included. You certainly could have checked your source to determine that.

To get something from a system that is crashing, you should switch to
the logging console by pressing CTRL/ALT/F10 before you do whatever it
takes to crash it. You will not get a full dump, but hopefully, there
will be enough of the stack visible when the panic occurs. Either copy
down the stack list, or take a picture of the screen and post a link
to it. FYI, you can get back to the graphical screen with CTRL/ALT/F7.

If you have a wired connection in addition to the wireless one, and
you have a second Linux host, you can also capture the dump using
netconsole. Using this facility is not easy, so we'll go the other
route first.

When you report what info you have, please tell what architecture you
are running (i386, x86_64, ppc,...) and whether your device is an
RTL8187L or RTL8187B. It may not matter, but who knows.

Larry

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Richard Farina]

Larry Finger wrote:
> Richard Farina wrote:
>   
>> Larry Finger wrote:
>>     
>>> When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
>>> may oops when the module is unloaded as the workqueue for led_on was
>>> not being cancelled.
>>>
>>> This patch fixes the problem reported in
>>> http://marc.info/?l=linux-wireless&m=124742957615781&w=2.
>>>
>>> Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
>>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
>>> ---
>>>
>>> V2 - Do not create a new workqueue.
>>>
>>> John,
>>>
>>> This patch is 2.6.31 material. To the best of my knowledge, a formal bug
>>> report was never filed; however, it was reported in the reference
>>> given above.
>>>
>>>   
>>>       
>> Anyone know what happened here? This bug still seems very much alive in
>> compat-wireless-2.6.31-rc7.  I know the window is closed and this really
>> isn't "earthshattering" but a kernel panick is kind of "a big deal".  I
>> seems like it was tested doing a proper modprobe -r but not if you just
>> unplug the usb card.
>>     
>
> This kind of accusation is not the best way to get your problem solved.
>
>   
I apologize if you thought there was an accusation here, that certainly 
was not my intention. With all due respect assuming that I'm
a jerk is neither safe nor polite.  In your original patch for this I 
find the following text:

"Gábor,

I hope this version of the patch fixes your problem. On my system
I ran more than 20 rmmod/insmod cycles without a problem.

Larry"

My assumption in your testing manner was based only on what you said and the kernel panick I still experience

>> When I unplug the usb I get instadeath, very uncool.  If someone can
>> teach me how to get the kernel output from a non-functional system I am
>> happy to provide whatever.
>>     
>
> This patch has been in wireless testing since August 3. I have no idea
> why it wouldn't be in compat-wireless since then. In addition, I have
> unplugged/plugged my RTL8187B device many times with no problem.
>
> I just downloaded compat-wireless-2009-08-26 - it has the patch
> included. You certainly could have checked your source to determine that.
>
>   
Yes, I realize that, which is why I said the "bug is still alive" not 
"the patch is not applied".  I even went out of my way to describe how 
the testing was different...
> To get something from a system that is crashing, you should switch to
> the logging console by pressing CTRL/ALT/F10 before you do whatever it
> takes to crash it. You will not get a full dump, but hopefully, there
> will be enough of the stack visible when the panic occurs. Either copy
> down the stack list, or take a picture of the screen and post a link
> to it. FYI, you can get back to the graphical screen with CTRL/ALT/F7.
>
> If you have a wired connection in addition to the wireless one, and
> you have a second Linux host, you can also capture the dump using
> netconsole. Using this facility is not easy, so we'll go the other
> route first.
>
> When you report what info you have, please tell what architecture you
> are running (i386, x86_64, ppc,...) and whether your device is an
> RTL8187L or RTL8187B. It may not matter, but who knows.
>
>   
I will attempt to reproduce and take a picture with both x86 and x86_64, 
and I'll look into netconsole because that sounds very useful.

Thanks for the assistance,
Rick Farina

> Larry
>
>
>   

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Hin-Tak Leung]

On Wed, Aug 26, 2009 at 10:34 PM, Richard Farina<sidhayn@gmail.com> wrote:

> I apologize if you thought there was an accusation here, that certainly was
> not my intention. With all due respect assuming that I'm
> a jerk is neither safe nor polite.  In your original patch for this I find
> the following text:

> Yes, I realize that, which is why I said the "bug is still alive" not "the
> patch is not applied".  I even went out of my way to describe how the
> testing was different...

While I thought Larry's reply was a bit brief, I looked back on the
thread and I think I am on Larry's side. All you mentioned was that
you had a crash on unplug, and then you automatically assume that it
is because the most recent crash-related fix (which Larry kindly made)
did not work. You have no proof that your problem is even the same as
what the fix tried to solve. You did not try to go back to a non-LED
enabled compat-wireless, for example.

To be honest, I think you 'went out of your way' to try to pin your
problem on one of the developers, so that your problem is guaranteed
to receive attention. That's not very fair...

That said, I think this thread has gone on long enough - all that
happened is that (1) you had a crash on unplug, (2) you tried to pin
it on Larry, (3) many people has checked and verified that what Larry
did went in, (4) we still don't know whether your crash is related to
the LED change or not.

Since we still don't know whether your crash is related to any
LED-related change or not, this thread is not going well.

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Richard Farina]

Hin-Tak Leung wrote:
> On Wed, Aug 26, 2009 at 10:34 PM, Richard Farina<sidhayn@gmail.com> wrote:
>
>   
>> I apologize if you thought there was an accusation here, that certainly was
>> not my intention. With all due respect assuming that I'm
>> a jerk is neither safe nor polite.  In your original patch for this I find
>> the following text:
>>     
>
>   
>> Yes, I realize that, which is why I said the "bug is still alive" not "the
>> patch is not applied".  I even went out of my way to describe how the
>> testing was different...
>>     
>
> While I thought Larry's reply was a bit brief, I looked back on the
> thread and I think I am on Larry's side. All you mentioned was that
> you had a crash on unplug, and then you automatically assume that it
> is because the most recent crash-related fix (which Larry kindly made)
> did not work. You have no proof that your problem is even the same as
> what the fix tried to solve. You did not try to go back to a non-LED
> enabled compat-wireless, for example.
>
> To be honest, I think you 'went out of your way' to try to pin your
> problem on one of the developers, so that your problem is guaranteed
> to receive attention. That's not very fair...
>
> That said, I think this thread has gone on long enough - all that
> happened is that (1) you had a crash on unplug, (2) you tried to pin
> it on Larry, (3) many people has checked and verified that what Larry
> did went in, (4) we still don't know whether your crash is related to
> the LED change or not.
>
> Since we still don't know whether your crash is related to any
> LED-related change or not, this thread is not going well.
>
>   
I accept your criticism.  I should have been more clear.  This happens 
only when CONFIG_RTL8187_LEDS=y is set.

I'm not trying to "pin" the bug on Larry claiming it is his fault, I'm 
merely stating that I am experiencing a problem that looks extremely 
similar to what he described when he pushed the mentioned patch.

Again, I'm not trying to point my finger blaming someone, I'm trying to 
help fix a problem.  I am traveling for the next few days, when I get 
home to my camera I hope you are all calm enough to help me troubleshoot 
this bug so we can find a solution instead of accusing each other of 
accusing each other.

My apologies for any perceived insults against anyone, seriously, that's 
not my goal here.


-Rick

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Andrey Yurovsky]

Hi Richard,

On Wed, Aug 26, 2009 at 4:11 PM, Richard Farina<sidhayn@gmail.com> wrote:
> Again, I'm not trying to point my finger blaming someone, I'm trying to help
> fix a problem.  I am traveling for the next few days, when I get home to my
> camera I hope you are all calm enough to help me troubleshoot this bug so we
> can find a solution instead of accusing each other of accusing each other.

you may want to try kexec in place of having to take photos of the
kernel panic output.  This will enable you to load a crash-recovery
kernel, which will be booted automatically once you've successfully
reproduced the problem.  From there you can use gdb together with
/proc/vmcore and your kernel image to reconstruct what you'd see in
dmesg and paste the entire oops.  Please see
Documentation/kdump/kdump.txt for more details and use
Documentation/kdump/gdbmacros.txt to give gdb a "dmesg" macro.

  -Andrey

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Gábor Stefanik]

On Thu, Aug 27, 2009 at 4:03 AM, Andrey Yurovsky<andrey@cozybit.com> wrote:
> Hi Richard,
>
> On Wed, Aug 26, 2009 at 4:11 PM, Richard Farina<sidhayn@gmail.com> wrote:
>> Again, I'm not trying to point my finger blaming someone, I'm trying to help
>> fix a problem.  I am traveling for the next few days, when I get home to my
>> camera I hope you are all calm enough to help me troubleshoot this bug so we
>> can find a solution instead of accusing each other of accusing each other.
>
> you may want to try kexec in place of having to take photos of the
> kernel panic output.  This will enable you to load a crash-recovery
> kernel, which will be booted automatically once you've successfully
> reproduced the problem.  From there you can use gdb together with
> /proc/vmcore and your kernel image to reconstruct what you'd see in
> dmesg and paste the entire oops.  Please see
> Documentation/kdump/kdump.txt for more details and use
> Documentation/kdump/gdbmacros.txt to give gdb a "dmesg" macro.

Or use the "crash" utility (a wrapper for gdb), which can directly
extract the kernel log from vmcore.

>
>  -Andrey
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[John W. Linville]

On Tue, Aug 25, 2009 at 11:46:38PM -0400, Richard Farina wrote:
> Larry Finger wrote:
>> When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
>> may oops when the module is unloaded as the workqueue for led_on was
>> not being cancelled.
>>
>> This patch fixes the problem reported in
>> http://marc.info/?l=linux-wireless&m=124742957615781&w=2.
>>
>> Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
>> ---
>>
>> V2 - Do not create a new workqueue.
>>
>> John,
>>
>> This patch is 2.6.31 material. To the best of my knowledge, a formal bug
>> report was never filed; however, it was reported in the reference given above.
>>
>>   
> Anyone know what happened here? This bug still seems very much alive in  
> compat-wireless-2.6.31-rc7.  I know the window is closed and this really
> isn't "earthshattering" but a kernel panick is kind of "a big deal".  I  
> seems like it was tested doing a proper modprobe -r but not if you just  
> unplug the usb card.
> When I unplug the usb I get instadeath, very uncool.  If someone can  
> teach me how to get the kernel output from a non-functional system I am  
> happy to provide whatever.

commit 3da7429ce92abd79b14e2275a28be144ce2c3013
Author: Larry Finger <Larry.Finger@lwfinger.net>
Date:   Tue Jul 14 15:55:16 2009 -0500

    rtl8187: Fix for kernel oops when unloading with LEDs enabled
    
    When rtl8187 is unloaded and CONFIG_RTL8187_LEDS is set, the kernel
    may oops when the module is unloaded as the workqueue for led_on was
    not being cancelled.
    
    This patch fixes the problem reported in
    http://marc.info/?l=linux-wireless&m=124742957615781&w=2.
    
    Reported-by: Gábor Stefanik <netrolller.3d@gmail.com>
    Signed-off-by: Larry Finger <Larry.Finger@lwfinger>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

The above commit is in linux-2.6 since v2.6.31-rc2.  If it isn't in
compat-wireless, I have no idea why.

Hth...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Larry Finger]

John W. Linville wrote:
> 
> The above commit is in linux-2.6 since v2.6.31-rc2.  If it isn't in
> compat-wireless, I have no idea why.

It is - I looked at the code.

Larry

Re: [PATCH] rtl8187: Fix for kernel oops when unloading with LEDs enabled

[Luis R. Rodriguez]

On Wed, Aug 26, 2009 at 9:03 AM, Larry Finger<Larry.Finger@lwfinger.net> wrote:
> John W. Linville wrote:
>>
>> The above commit is in linux-2.6 since v2.6.31-rc2.  If it isn't in
>> compat-wireless, I have no idea why.
>
> It is - I looked at the code.

Just a heads up -- I pull "stable" code for compat-wireless using
hpa's tree, not Linus:

git://git.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-allstable.git

Its convenient for me as I get to have branches for each kernel
release. Regardless though hpa seems to update this stable tree when
Linus updates his, so its at rc7 now for the master branch. The code
pulled into the stable compat-wireless stuff then is from the latest
stable code 2.6.2x.y or Linus' latest rc as on hpa's master branch.

  Luis