Re: [RFC, RFT] Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-18 Thread Michael Buesch 
On Tuesday 18 July 2006 18:57, Jiri Benc wrote:
 On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
  This patch makes sleeping in the hw-config callback possible
  by removing the only atomic caller. The atomic caller was
  a timer and is replaced by a workqueue.
 
 This is a modified version of the patch that doesn't use
 cancel_rearming_delayed_work.
 
 Signed-off-by: Jiri Benc [EMAIL PROTECTED]
 
 ---
  net/d80211/ieee80211.c   |   23 +++
  net/d80211/ieee80211_i.h |3 ++-
  net/d80211/ieee80211_iface.c |   10 --
  net/d80211/ieee80211_sta.c   |   37 +
  4 files changed, 42 insertions(+), 31 deletions(-)
 
 --- dscape.orig/net/d80211/ieee80211.c
 +++ dscape/net/d80211/ieee80211.c
 @@ -4552,14 +4552,6 @@ void ieee80211_unregister_hw(struct net_
  tasklet_disable(local-tasklet);
  /* TODO: skb_queue should be empty here, no need to do anything? */
  
 - if (local-rate_limit)
 - del_timer_sync(local-rate_limit_timer);
 - if (local-stat_time)
 - del_timer_sync(local-stat_timer);
 - if (local-scan_timer.data)
 - del_timer_sync(local-scan_timer);
 - ieee80211_rx_bss_list_deinit(dev);
 -
   rtnl_lock();
   local-reg_state = IEEE80211_DEV_UNREGISTERED;
   if (local-apdev)
 @@ -4572,6 +4564,21 @@ void ieee80211_unregister_hw(struct net_
   }
   rtnl_unlock();
  
 + if (local-rate_limit)
 + del_timer_sync(local-rate_limit_timer);
 + if (local-stat_time)
 + del_timer_sync(local-stat_timer);
 + if (local-scan_work.data) {
 + local-sta_scanning = 0;
 + cancel_delayed_work(local-scan_work);
 + flush_scheduled_work();
 + /* The scan_work is guaranteed not to be called at this
 +  * point. It is not scheduled and not running now. It can be

If it is guaranteed to be not to be called, the
cancel_delayed_work(local-scan_work);
is unnecessary.
If it is possible to be scheduled and delayed here,
it's a bug.

So, if the first is true, we should remove it and if the second
is true, well :)

The flush_scheduled_work() call is necessary, though.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC, RFT] Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-18 Thread Jiri Benc 
On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
 This patch makes sleeping in the hw-config callback possible
 by removing the only atomic caller. The atomic caller was
 a timer and is replaced by a workqueue.

This is a modified version of the patch that doesn't use
cancel_rearming_delayed_work.

Signed-off-by: Jiri Benc [EMAIL PROTECTED]

---
 net/d80211/ieee80211.c   |   23 +++
 net/d80211/ieee80211_i.h |3 ++-
 net/d80211/ieee80211_iface.c |   10 --
 net/d80211/ieee80211_sta.c   |   37 +
 4 files changed, 42 insertions(+), 31 deletions(-)

--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -4552,14 +4552,6 @@ void ieee80211_unregister_hw(struct net_
 tasklet_disable(local-tasklet);
 /* TODO: skb_queue should be empty here, no need to do anything? */
 
-   if (local-rate_limit)
-   del_timer_sync(local-rate_limit_timer);
-   if (local-stat_time)
-   del_timer_sync(local-stat_timer);
-   if (local-scan_timer.data)
-   del_timer_sync(local-scan_timer);
-   ieee80211_rx_bss_list_deinit(dev);
-
rtnl_lock();
local-reg_state = IEEE80211_DEV_UNREGISTERED;
if (local-apdev)
@@ -4572,6 +4564,21 @@ void ieee80211_unregister_hw(struct net_
}
rtnl_unlock();
 
+   if (local-rate_limit)
+   del_timer_sync(local-rate_limit_timer);
+   if (local-stat_time)
+   del_timer_sync(local-stat_timer);
+   if (local-scan_work.data) {
+   local-sta_scanning = 0;
+   cancel_delayed_work(local-scan_work);
+   flush_scheduled_work();
+   /* The scan_work is guaranteed not to be called at this
+* point. It is not scheduled and not running now. It can be
+* scheduled again only by some sta_timer (all of them are
+* stopped by now) or under rtnl lock. */
+   }
+
+   ieee80211_rx_bss_list_deinit(dev);
ieee80211_clear_tx_pending(local);
sta_info_stop(local);
rate_control_remove_attrs(local, local-rate_ctrl_priv,
--- dscape.orig/net/d80211/ieee80211_i.h
+++ dscape/net/d80211/ieee80211_i.h
@@ -17,6 +17,7 @@
 #include linux/list.h
 #include linux/netdevice.h
 #include linux/skbuff.h
+#include linux/workqueue.h
 #include ieee80211_key.h
 #include sta_info.h
 
@@ -430,7 +431,7 @@ struct ieee80211_local {
int scan_channel_idx;
enum { SCAN_SET_CHANNEL, SCAN_SEND_PROBE } scan_state;
unsigned long last_scan_completed;
-   struct timer_list scan_timer;
+   struct work_struct scan_work;
int scan_oper_channel;
int scan_oper_channel_val;
int scan_oper_power_level;
--- dscape.orig/net/d80211/ieee80211_iface.c
+++ dscape/net/d80211/ieee80211_iface.c
@@ -287,8 +287,14 @@ void ieee80211_if_reinit(struct net_devi
case IEEE80211_IF_TYPE_STA:
case IEEE80211_IF_TYPE_IBSS:
del_timer_sync(sdata-u.sta.timer);
-   if (local-scan_timer.data == (unsigned long) sdata-dev)
-   del_timer_sync(local-scan_timer);
+   if (local-scan_work.data == sdata-dev) {
+   local-sta_scanning = 0;
+   cancel_delayed_work(local-scan_work);
+   flush_scheduled_work();
+   /* see comment in ieee80211_unregister_hw to
+* understand why this works */
+   local-scan_work.data = NULL;
+   }
kfree(sdata-u.sta.extra_ie);
sdata-u.sta.extra_ie = NULL;
kfree(sdata-u.sta.assocreq_ies);
--- dscape.orig/net/d80211/ieee80211_sta.c
+++ dscape/net/d80211/ieee80211_sta.c
@@ -2417,15 +2417,16 @@ static int ieee80211_active_scan(struct 
 }
 
 
-static void ieee80211_sta_scan_timer(unsigned long ptr)
+static void ieee80211_sta_scan_work(void *ptr)
 {
-   struct net_device *dev = (struct net_device *) ptr;
+   struct net_device *dev = ptr;
struct ieee80211_local *local = dev-ieee80211_ptr;
 struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_hw_modes *mode;
struct ieee80211_channel *chan;
int skip;
union iwreq_data wrqu;
+   unsigned long next_delay = 0;
 
if (!local-sta_scanning)
return;
@@ -2493,31 +2494,30 @@ static void ieee80211_sta_scan_timer(uns
local-scan_channel_idx = 0;
}
 
-   if (skip) {
-   local-scan_timer.expires = jiffies;
+   if (skip)
break;
-   }
 
-   local-scan_timer.expires =
-   jiffies + IEEE80211_PROBE_DELAY +
-   usecs_to_jiffies(local-hw-channel_change_time);
+   next_delay = IEEE80211_PROBE_DELAY

Re: [RFC, RFT] Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-18 Thread Michael Buesch 
On Tuesday 18 July 2006 19:36, Michael Buesch wrote:
 On Tuesday 18 July 2006 18:57, Jiri Benc wrote:
  On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
   This patch makes sleeping in the hw-config callback possible
   by removing the only atomic caller. The atomic caller was
   a timer and is replaced by a workqueue.
  
  This is a modified version of the patch that doesn't use
  cancel_rearming_delayed_work.
  
  Signed-off-by: Jiri Benc [EMAIL PROTECTED]
  
  ---
   net/d80211/ieee80211.c   |   23 +++
   net/d80211/ieee80211_i.h |3 ++-
   net/d80211/ieee80211_iface.c |   10 --
   net/d80211/ieee80211_sta.c   |   37 +
   4 files changed, 42 insertions(+), 31 deletions(-)
  
  --- dscape.orig/net/d80211/ieee80211.c
  +++ dscape/net/d80211/ieee80211.c
  @@ -4552,14 +4552,6 @@ void ieee80211_unregister_hw(struct net_
   tasklet_disable(local-tasklet);
   /* TODO: skb_queue should be empty here, no need to do anything? */
   
  -   if (local-rate_limit)
  -   del_timer_sync(local-rate_limit_timer);
  -   if (local-stat_time)
  -   del_timer_sync(local-stat_timer);
  -   if (local-scan_timer.data)
  -   del_timer_sync(local-scan_timer);
  -   ieee80211_rx_bss_list_deinit(dev);
  -
  rtnl_lock();
  local-reg_state = IEEE80211_DEV_UNREGISTERED;
  if (local-apdev)
  @@ -4572,6 +4564,21 @@ void ieee80211_unregister_hw(struct net_
  }
  rtnl_unlock();
   
  +   if (local-rate_limit)
  +   del_timer_sync(local-rate_limit_timer);
  +   if (local-stat_time)
  +   del_timer_sync(local-stat_timer);
  +   if (local-scan_work.data) {
  +   local-sta_scanning = 0;
  +   cancel_delayed_work(local-scan_work);
  +   flush_scheduled_work();
  +   /* The scan_work is guaranteed not to be called at this
  +* point. It is not scheduled and not running now. It can be
 
 If it is guaranteed to be not to be called, the
 cancel_delayed_work(local-scan_work);
 is unnecessary.
 If it is possible to be scheduled and delayed here,
 it's a bug.
 
 So, if the first is true, we should remove it and if the second
 is true, well :)
 
 The flush_scheduled_work() call is necessary, though.

Oh, no. I misread the code.
I think it's ok. Sorry for the noise :(

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-12 Thread Jiri Benc 
On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
 Please apply this to wireless-dev.
 Note that this is the second try to submit this patch.
 The first try contained a little bug. I'm sorry for that.
 If you already applied the first one, I can provide an incremental patch.
 
 Note2 that this patch depends on the
 [PATCH] cancel_rearming_delayed_work infinite loop fix
 I just sent out to the lists and akpm.

I'm still digging through this. But I think it is not necessary to use
cancel_rearming_delayed_work at all; will try to make a patch.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-12 Thread Michael Buesch 
On Wednesday 12 July 2006 18:53, Jiri Benc wrote:
 On Tue, 11 Jul 2006 00:54:33 +0200, Michael Buesch wrote:
  Please apply this to wireless-dev.
  Note that this is the second try to submit this patch.
  The first try contained a little bug. I'm sorry for that.
  If you already applied the first one, I can provide an incremental patch.
  
  Note2 that this patch depends on the
  [PATCH] cancel_rearming_delayed_work infinite loop fix
  I just sent out to the lists and akpm.
 
 I'm still digging through this. But I think it is not necessary to use
 cancel_rearming_delayed_work at all; will try to make a patch.

Jiri, so you care to get that patch upstream?
Would be cool, thanks.

John, do you manage to apply the bcm43xx init fix patch, which
depends on this one, to the tree after this one got in?
Or should I resend it?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-11 Thread Michael Buesch 
On Tuesday 11 July 2006 06:25, you wrote:
 On Tue, 11 Jul 2006 00:54:33 +0200
 Michael Buesch [EMAIL PROTECTED] wrote:
 
  Please apply this to wireless-dev.
  Note that this is the second try to submit this patch.
  The first try contained a little bug. I'm sorry for that.
  If you already applied the first one, I can provide an incremental patch.
  
  Note2 that this patch depends on the
  [PATCH] cancel_rearming_delayed_work infinite loop fix
  I just sent out to the lists and akpm.
 
 Am still scratching my head over that.  I wouldn't really call it a fix. 
 More an enhcancement to cover unanticipated (and arguably strange) usage.
 
 It's odd to call cancel_rearming_delayed_work() against a rearming
 workqueue which isn't actually running.  It tends to indicate that the
 caller has lost track of what it's up to.

No, I don't think so.
Let's say we have the following scenario:
A wq reschedules itself x times after it was scheduled once from outside.
After these x times, it does not reschedule anymore. That's what happens
in d80211. And I don't see a solution to sync this, other than modifying
the function, because we may call it after the x reschedule times.
Or do you think we should really do a statemachine to workaround it?

I am not the first one to hit this (I call it) bug.
It is _very_ confusing to see this sync function blocking forever.
I saw several people complaining about it. Also on #kernelnewbies.

 But as a convenience thing I guess it's an OK thing to do.  I need to stare
 at the implementation for a bit longer - that stuff's tricky.

Actually, I think there's still a little race.
I will send a more complex fix for this, if you agree to change the function.
If you say no, we don't fix this. Insert a statemachine or something in your
code instead, I can use the time for better things. :)

But I think the following is also broken in the old code:
A wq is not pending anymore, but just executing (before it reschedules itself).
I think that would also loop forever. I don't think that's what we want.
Because we can't really keep track of _this_.

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-11 Thread Andrew Morton 
On Tue, 11 Jul 2006 11:11:27 +0200
Michael Buesch [EMAIL PROTECTED] wrote:

 But I think the following is also broken in the old code:
 A wq is not pending anymore, but just executing (before it reschedules 
 itself).
 I think that would also loop forever. I don't think that's what we want.
 Because we can't really keep track of _this_.

The present implementation assumes that the handler will re-arm itself.

I agree that extending that makes sense.  But beware that it's easy to
leave subtle holes in this logic.  Needs careful thought to get right.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-11 Thread Michael Buesch 
On Tuesday 11 July 2006 11:31, you wrote:
 On Tue, 11 Jul 2006 11:11:27 +0200
 Michael Buesch [EMAIL PROTECTED] wrote:
 
  But I think the following is also broken in the old code:
  A wq is not pending anymore, but just executing (before it reschedules 
  itself).
  I think that would also loop forever. I don't think that's what we want.
  Because we can't really keep track of _this_.
 
 The present implementation assumes that the handler will re-arm itself.
 
 I agree that extending that makes sense.  But beware that it's easy to
 leave subtle holes in this logic.  Needs careful thought to get right.

Yeah, as I said. There is still a race.
Should I redo the patch to fix it?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] d80211: make sleeping in hw-config possible #2

2006-07-10 Thread Michael Buesch 
Hi John,

Please apply this to wireless-dev.
Note that this is the second try to submit this patch.
The first try contained a little bug. I'm sorry for that.
If you already applied the first one, I can provide an incremental patch.

Note2 that this patch depends on the
[PATCH] cancel_rearming_delayed_work infinite loop fix
I just sent out to the lists and akpm.

--

This patch makes sleeping in the hw-config callback possible
by removing the only atomic caller. The atomic caller was
a timer and is replaced by a workqueue.

In general, allowing to sleep in the config callback is a
good thing. bcm43xx must be able to sleep here, as it is
required to lock a mutex.
But there are other good reasons to sleep here. We might
want to sleep for a grace period on channel switch, for example.

Signed-off-by: Michael Buesch [EMAIL PROTECTED]

Index: wireless-dev-dscapeports/net/d80211/ieee80211.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211.c2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211.c 2006-07-11 
00:53:44.0 +0200
@@ -4327,8 +4327,8 @@
del_timer_sync(local-rate_limit_timer);
if (local-stat_time)
del_timer_sync(local-stat_timer);
-   if (local-scan_timer.data)
-   del_timer_sync(local-scan_timer);
+   if (local-scan_work.data)
+   cancel_rearming_delayed_work(local-scan_work);
ieee80211_rx_bss_list_deinit(dev);
 
rtnl_lock();
Index: wireless-dev-dscapeports/net/d80211/ieee80211_i.h
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_i.h  2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_i.h   2006-07-09 
19:52:07.0 +0200
@@ -17,6 +17,7 @@
 #include linux/list.h
 #include linux/netdevice.h
 #include linux/skbuff.h
+#include linux/workqueue.h
 #include ieee80211_key.h
 #include sta_info.h
 
@@ -407,7 +408,7 @@
int scan_channel_idx;
enum { SCAN_SET_CHANNEL, SCAN_SEND_PROBE } scan_state;
unsigned long last_scan_completed;
-   struct timer_list scan_timer;
+   struct work_struct scan_work;
int scan_oper_channel;
int scan_oper_channel_val;
int scan_oper_power_level;
Index: wireless-dev-dscapeports/net/d80211/ieee80211_iface.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_iface.c  2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_iface.c   2006-07-09 
20:03:32.0 +0200
@@ -271,8 +271,8 @@
case IEEE80211_IF_TYPE_STA:
case IEEE80211_IF_TYPE_IBSS:
del_timer_sync(sdata-u.sta.timer);
-   if (local-scan_timer.data == (unsigned long) sdata-dev)
-   del_timer_sync(local-scan_timer);
+   if (local-scan_work.data == sdata-dev)
+   cancel_rearming_delayed_work(local-scan_work);
kfree(sdata-u.sta.extra_ie);
sdata-u.sta.extra_ie = NULL;
kfree(sdata-u.sta.assocreq_ies);
Index: wireless-dev-dscapeports/net/d80211/ieee80211_sta.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_sta.c2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_sta.c 2006-07-09 
20:21:44.0 +0200
@@ -2422,15 +2422,16 @@
 }
 
 
-static void ieee80211_sta_scan_timer(unsigned long ptr)
+static void ieee80211_sta_scan_work(void *_data)
 {
-   struct net_device *dev = (struct net_device *) ptr;
+   struct net_device *dev = _data;
struct ieee80211_local *local = dev-ieee80211_ptr;
 struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_hw_modes *mode;
struct ieee80211_channel *chan;
int skip;
union iwreq_data wrqu;
+   unsigned long next_delay = 0;
 
if (!local-sta_scanning)
return;
@@ -2498,29 +2499,27 @@
local-scan_channel_idx = 0;
}
 
-   if (skip) {
-   local-scan_timer.expires = jiffies;
+   if (skip)
break;
-   }
 
-   local-scan_timer.expires = jiffies + IEEE80211_PROBE_DELAY;
+   next_delay = IEEE80211_PROBE_DELAY;
local-scan_state = SCAN_SEND_PROBE;
break;
case SCAN_SEND_PROBE:
if (ieee80211_active_scan(local)) {
ieee80211_send_probe_req(dev, NULL, local-scan_ssid,
 local-scan_ssid_len);
-   local-scan_timer.expires =
-   jiffies + IEEE80211_CHANNEL_TIME;
-

Re: [PATCH] d80211: make sleeping in hw-config possible #2

2006-07-10 Thread Andrew Morton 
On Tue, 11 Jul 2006 00:54:33 +0200
Michael Buesch [EMAIL PROTECTED] wrote:

 Please apply this to wireless-dev.
 Note that this is the second try to submit this patch.
 The first try contained a little bug. I'm sorry for that.
 If you already applied the first one, I can provide an incremental patch.
 
 Note2 that this patch depends on the
 [PATCH] cancel_rearming_delayed_work infinite loop fix
 I just sent out to the lists and akpm.

Am still scratching my head over that.  I wouldn't really call it a fix. 
More an enhcancement to cover unanticipated (and arguably strange) usage.

It's odd to call cancel_rearming_delayed_work() against a rearming
workqueue which isn't actually running.  It tends to indicate that the
caller has lost track of what it's up to.

But as a convenience thing I guess it's an OK thing to do.  I need to stare
at the implementation for a bit longer - that stuff's tricky.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] d80211: make sleeping in hw-config possible

2006-07-09 Thread Michael Buesch 
Hi John,

Please apply this to wireless-dev.

--

This patch makes sleeping in the hw-config callback possible
by removing the only atomic caller. The atomic caller was
a timer and is replaced by a workqueue.

In general, allowing to sleep in the config callback is a
good thing. bcm43xx must be able to sleep here, as it is
required to lock a mutex.
But there are other good reasons to sleep here. We might
want to sleep for a grace period on channel switch, for example.

Signed-off-by: Michael Buesch [EMAIL PROTECTED]

Index: wireless-dev-dscapeports/net/d80211/ieee80211.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211.c2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211.c 2006-07-09 
20:01:42.0 +0200
@@ -4327,8 +4327,7 @@
del_timer_sync(local-rate_limit_timer);
if (local-stat_time)
del_timer_sync(local-stat_timer);
-   if (local-scan_timer.data)
-   del_timer_sync(local-scan_timer);
+   cancel_rearming_delayed_work(local-scan_work);
ieee80211_rx_bss_list_deinit(dev);
 
rtnl_lock();
Index: wireless-dev-dscapeports/net/d80211/ieee80211_i.h
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_i.h  2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_i.h   2006-07-09 
19:52:07.0 +0200
@@ -17,6 +17,7 @@
 #include linux/list.h
 #include linux/netdevice.h
 #include linux/skbuff.h
+#include linux/workqueue.h
 #include ieee80211_key.h
 #include sta_info.h
 
@@ -407,7 +408,7 @@
int scan_channel_idx;
enum { SCAN_SET_CHANNEL, SCAN_SEND_PROBE } scan_state;
unsigned long last_scan_completed;
-   struct timer_list scan_timer;
+   struct work_struct scan_work;
int scan_oper_channel;
int scan_oper_channel_val;
int scan_oper_power_level;
Index: wireless-dev-dscapeports/net/d80211/ieee80211_iface.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_iface.c  2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_iface.c   2006-07-09 
20:03:32.0 +0200
@@ -271,8 +271,8 @@
case IEEE80211_IF_TYPE_STA:
case IEEE80211_IF_TYPE_IBSS:
del_timer_sync(sdata-u.sta.timer);
-   if (local-scan_timer.data == (unsigned long) sdata-dev)
-   del_timer_sync(local-scan_timer);
+   if (local-scan_work.data == sdata-dev)
+   cancel_rearming_delayed_work(local-scan_work);
kfree(sdata-u.sta.extra_ie);
sdata-u.sta.extra_ie = NULL;
kfree(sdata-u.sta.assocreq_ies);
Index: wireless-dev-dscapeports/net/d80211/ieee80211_sta.c
===
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211_sta.c2006-06-17 
21:26:10.0 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_sta.c 2006-07-09 
20:21:44.0 +0200
@@ -2422,15 +2422,16 @@
 }
 
 
-static void ieee80211_sta_scan_timer(unsigned long ptr)
+static void ieee80211_sta_scan_work(void *_data)
 {
-   struct net_device *dev = (struct net_device *) ptr;
+   struct net_device *dev = _data;
struct ieee80211_local *local = dev-ieee80211_ptr;
 struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_hw_modes *mode;
struct ieee80211_channel *chan;
int skip;
union iwreq_data wrqu;
+   unsigned long next_delay = 0;
 
if (!local-sta_scanning)
return;
@@ -2498,29 +2499,27 @@
local-scan_channel_idx = 0;
}
 
-   if (skip) {
-   local-scan_timer.expires = jiffies;
+   if (skip)
break;
-   }
 
-   local-scan_timer.expires = jiffies + IEEE80211_PROBE_DELAY;
+   next_delay = IEEE80211_PROBE_DELAY;
local-scan_state = SCAN_SEND_PROBE;
break;
case SCAN_SEND_PROBE:
if (ieee80211_active_scan(local)) {
ieee80211_send_probe_req(dev, NULL, local-scan_ssid,
 local-scan_ssid_len);
-   local-scan_timer.expires =
-   jiffies + IEEE80211_CHANNEL_TIME;
-   } else {
-   local-scan_timer.expires =
-   jiffies + IEEE80211_PASSIVE_CHANNEL_TIME;
-   }
+   next_delay = IEEE80211_CHANNEL_TIME;
+   } else
+   next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
local-scan_state =

Re: [PATCH] d80211: make sleeping in hw-config possible

2006-07-09 Thread Ivo Van Doorn 

Hi,


Please apply this to wireless-dev.

--

This patch makes sleeping in the hw-config callback possible
by removing the only atomic caller. The atomic caller was
a timer and is replaced by a workqueue.

In general, allowing to sleep in the config callback is a
good thing. bcm43xx must be able to sleep here, as it is
required to lock a mutex.
But there are other good reasons to sleep here. We might
want to sleep for a grace period on channel switch, for example.


Excellent work.
Sleeping in hw-config is a much desired feature for rt2x00 as well,
this is especially beneficial for the USB drivers who always need sleeping.

Ivo
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] d80211: make sleeping in hw-config possible

2006-07-09 Thread Michael Buesch 
On Sunday 09 July 2006 20:47, you wrote:
 ===
 --- wireless-dev-dscapeports.orig/net/d80211/ieee80211.c  2006-06-17 
 21:26:10.0 +0200
 +++ wireless-dev-dscapeports/net/d80211/ieee80211.c   2006-07-09 
 20:01:42.0 +0200
 @@ -4327,8 +4327,7 @@
   del_timer_sync(local-rate_limit_timer);
   if (local-stat_time)
   del_timer_sync(local-stat_timer);
 - if (local-scan_timer.data)
 - del_timer_sync(local-scan_timer);
 + cancel_rearming_delayed_work(local-scan_work);
   ieee80211_rx_bss_list_deinit(dev);
  
   rtnl_lock();

There seems to be a problem. It sometimes loops inside of
cancel_rearming_delayed_work forever. So I tried to fix it like this:
if (local-scan_work.data)
cancel_rearming_delayed_work(local-scan_work);
This fixes the case where we don't scan (have no STA), so don't
initialize the work struct.

But it still loops infinite, sometimes.
Any suggestions?

-- 
Greetings Michael.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html