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 611d98320593869dc9c26faf2c64218c8c32e3ec..b4bf1e09cf9a906447ed8a5770e9e0cdfec52626 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 2673d568bcac286dc9f3cbdb8da9d3c5051ff7cc..c997d4f28ab39bd31e7e8b25adad8ea278b1c198 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);
 }
 
@@ -136,14 +136,6 @@ void rt2x00lib_disable_radio(struct rt2x00_dev *rt2x00dev)
        if (!__test_and_clear_bit(DEVICE_ENABLED_RADIO, &rt2x00dev->flags))
                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.
         */
@@ -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);
@@ -1189,6 +1194,13 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
        rt2x00rfkill_free(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.
         */
index 87e280a21971faf5dbb8e5f15186ec8d41c21ec2..9cb023edd2e9bf7eaad394c5a205c2af3a6e9076 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);
 }