[PATCH] mac80211: Add RTNL warning for workqueue

[PATCH] mac80211: Add RTNL warning for workqueue

[Ivo van Doorn]

The workqueue provided by mac80211 should not be used for
scheduled tasks that acquire the RTNL lock. This could be done
when the driver uses the function ieee80211_iterate_active_interfaces()
within the scheduled work. Such behavior will end in locking
dependencies problems when an interface is being removed.

This patch will add a notification about the RTNL locking and
the mac80211 workqueue to prevent driver developers from
blindly using it.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
---
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 19f1e41..02c79e6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -738,8 +738,11 @@ enum ieee80211_hw_flags {
  * @conf: &struct ieee80211_conf, device configuration, don't use.
  *
  * @workqueue: single threaded workqueue available for driver use,
- *	allocated by mac80211 on registration and flushed on
- *	unregistration.
+ *	allocated by mac80211 on registration and flushed when an
+ *	interface is removed.
+ *	NOTICE: All work performed on this workqueue should NEVER
+ *	acquire the RTNL lock (i.e. Don't use the function
+ *	ieee80211_iterate_active_interfaces())
  *
  * @priv: pointer to private area that was allocated for driver use
  *	along with this structure.

Re: [PATCH] mac80211: Add RTNL warning for workqueue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

On Tue, 2008-06-24 at 19:23 +0200, Ivo van Doorn wrote:
> The workqueue provided by mac80211 should not be used for
> scheduled tasks that acquire the RTNL lock. This could be done
> when the driver uses the function ieee80211_iterate_active_interfaces()
> within the scheduled work. Such behavior will end in locking
> dependencies problems when an interface is being removed.
> 
> This patch will add a notification about the RTNL locking and
> the mac80211 workqueue to prevent driver developers from
> blindly using it.
> 
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> ---
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 19f1e41..02c79e6 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -738,8 +738,11 @@ enum ieee80211_hw_flags {
>   * @conf: &struct ieee80211_conf, device configuration, don't use.
>   *
>   * @workqueue: single threaded workqueue available for driver use,
> - *	allocated by mac80211 on registration and flushed on
> - *	unregistration.
> + *	allocated by mac80211 on registration and flushed when an
> + *	interface is removed.

Interesting point. Should it be there or rather be moved down a bit so
that it is actually only flushed when _all_ interfaces are stopped, i.e.
before ->stop is invoked?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: Add RTNL warning for workqueue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

On Tue, 2008-06-24 at 19:28 +0200, Johannes Berg wrote:
> On Tue, 2008-06-24 at 19:23 +0200, Ivo van Doorn wrote:
> > The workqueue provided by mac80211 should not be used for
> > scheduled tasks that acquire the RTNL lock. This could be done
> > when the driver uses the function ieee80211_iterate_active_interfaces()
> > within the scheduled work. Such behavior will end in locking
> > dependencies problems when an interface is being removed.
> > 
> > This patch will add a notification about the RTNL locking and
> > the mac80211 workqueue to prevent driver developers from
> > blindly using it.
> > 
> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> > Cc: Johannes Berg <johannes@sipsolutions.net>

> Interesting point. Should it be there or rather be moved down a bit so
> that it is actually only flushed when _all_ interfaces are stopped, i.e.
> before ->stop is invoked?

Either way though, the patch is obviously correct and a follow-up patch
can change the behaviour if we want that, so for this one:

Acked-by: Johannes Berg <johannes@sipsolutions.net>

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: Add RTNL warning for workqueue

[Ivo van Doorn]

On Tuesday 24 June 2008, Johannes Berg wrote:
> On Tue, 2008-06-24 at 19:23 +0200, Ivo van Doorn wrote:
> > The workqueue provided by mac80211 should not be used for
> > scheduled tasks that acquire the RTNL lock. This could be done
> > when the driver uses the function ieee80211_iterate_active_interfaces()
> > within the scheduled work. Such behavior will end in locking
> > dependencies problems when an interface is being removed.
> > 
> > This patch will add a notification about the RTNL locking and
> > the mac80211 workqueue to prevent driver developers from
> > blindly using it.
> > 
> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > ---
> > diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> > index 19f1e41..02c79e6 100644
> > --- a/include/net/mac80211.h
> > +++ b/include/net/mac80211.h
> > @@ -738,8 +738,11 @@ enum ieee80211_hw_flags {
> >   * @conf: &struct ieee80211_conf, device configuration, don't use.
> >   *
> >   * @workqueue: single threaded workqueue available for driver use,
> > - *	allocated by mac80211 on registration and flushed on
> > - *	unregistration.
> > + *	allocated by mac80211 on registration and flushed when an
> > + *	interface is removed.
> 
> Interesting point. Should it be there or rather be moved down a bit so
> that it is actually only flushed when _all_ interfaces are stopped, i.e.
> before ->stop is invoked?

Personally I think it makes most sense when all interfaces are gone,
and thus either before or after ->stop(). But I am not sure which
drivers currently rely on the current position of flush_workqueue().

Ivo