[RFC, RFT] Re: [PATCH] d80211: make sleeping in hw->config possible #2
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 #include #include +#include #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
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
Re: [RFC, RFT] Re: [PATCH] d80211: make sleeping in hw->config possible #2
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