[Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Huaxu Wan
From f3628f0ac6c9713afadaa9654815c37a294ce7e2 Mon Sep 17 00:00:00 2001
From: Huaxu Wan <[hidden email]>
Date: Fri, 8 May 2009 07:36:58 -0400
Subject: [PATCH] cancel rfkill_poll with sync when module is exiting

Re-write the patch due to there is no response from TJ.

When the the interface is down, a delayed work rfkill_poll is queued.
This work must be canceled before the module is unloaded, or the OS would
suffer a hard lock-up.
 
This patch fix the bug LP: #345710 by make sure the queued work is canceled.


Original info:
http://marc.info/?l=linux-wireless&m=123791044313158&w=2
Bug: #345710

When the wireless interface is active and the iwl3945 module is unloaded the
call to ieee80211_unregister_hw() would call iwl3945_mac_stop() which would
restart the delayed workqueue for rfkill_poll. That workqueue had already been
cancelled so when the next work item was run (2 seconds later) the system would
suffer a hard lock-up because the module had been unloaded by then.

This patch implements STATUS_EXIT_PENDING checks in places where the rfkill_poll
work is scheduled, and moves the final workqueue cancellation to occur after the
call to ieee80211_unregister_hw().

Bug discovered, experienced and fix tested on my PC.

Signed-off-by: TJ <[hidden email]>
Signed-off-by: Huaxu Wan <[hidden email]>
---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index bb92db2..08c2a9d 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -8166,7 +8166,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
  sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
 
  iwl3945_rfkill_unregister(priv);
- cancel_delayed_work(&priv->rfkill_poll);
  iwl3945_dealloc_ucode_pci(priv);
 
  if (priv->rxq.bd)
@@ -8181,6 +8180,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
 
  /*netif_stop_queue(dev); */
  flush_workqueue(priv->workqueue);
+ cancel_delayed_work_sync(&priv->rfkill_poll);
 
  /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
  * priv->workqueue... so we can't take down the workqueue
--
1.6.0.4



--
Thanks
Huaxu

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Stefan Bader-2
Hi Huaxu,

thanks for the work and sorry for being so late in response. It seems, by now
the patch made it upstream as

commit 71d449b55abf5018d7c711b2b62abc0c083723c4
Author: Reinette Chatre <[hidden email]>
Date:   Mon Apr 20 14:37:01 2009 -0700

     iwl3945: use cancel_delayed_work_sync to cancel rfkill_poll

     Users reported lockup with work still trying to run
     after module has been unloaded.

     http://thread.gmane.org/gmane.linux.kernel.wireless.general/30594/focus=3060

     Signed-off-by: Reinette Chatre <[hidden email]>
     Reported-by: TJ <[hidden email]>
     Reported-by: Huaxu Wan <[hidden email]>
     Signed-off-by: John W. Linville <[hidden email]>


This looks even simpler by just syncing the cancellation. I will go on and try
to get this version into SRU. Thanks again.

Stefan

Huaxu Wan wrote:

> From f3628f0ac6c9713afadaa9654815c37a294ce7e2 Mon Sep 17 00:00:00 2001
> From: Huaxu Wan <[hidden email]>
> Date: Fri, 8 May 2009 07:36:58 -0400
> Subject: [PATCH] cancel rfkill_poll with sync when module is exiting
>
> Re-write the patch due to there is no response from TJ.
>
> When the the interface is down, a delayed work rfkill_poll is queued.
> This work must be canceled before the module is unloaded, or the OS would
> suffer a hard lock-up.
>  
> This patch fix the bug LP: #345710 by make sure the queued work is canceled.
>
>
> Original info:
> http://marc.info/?l=linux-wireless&m=123791044313158&w=2
> Bug: #345710
>
> When the wireless interface is active and the iwl3945 module is unloaded the
> call to ieee80211_unregister_hw() would call iwl3945_mac_stop() which would
> restart the delayed workqueue for rfkill_poll. That workqueue had already been
> cancelled so when the next work item was run (2 seconds later) the system would
> suffer a hard lock-up because the module had been unloaded by then.
>
> This patch implements STATUS_EXIT_PENDING checks in places where the rfkill_poll
> work is scheduled, and moves the final workqueue cancellation to occur after the
> call to ieee80211_unregister_hw().
>
> Bug discovered, experienced and fix tested on my PC.
>
> Signed-off-by: TJ <[hidden email]>
> Signed-off-by: Huaxu Wan <[hidden email]>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index bb92db2..08c2a9d 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -8166,7 +8166,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>   sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
>  
>   iwl3945_rfkill_unregister(priv);
> - cancel_delayed_work(&priv->rfkill_poll);
>   iwl3945_dealloc_ucode_pci(priv);
>  
>   if (priv->rxq.bd)
> @@ -8181,6 +8180,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>  
>   /*netif_stop_queue(dev); */
>   flush_workqueue(priv->workqueue);
> + cancel_delayed_work_sync(&priv->rfkill_poll);
>  
>   /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
>   * priv->workqueue... so we can't take down the workqueue


--

When all other means of communication fail, try words!



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Huaxu Wan
Hi Stefan,

The patch 71d449b55abf5018d7c711b2b62abc0c083723c4 doesn't work in my test
with 2.6.28 kernel. Please double check before it's pulled in.

Thanks
Huaxu

On 15:53 Wed 10 Jun, Stefan Bader wrote:

> Hi Huaxu,
>
> thanks for the work and sorry for being so late in response. It seems, by
> now the patch made it upstream as
>
> commit 71d449b55abf5018d7c711b2b62abc0c083723c4
> Author: Reinette Chatre <[hidden email]>
> Date:   Mon Apr 20 14:37:01 2009 -0700
>
>     iwl3945: use cancel_delayed_work_sync to cancel rfkill_poll
>
>     Users reported lockup with work still trying to run
>     after module has been unloaded.
>
>     http://thread.gmane.org/gmane.linux.kernel.wireless.general/30594/focus=3060
>
>     Signed-off-by: Reinette Chatre <[hidden email]>
>     Reported-by: TJ <[hidden email]>
>     Reported-by: Huaxu Wan <[hidden email]>
>     Signed-off-by: John W. Linville <[hidden email]>
>
>
> This looks even simpler by just syncing the cancellation. I will go on
> and try to get this version into SRU. Thanks again.
>
> Stefan
>
> Huaxu Wan wrote:
>> From f3628f0ac6c9713afadaa9654815c37a294ce7e2 Mon Sep 17 00:00:00 2001
>> From: Huaxu Wan <[hidden email]>
>> Date: Fri, 8 May 2009 07:36:58 -0400
>> Subject: [PATCH] cancel rfkill_poll with sync when module is exiting
>>
>> Re-write the patch due to there is no response from TJ.
>>
>> When the the interface is down, a delayed work rfkill_poll is queued.
>> This work must be canceled before the module is unloaded, or the OS would
>> suffer a hard lock-up.
>>  This patch fix the bug LP: #345710 by make sure the queued work is
>> canceled.
>>
>>
>> Original info:
>> http://marc.info/?l=linux-wireless&m=123791044313158&w=2
>> Bug: #345710
>>
>> When the wireless interface is active and the iwl3945 module is unloaded the
>> call to ieee80211_unregister_hw() would call iwl3945_mac_stop() which would
>> restart the delayed workqueue for rfkill_poll. That workqueue had already been
>> cancelled so when the next work item was run (2 seconds later) the system would
>> suffer a hard lock-up because the module had been unloaded by then.
>>
>> This patch implements STATUS_EXIT_PENDING checks in places where the rfkill_poll
>> work is scheduled, and moves the final workqueue cancellation to occur after the
>> call to ieee80211_unregister_hw().
>>
>> Bug discovered, experienced and fix tested on my PC.
>>
>> Signed-off-by: TJ <[hidden email]>
>> Signed-off-by: Huaxu Wan <[hidden email]>
>> ---
>>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> index bb92db2..08c2a9d 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> @@ -8166,7 +8166,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>   sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
>>   iwl3945_rfkill_unregister(priv);
>> - cancel_delayed_work(&priv->rfkill_poll);
>>   iwl3945_dealloc_ucode_pci(priv);
>>   if (priv->rxq.bd)
>> @@ -8181,6 +8180,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>   /*netif_stop_queue(dev); */
>>   flush_workqueue(priv->workqueue);
>> + cancel_delayed_work_sync(&priv->rfkill_poll);
>>   /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
>>   * priv->workqueue... so we can't take down the workqueue
>
>
> --
>
> When all other means of communication fail, try words!
>
>

--
Thanks
Huaxu

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Stefan Bader-2
Huaxu Wan wrote:

> Hi Stefan,
>
> The patch 71d449b55abf5018d7c711b2b62abc0c083723c4 doesn't work in my test
> with 2.6.28 kernel. Please double check before it's pulled in.
>
> Thanks
> Huaxu
>
> On 15:53 Wed 10 Jun, Stefan Bader wrote:
>> Hi Huaxu,
>>
>> thanks for the work and sorry for being so late in response. It seems, by
>> now the patch made it upstream as
>>
>> commit 71d449b55abf5018d7c711b2b62abc0c083723c4
>> Author: Reinette Chatre <[hidden email]>
>> Date:   Mon Apr 20 14:37:01 2009 -0700
>>
>>     iwl3945: use cancel_delayed_work_sync to cancel rfkill_poll
>>
>>     Users reported lockup with work still trying to run
>>     after module has been unloaded.
>>
>>     http://thread.gmane.org/gmane.linux.kernel.wireless.general/30594/focus=3060
>>
>>     Signed-off-by: Reinette Chatre <[hidden email]>
>>     Reported-by: TJ <[hidden email]>
>>     Reported-by: Huaxu Wan <[hidden email]>
>>     Signed-off-by: John W. Linville <[hidden email]>
>>
>>
>> This looks even simpler by just syncing the cancellation. I will go on
>> and try to get this version into SRU. Thanks again.
>>
>> Stefan
>>
>> Huaxu Wan wrote:
>>> From f3628f0ac6c9713afadaa9654815c37a294ce7e2 Mon Sep 17 00:00:00 2001
>>> From: Huaxu Wan <[hidden email]>
>>> Date: Fri, 8 May 2009 07:36:58 -0400
>>> Subject: [PATCH] cancel rfkill_poll with sync when module is exiting
>>>
>>> Re-write the patch due to there is no response from TJ.
>>>
>>> When the the interface is down, a delayed work rfkill_poll is queued.
>>> This work must be canceled before the module is unloaded, or the OS would
>>> suffer a hard lock-up.
>>>  This patch fix the bug LP: #345710 by make sure the queued work is
>>> canceled.
>>>
>>>
>>> Original info:
>>> http://marc.info/?l=linux-wireless&m=123791044313158&w=2
>>> Bug: #345710
>>>
>>> When the wireless interface is active and the iwl3945 module is unloaded the
>>> call to ieee80211_unregister_hw() would call iwl3945_mac_stop() which would
>>> restart the delayed workqueue for rfkill_poll. That workqueue had already been
>>> cancelled so when the next work item was run (2 seconds later) the system would
>>> suffer a hard lock-up because the module had been unloaded by then.
>>>
>>> This patch implements STATUS_EXIT_PENDING checks in places where the rfkill_poll
>>> work is scheduled, and moves the final workqueue cancellation to occur after the
>>> call to ieee80211_unregister_hw().
>>>
>>> Bug discovered, experienced and fix tested on my PC.
>>>
>>> Signed-off-by: TJ <[hidden email]>
>>> Signed-off-by: Huaxu Wan <[hidden email]>
>>> ---
>>>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> index bb92db2..08c2a9d 100644
>>> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> @@ -8166,7 +8166,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>>   sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
>>>   iwl3945_rfkill_unregister(priv);
>>> - cancel_delayed_work(&priv->rfkill_poll);
>>>   iwl3945_dealloc_ucode_pci(priv);
>>>   if (priv->rxq.bd)
>>> @@ -8181,6 +8180,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>>   /*netif_stop_queue(dev); */
>>>   flush_workqueue(priv->workqueue);
>>> + cancel_delayed_work_sync(&priv->rfkill_poll);
>>>   /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
>>>   * priv->workqueue... so we can't take down the workqueue
>>
>> --
>>
>> When all other means of communication fail, try words!
>>
>>
>
Hi Huaxu,

thanks for the heads up. I made a test kernel and added it to the bug report. I
hope someone can/will test that and give feedback.

Stefan

--

When all other means of communication fail, try words!



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Stefan Bader-2
In reply to this post by Huaxu Wan
Huaxu Wan wrote:

> Hi Stefan,
>
> The patch 71d449b55abf5018d7c711b2b62abc0c083723c4 doesn't work in my test
> with 2.6.28 kernel. Please double check before it's pulled in.
>
> Thanks
> Huaxu
>
> On 15:53 Wed 10 Jun, Stefan Bader wrote:
>> Hi Huaxu,
>>
>> thanks for the work and sorry for being so late in response. It seems, by
>> now the patch made it upstream as
>>
>> commit 71d449b55abf5018d7c711b2b62abc0c083723c4
>> Author: Reinette Chatre <[hidden email]>
>> Date:   Mon Apr 20 14:37:01 2009 -0700
>>
>>     iwl3945: use cancel_delayed_work_sync to cancel rfkill_poll
>>
>>     Users reported lockup with work still trying to run
>>     after module has been unloaded.
>>
>>     http://thread.gmane.org/gmane.linux.kernel.wireless.general/30594/focus=3060
>>
>>     Signed-off-by: Reinette Chatre <[hidden email]>
>>     Reported-by: TJ <[hidden email]>
>>     Reported-by: Huaxu Wan <[hidden email]>
>>     Signed-off-by: John W. Linville <[hidden email]>
>>
>>
>> This looks even simpler by just syncing the cancellation. I will go on
>> and try to get this version into SRU. Thanks again.
>>
>> Stefan
>>
>> Huaxu Wan wrote:
>>> From f3628f0ac6c9713afadaa9654815c37a294ce7e2 Mon Sep 17 00:00:00 2001
>>> From: Huaxu Wan <[hidden email]>
>>> Date: Fri, 8 May 2009 07:36:58 -0400
>>> Subject: [PATCH] cancel rfkill_poll with sync when module is exiting
>>>
>>> Re-write the patch due to there is no response from TJ.
>>>
>>> When the the interface is down, a delayed work rfkill_poll is queued.
>>> This work must be canceled before the module is unloaded, or the OS would
>>> suffer a hard lock-up.
>>>  This patch fix the bug LP: #345710 by make sure the queued work is
>>> canceled.
>>>
>>>
>>> Original info:
>>> http://marc.info/?l=linux-wireless&m=123791044313158&w=2
>>> Bug: #345710
>>>
>>> When the wireless interface is active and the iwl3945 module is unloaded the
>>> call to ieee80211_unregister_hw() would call iwl3945_mac_stop() which would
>>> restart the delayed workqueue for rfkill_poll. That workqueue had already been
>>> cancelled so when the next work item was run (2 seconds later) the system would
>>> suffer a hard lock-up because the module had been unloaded by then.
>>>
>>> This patch implements STATUS_EXIT_PENDING checks in places where the rfkill_poll
>>> work is scheduled, and moves the final workqueue cancellation to occur after the
>>> call to ieee80211_unregister_hw().
>>>
>>> Bug discovered, experienced and fix tested on my PC.
>>>
>>> Signed-off-by: TJ <[hidden email]>
>>> Signed-off-by: Huaxu Wan <[hidden email]>
>>> ---
>>>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> index bb92db2..08c2a9d 100644
>>> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>>> @@ -8166,7 +8166,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>>   sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
>>>   iwl3945_rfkill_unregister(priv);
>>> - cancel_delayed_work(&priv->rfkill_poll);
>>>   iwl3945_dealloc_ucode_pci(priv);
>>>   if (priv->rxq.bd)
>>> @@ -8181,6 +8180,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>>   /*netif_stop_queue(dev); */
>>>   flush_workqueue(priv->workqueue);
>>> + cancel_delayed_work_sync(&priv->rfkill_poll);
>>>   /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
>>>   * priv->workqueue... so we can't take down the workqueue
>>
>> --
>>
>> When all other means of communication fail, try words!
>>
>>
>
Hi Huaxu,

we got now confirmed in the bug, that the upstream part alone does not fix the
problem.  prepared the following update to it. I added your signed-off for now,
is that ok? Should I send it upstream or do you want to do that?

Thanks,
Stefan


--

When all other means of communication fail, try words!



From c64c59ea22b61a2c3f64dd269812b7ae5d3331ec Mon Sep 17 00:00:00 2001
From: Stefan Bader <[hidden email]>
Date: Fri, 26 Jun 2009 09:01:33 +0200
Subject: [PATCH] iwl3945: Move cancel_delayed_work_sync to avoid lockup

BugLink: http://bugs.launchpad.net/ubuntu/+bug/345710

The issue of calling the rfkill workqueue on module remove had been tried to
solve by commit 71d449b55abf5018d7c711b2b62abc0c083723c4.
However the change from cancel_delayed_work to the sync version does not seem
to be enough to prevent the lockup. Moving the cancelation down to a later
time according to Huaxu Wan's version 2 patch.

Signed-off-by: Huaxu Wan <[hidden email]>
Signed-off-by: Stefan Bader <[hidden email]>
Tested-by: Mcarni <[hidden email]>
---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index cb9bd4c..e977af4 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -4245,8 +4245,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
 
  sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
 
- cancel_delayed_work_sync(&priv->rfkill_poll);
-
  iwl3945_dealloc_ucode_pci(priv);
 
  if (priv->rxq.bd)
@@ -4258,6 +4256,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
 
  /*netif_stop_queue(dev); */
  flush_workqueue(priv->workqueue);
+ cancel_delayed_work_sync(&priv->rfkill_poll);
 
  /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
  * priv->workqueue... so we can't take down the workqueue
--
1.5.4.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Huaxu Wan
Hi Stefan,

The patch is fine for me. It works for 2.6.28 kernel. But, this bug is
not found with 2.6.30 or above kernel. This patch is not valuable to
2.6.30+, but 2.6.28 stable. I have no idea about whether it should be
send to upstream. Please do it if you want.

Thanks
Huaxu


On 11:49 Tue 30 Jun, Stefan Bader wrote:

> Hi Huaxu,
>
> we got now confirmed in the bug, that the upstream part alone does not
> fix the problem.  prepared the following update to it. I added your
> signed-off for now, is that ok? Should I send it upstream or do you want
> to do that?
>
> Thanks,
> Stefan
>
>
> --
>
> When all other means of communication fail, try words!
>
>

> >From c64c59ea22b61a2c3f64dd269812b7ae5d3331ec Mon Sep 17 00:00:00 2001
> From: Stefan Bader <[hidden email]>
> Date: Fri, 26 Jun 2009 09:01:33 +0200
> Subject: [PATCH] iwl3945: Move cancel_delayed_work_sync to avoid lockup
>
> BugLink: http://bugs.launchpad.net/ubuntu/+bug/345710
>
> The issue of calling the rfkill workqueue on module remove had been tried to
> solve by commit 71d449b55abf5018d7c711b2b62abc0c083723c4.
> However the change from cancel_delayed_work to the sync version does not seem
> to be enough to prevent the lockup. Moving the cancelation down to a later
> time according to Huaxu Wan's version 2 patch.
>
> Signed-off-by: Huaxu Wan <[hidden email]>
> Signed-off-by: Stefan Bader <[hidden email]>
> Tested-by: Mcarni <[hidden email]>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index cb9bd4c..e977af4 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -4245,8 +4245,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>  
>   sysfs_remove_group(&pdev->dev.kobj, &iwl3945_attribute_group);
>  
> - cancel_delayed_work_sync(&priv->rfkill_poll);
> -
>   iwl3945_dealloc_ucode_pci(priv);
>  
>   if (priv->rxq.bd)
> @@ -4258,6 +4256,7 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>  
>   /*netif_stop_queue(dev); */
>   flush_workqueue(priv->workqueue);
> + cancel_delayed_work_sync(&priv->rfkill_poll);
>  
>   /* ieee80211_unregister_hw calls iwl3945_mac_stop, which flushes
>   * priv->workqueue... so we can't take down the workqueue
> --
> 1.5.4.3
>


--
Thanks
Huaxu

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Stefan Bader-2
Huaxu Wan wrote:
> Hi Stefan,
>
> The patch is fine for me. It works for 2.6.28 kernel. But, this bug is
> not found with 2.6.30 or above kernel. This patch is not valuable to
> 2.6.30+, but 2.6.28 stable. I have no idea about whether it should be
> send to upstream. Please do it if you want.
>

Hi Huaxu,

how stupid of me. I guess we can forget about moving he cancel statement. I
still have to get it verified but looking at the upstream code we got the
following patch which moves ieee80211_unregister_hw from below stopping the
rfkill_poll to above. And (though I did not follow the code completely) it
seems likely that ieee80211_unregister_hw could call iwl3945_mac_stop which
restarts the delayed work.
That patch went in after 2.6.29-rc2 and the one doing the cancel as sync after
2.6.30-rc1. So the final fix is/was with 2.6.30.
Does this sound reasonable?

Regards,
Stefan

--

When all other means of communication fail, try words!



From d552bfb65241a35d48e44ddb0d27e0454f579ab4 Mon Sep 17 00:00:00 2001
From: Kolekar, Abhijeet <[hidden email]>
Date: Fri, 19 Dec 2008 10:37:41 +0800
Subject: [PATCH] iwl3945: release resources before shutting down

Release resource before shutting down and notify upper stack.

Signed-off-by: Abhijeet Kolekar <[hidden email]>
Signed-off-by: Zhu Yi <[hidden email]>
Signed-off-by: John W. Linville <[hidden email]>
Signed-off-by: Stefan Bader <[hidden email]>
---
 drivers/net/wireless/iwlwifi/iwl3945-base.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index b845097..17f01a6 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -7722,7 +7722,12 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
 
  set_bit(STATUS_EXIT_PENDING, &priv->status);
 
- iwl3945_down(priv);
+ if (priv->mac80211_registered) {
+ ieee80211_unregister_hw(priv->hw);
+ priv->mac80211_registered = 0;
+ } else {
+ iwl3945_down(priv);
+ }
 
  /* make sure we flush any pending irq or
  * tasklet for the driver
@@ -7745,9 +7750,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
  iwl3945_unset_hw_params(priv);
  iwl3945_clear_stations_table(priv);
 
- if (priv->mac80211_registered)
- ieee80211_unregister_hw(priv->hw);
-
  /*netif_stop_queue(dev); */
  flush_workqueue(priv->workqueue);
 
--
1.5.4.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Huaxu Wan
On 11:31 Wed 01 Jul, Stefan Bader wrote:

> Huaxu Wan wrote:
>> Hi Stefan,
>>
>> The patch is fine for me. It works for 2.6.28 kernel. But, this bug is
>> not found with 2.6.30 or above kernel. This patch is not valuable to
>> 2.6.30+, but 2.6.28 stable. I have no idea about whether it should be
>> send to upstream. Please do it if you want.
>>
>
> Hi Huaxu,
>
> how stupid of me. I guess we can forget about moving he cancel statement.
> I still have to get it verified but looking at the upstream code we got
> the following patch which moves ieee80211_unregister_hw from below
> stopping the rfkill_poll to above. And (though I did not follow the code
> completely) it seems likely that ieee80211_unregister_hw could call
> iwl3945_mac_stop which restarts the delayed work.
> That patch went in after 2.6.29-rc2 and the one doing the cancel as sync
> after 2.6.30-rc1. So the final fix is/was with 2.6.30.
> Does this sound reasonable?

Hi Stefan,

I'm confused :S.

Let me clarify something. The original bug(system freezed when module exit)
was introduced by backport the patch [iwl3945: report killswitch changes
even if the interface is down(2663516d8fb896430bf42dce41b3e2f141d63bd5)]
to fix a bug about rfkill. Then the patch we are discussing fix the bug.

I once tried to reproduce the system freeze bug with 2.6.30 without the
patch, but seems it was fixed in another way.

Upstream wanted to pick the patch into 2.6.30 because it's reasonable to
cancel rfkill_poll with cancel_delayed_work_sync, I think. However, I
happened to be on vacation when the upstream guy ask for patch. Since no
response from me, they made a patch directly. But, the patch does not
fix the bug in 2.6.28.  ^_^

>
> Regards,
> Stefan
>
> --
>
> When all other means of communication fail, try words!
>
>

> >From d552bfb65241a35d48e44ddb0d27e0454f579ab4 Mon Sep 17 00:00:00 2001
> From: Kolekar, Abhijeet <[hidden email]>
> Date: Fri, 19 Dec 2008 10:37:41 +0800
> Subject: [PATCH] iwl3945: release resources before shutting down
>
> Release resource before shutting down and notify upper stack.
>
> Signed-off-by: Abhijeet Kolekar <[hidden email]>
> Signed-off-by: Zhu Yi <[hidden email]>
> Signed-off-by: John W. Linville <[hidden email]>
> Signed-off-by: Stefan Bader <[hidden email]>
> ---
>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index b845097..17f01a6 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -7722,7 +7722,12 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>  
>   set_bit(STATUS_EXIT_PENDING, &priv->status);
>  
> - iwl3945_down(priv);
> + if (priv->mac80211_registered) {
> + ieee80211_unregister_hw(priv->hw);
> + priv->mac80211_registered = 0;
> + } else {
> + iwl3945_down(priv);
> + }
>  
>   /* make sure we flush any pending irq or
>   * tasklet for the driver
> @@ -7745,9 +7750,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>   iwl3945_unset_hw_params(priv);
>   iwl3945_clear_stations_table(priv);
>  
> - if (priv->mac80211_registered)
> - ieee80211_unregister_hw(priv->hw);
> -
>   /*netif_stop_queue(dev); */
>   flush_workqueue(priv->workqueue);
>  
> --
> 1.5.4.3
>


--
Thanks
Huaxu

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [Jaunty SRU] LP#345710 [v2: description modified] iwl3945: cancel rfkill_poll with sync when?the module is exiting

Stefan Bader-2
Huaxu Wan wrote:

> On 11:31 Wed 01 Jul, Stefan Bader wrote:
>> Huaxu Wan wrote:
>>> Hi Stefan,
>>>
>>> The patch is fine for me. It works for 2.6.28 kernel. But, this bug is
>>> not found with 2.6.30 or above kernel. This patch is not valuable to
>>> 2.6.30+, but 2.6.28 stable. I have no idea about whether it should be
>>> send to upstream. Please do it if you want.
>>>
>> Hi Huaxu,
>>
>> how stupid of me. I guess we can forget about moving he cancel statement.
>> I still have to get it verified but looking at the upstream code we got
>> the following patch which moves ieee80211_unregister_hw from below
>> stopping the rfkill_poll to above. And (though I did not follow the code
>> completely) it seems likely that ieee80211_unregister_hw could call
>> iwl3945_mac_stop which restarts the delayed work.
>> That patch went in after 2.6.29-rc2 and the one doing the cancel as sync
>> after 2.6.30-rc1. So the final fix is/was with 2.6.30.
>> Does this sound reasonable?
>
> Hi Stefan,
>
> I'm confused :S.
>
> Let me clarify something. The original bug(system freezed when module exit)
> was introduced by backport the patch [iwl3945: report killswitch changes
> even if the interface is down(2663516d8fb896430bf42dce41b3e2f141d63bd5)]
> to fix a bug about rfkill. Then the patch we are discussing fix the bug.
>
> I once tried to reproduce the system freeze bug with 2.6.30 without the
> patch, but seems it was fixed in another way.
>
> Upstream wanted to pick the patch into 2.6.30 because it's reasonable to
> cancel rfkill_poll with cancel_delayed_work_sync, I think. However, I
> happened to be on vacation when the upstream guy ask for patch. Since no
> response from me, they made a patch directly. But, the patch does not
> fix the bug in 2.6.28.  ^_^
>

Hi Huaxu,

it makes sense to me. :) I try to better explain my thinking. The patch you
mention "report rfkill changes event if interface is down" added the
rfkill_poll workqueue. One part of the problem is, that it starts the worker
when the interface is going down:

iwl3945_mac_stop -> queue_delayed_work(priv->workqueue, &priv->rfkill_poll

And this is also being done from iwl3945_pci_remove through calling
ieee80211_unregister_hw which is called in 2.6.28 _after_ the rfkill_poll has
been cancelled. So it is running again. The other patch "Release resource
before shutting down and notify upper stack." not only calls only either
iwl3945_down or 80211_unregister_hw, but it also does it _before_ cancelling
the rfkill_poll. So this patch is probably the one that fixes more occurances
of this, while making the cancel call sync is another sensible step to prevent
any remaining loopholes.
Your v2 patch basically did the same because it moved the cancel call _below_
the ieee80211_unregister_hw.
I hope this un-confuses things.

Cheers,
Stefan


>> Regards,
>> Stefan
>>
>> --
>>
>> When all other means of communication fail, try words!
>>
>>
>
>> >From d552bfb65241a35d48e44ddb0d27e0454f579ab4 Mon Sep 17 00:00:00 2001
>> From: Kolekar, Abhijeet <[hidden email]>
>> Date: Fri, 19 Dec 2008 10:37:41 +0800
>> Subject: [PATCH] iwl3945: release resources before shutting down
>>
>> Release resource before shutting down and notify upper stack.
>>
>> Signed-off-by: Abhijeet Kolekar <[hidden email]>
>> Signed-off-by: Zhu Yi <[hidden email]>
>> Signed-off-by: John W. Linville <[hidden email]>
>> Signed-off-by: Stefan Bader <[hidden email]>
>> ---
>>  drivers/net/wireless/iwlwifi/iwl3945-base.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> index b845097..17f01a6 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> @@ -7722,7 +7722,12 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>  
>>   set_bit(STATUS_EXIT_PENDING, &priv->status);
>>  
>> - iwl3945_down(priv);
>> + if (priv->mac80211_registered) {
>> + ieee80211_unregister_hw(priv->hw);
>> + priv->mac80211_registered = 0;
>> + } else {
>> + iwl3945_down(priv);
>> + }
>>  
>>   /* make sure we flush any pending irq or
>>   * tasklet for the driver
>> @@ -7745,9 +7750,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev)
>>   iwl3945_unset_hw_params(priv);
>>   iwl3945_clear_stations_table(priv);
>>  
>> - if (priv->mac80211_registered)
>> - ieee80211_unregister_hw(priv->hw);
>> -
>>   /*netif_stop_queue(dev); */
>>   flush_workqueue(priv->workqueue);
>>  
>> --
>> 1.5.4.3
>>
>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team