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

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

[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 <mb@bu3sch.de>

Index: wireless-dev-dscapeports/net/d80211/ieee80211.c
===================================================================
--- wireless-dev-dscapeports.orig/net/d80211/ieee80211.c	2006-06-17 21:26:10.000000000 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211.c	2006-07-11 00:53:44.000000000 +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.000000000 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_i.h	2006-07-09 19:52:07.000000000 +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.000000000 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_iface.c	2006-07-09 20:03:32.000000000 +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.c	2006-06-17 21:26:10.000000000 +0200
+++ wireless-dev-dscapeports/net/d80211/ieee80211_sta.c	2006-07-09 20:21:44.000000000 +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 = SCAN_SET_CHANNEL;
 		break;
 	}
 
-	add_timer(&local->scan_timer);
+	if (next_delay)
+		schedule_delayed_work(&local->scan_work, next_delay);
+	else
+		schedule_work(&local->scan_work);
 }
 
 
@@ -2569,11 +2568,8 @@
 	local->scan_state = SCAN_SET_CHANNEL;
 	local->scan_hw_mode_idx = 0;
 	local->scan_channel_idx = 0;
-	init_timer(&local->scan_timer);
-	local->scan_timer.data = (unsigned long) dev;
-	local->scan_timer.function = ieee80211_sta_scan_timer;
-	local->scan_timer.expires = jiffies + 1;
-	add_timer(&local->scan_timer);
+	INIT_WORK(&local->scan_work, ieee80211_sta_scan_work, dev);
+	schedule_work(&local->scan_work);
 
 	return 0;
 }

-- 
Greetings Michael.

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

[Andrew Morton]

On Tue, 11 Jul 2006 00:54:33 +0200
Michael Buesch <mb@bu3sch.de> 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.

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

[Michael Buesch]

On Tuesday 11 July 2006 06:25, you wrote:
> On Tue, 11 Jul 2006 00:54:33 +0200
> Michael Buesch <mb@bu3sch.de> 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.

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

[Andrew Morton]

On Tue, 11 Jul 2006 11:11:27 +0200
Michael Buesch <mb@bu3sch.de> 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.

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

[Michael Buesch]

On Tuesday 11 July 2006 11:31, you wrote:
> On Tue, 11 Jul 2006 11:11:27 +0200
> Michael Buesch <mb@bu3sch.de> 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.

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

[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

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

[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.

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

[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 <jbenc@suse.cz>

---
 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 +
+			     usecs_to_jiffies(local->hw->channel_change_time);
 		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 = SCAN_SET_CHANNEL;
 		break;
 	}
 
-	add_timer(&local->scan_timer);
+	if (local->sta_scanning) {
+		if (next_delay)
+			schedule_delayed_work(&local->scan_work, next_delay);
+		else
+			schedule_work(&local->scan_work);
+	}
 }
 
 
@@ -2566,11 +2566,8 @@ int ieee80211_sta_req_scan(struct net_de
 	local->scan_state = SCAN_SET_CHANNEL;
 	local->scan_hw_mode_idx = 0;
 	local->scan_channel_idx = 0;
-	init_timer(&local->scan_timer);
-	local->scan_timer.data = (unsigned long) dev;
-	local->scan_timer.function = ieee80211_sta_scan_timer;
-	local->scan_timer.expires = jiffies + 1;
-	add_timer(&local->scan_timer);
+	INIT_WORK(&local->scan_work, ieee80211_sta_scan_work, dev);
+	schedule_work(&local->scan_work);
 
 	return 0;
 }


-- 
Jiri Benc
SUSE Labs

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

[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 <jbenc-AlSwsSmVLrQ@public.gmane.org>
> 
> ---
>  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.

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

[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 <jbenc@suse.cz>
> > 
> > ---
> >  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.