[rt2x00-users] [PATCH] rt2x00: Don't queue ieee80211 work after USB removal

[Sean Cross]

On Oct 30, 2009, at Fri 30, 5:51 PM, Ivo van Doorn wrote:
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>>
>> @@ -362,8 +363,9 @@
>>
>>    rt2x00link_reset_tuner(rt2x00dev, false);
>>
>> -   ieee80211_queue_delayed_work(rt2x00dev->hw,
>> -                 &link->work, LINK_TUNE_INTERVAL);
>> +   if(test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
>> +      ieee80211_queue_delayed_work(rt2x00dev->hw,
>> +                  &link->work, LINK_TUNE_INTERVAL);
>> }
>>
>> void rt2x00link_stop_tuner(struct rt2x00_dev *rt2x00dev)
>> @@ -469,8 +471,10 @@
>>     * Increase tuner counter, and reschedule the next link tuner run.
>>     */
>>    link->count++;
>> -   ieee80211_queue_delayed_work(rt2x00dev->hw,
>> -                 &link->work, LINK_TUNE_INTERVAL);
>> +
>> +   if(test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
>> +      ieee80211_queue_delayed_work(rt2x00dev->hw,
>> +                  &link->work, LINK_TUNE_INTERVAL);
>> }
>
> Actually these 2 changes are incorrec, what happens when the device  
> disapears between scheduling
> and starting the thread? Wouldn't that cause the same error you are  
> trying to fix? Wouldn't checking
> the flag at the start of the thread be more meaningful?


Perhaps it does make more sense to test for DEVICE_STATE_PRESENT in  
rt2x00link_start_tuner, just so rt2x00link_reset_tuner() isn't called  
on a card that is no longer present.

But the timing issue I'm seeing is something like this:

1) Card is removed
2) ENODEV messages start getting received.
3) Something decides to start the tuner.
4) link->work gets queued to happen sometime in the future.
5) The USB system finally decides to unload the driver.  I think  
rt2x00usb_disconnect() gets called here.
6) link->work gets run, but as the device has been unloaded, the  
delayed_work is no longer valid, so a KP occurs.

The root cause is that link->work is suddenly no longer valid, so the  
flag can't get checked at the start of the thread.  Since I think  
ENODEV will get returned before rt2x00link_start_tuner() is called, I  
believe this is a valid method of catching the removal.


Sean