rt2x00: Fix lock dependency errror
authorIvo van Doorn <ivdoorn@gmail.com>
Wed, 25 Jun 2008 19:27:00 +0000 (21:27 +0200)
committerJohn W. Linville <linville@tuxdriver.com>
Fri, 27 Jun 2008 18:49:52 +0000 (14:49 -0400)
This fixes a circular locking dependency in the workqueue handling.
The interface work task uses the mac80211 function
ieee80211_iterate_active_interfaces() which grabs the RTNL lock.

However when the interface is brough down, this happens under the RTNL
lock as well, this causes problems because mac80211 will flush the workqueue
during the ifdown event. This causes mac80211 to wait until the driver has
completed all work which can't finish because it is waiting on the RTNL lock.

This is fixed by moving rt2x00 workqueue tasks on a different workqueue,
this workqueue can be flushed when the ieee80211_hw structure is removed
by the driver (when the driver is unloaded) which does not happen under the
RTNL lock.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
drivers/net/wireless/rt2x00/rt2x00.h
drivers/net/wireless/rt2x00/rt2x00dev.c
drivers/net/wireless/rt2x00/rt2x00mac.c

index 611d983..b4bf1e0 100644 (file)
@@ -821,6 +821,7 @@ struct rt2x00_dev {
        /*
         * Scheduled work.
         */
+       struct workqueue_struct *workqueue;
        struct work_struct intf_work;
        struct work_struct filter_work;
 
index 2673d56..c997d4f 100644 (file)
@@ -75,7 +75,7 @@ static void rt2x00lib_start_link_tuner(struct rt2x00_dev *rt2x00dev)
 
        rt2x00lib_reset_link_tuner(rt2x00dev);
 
-       queue_delayed_work(rt2x00dev->hw->workqueue,
+       queue_delayed_work(rt2x00dev->workqueue,
                           &rt2x00dev->link.work, LINK_TUNE_INTERVAL);
 }
 
@@ -137,14 +137,6 @@ void rt2x00lib_disable_radio(struct rt2x00_dev *rt2x00dev)
                return;
 
        /*
-        * Stop all scheduled work.
-        */
-       if (work_pending(&rt2x00dev->intf_work))
-               cancel_work_sync(&rt2x00dev->intf_work);
-       if (work_pending(&rt2x00dev->filter_work))
-               cancel_work_sync(&rt2x00dev->filter_work);
-
-       /*
         * Stop the TX queues.
         */
        ieee80211_stop_queues(rt2x00dev->hw);
@@ -398,8 +390,8 @@ static void rt2x00lib_link_tuner(struct work_struct *work)
         * Increase tuner counter, and reschedule the next link tuner run.
         */
        rt2x00dev->link.count++;
-       queue_delayed_work(rt2x00dev->hw->workqueue, &rt2x00dev->link.work,
-                          LINK_TUNE_INTERVAL);
+       queue_delayed_work(rt2x00dev->workqueue,
+                          &rt2x00dev->link.work, LINK_TUNE_INTERVAL);
 }
 
 static void rt2x00lib_packetfilter_scheduled(struct work_struct *work)
@@ -433,6 +425,15 @@ static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
 
        spin_unlock(&intf->lock);
 
+       /*
+        * It is possible the radio was disabled while the work had been
+        * scheduled. If that happens we should return here immediately,
+        * note that in the spinlock protected area above the delayed_flags
+        * have been cleared correctly.
+        */
+       if (!test_bit(DEVICE_ENABLED_RADIO, &rt2x00dev->flags))
+               return;
+
        if (delayed_flags & DELAYED_UPDATE_BEACON) {
                skb = ieee80211_beacon_get(rt2x00dev->hw, vif, &control);
                if (skb && rt2x00dev->ops->hw->beacon_update(rt2x00dev->hw,
@@ -441,7 +442,7 @@ static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
        }
 
        if (delayed_flags & DELAYED_CONFIG_ERP)
-               rt2x00lib_config_erp(rt2x00dev, intf, &intf->conf);
+               rt2x00lib_config_erp(rt2x00dev, intf, &conf);
 
        if (delayed_flags & DELAYED_LED_ASSOC)
                rt2x00leds_led_assoc(rt2x00dev, !!rt2x00dev->intf_associated);
@@ -487,7 +488,7 @@ void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev)
                                                   rt2x00lib_beacondone_iter,
                                                   rt2x00dev);
 
-       queue_work(rt2x00dev->hw->workqueue, &rt2x00dev->intf_work);
+       queue_work(rt2x00dev->workqueue, &rt2x00dev->intf_work);
 }
 EXPORT_SYMBOL_GPL(rt2x00lib_beacondone);
 
@@ -1130,6 +1131,10 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
        /*
         * Initialize configuration work.
         */
+       rt2x00dev->workqueue = create_singlethread_workqueue("rt2x00lib");
+       if (!rt2x00dev->workqueue)
+               goto exit;
+
        INIT_WORK(&rt2x00dev->intf_work, rt2x00lib_intf_scheduled);
        INIT_WORK(&rt2x00dev->filter_work, rt2x00lib_packetfilter_scheduled);
        INIT_DELAYED_WORK(&rt2x00dev->link.work, rt2x00lib_link_tuner);
@@ -1190,6 +1195,13 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
        rt2x00leds_unregister(rt2x00dev);
 
        /*
+        * Stop all queued work. Note that most tasks will already be halted
+        * during rt2x00lib_disable_radio() and rt2x00lib_uninitialize().
+        */
+       flush_workqueue(rt2x00dev->workqueue);
+       destroy_workqueue(rt2x00dev->workqueue);
+
+       /*
         * Free ieee80211_hw memory.
         */
        rt2x00lib_remove_hw(rt2x00dev);
index 87e280a..9cb023e 100644 (file)
@@ -428,7 +428,7 @@ void rt2x00mac_configure_filter(struct ieee80211_hw *hw,
        if (!test_bit(DRIVER_REQUIRE_SCHEDULED, &rt2x00dev->flags))
                rt2x00dev->ops->lib->config_filter(rt2x00dev, *total_flags);
        else
-               queue_work(rt2x00dev->hw->workqueue, &rt2x00dev->filter_work);
+               queue_work(rt2x00dev->workqueue, &rt2x00dev->filter_work);
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_configure_filter);
 
@@ -509,7 +509,7 @@ void rt2x00mac_bss_info_changed(struct ieee80211_hw *hw,
        memcpy(&intf->conf, bss_conf, sizeof(*bss_conf));
        if (delayed) {
                intf->delayed_flags |= delayed;
-               queue_work(rt2x00dev->hw->workqueue, &rt2x00dev->intf_work);
+               queue_work(rt2x00dev->workqueue, &rt2x00dev->intf_work);
        }
        spin_unlock(&intf->lock);
 }