[rt2x00-users] [PATCH] Move TX/RX work into dedicated workqueue

Gertjan van Wingerde gwingerde at gmail.com
Tue Jan 25 18:35:50 EST 2011


On 01/24/11 19:51, Ivo Van Doorn wrote:
> Hi,
> 
>>> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
>>> index 7661e4f..39bc2fa 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2x00.h
>>> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
>>> @@ -860,6 +860,13 @@ struct rt2x00_dev {
>>>        */
>>>       struct ieee80211_low_level_stats low_level_stats;
>>>
>>> +     /**
>>> +      * Work queue for all work which should not be placed
>>> +      * on the mac80211 workqueue (because of dependencies
>>> +      * between various work structures).
>>> +      */
>>> +     struct workqueue_struct *workqueue;
>>> +
>>>       /*
>>>        * Scheduled work.
>>>        * NOTE: intf_work will use ieee80211_iterate_active_interfaces()
>>
>>
>> The workqueue is only used for USB devices, should this be mentioned in the comment?
> 
> Yeah, I thought about that as well, but I was considering other cases as well,
> in which we can use the workqueue for generic work structures as well.
>>> @@ -1054,6 +1061,7 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
>>>       cancel_work_sync(&rt2x00dev->intf_work);
>>>       cancel_work_sync(&rt2x00dev->rxdone_work);
>>>       cancel_work_sync(&rt2x00dev->txdone_work);
>>> +     destroy_workqueue(rt2x00dev->workqueue);
>>>
>>>       /*
>>>        * Free the tx status fifo.
>>
>>
>> As the workqueue is only used for USB devices, can't we move the setup and destroy code
>> to the rt2x00usb module?
> 
> See earlier comment.
> Also several other USB/PCI specific components are handled
> in rt2x00lib as well. We should perhaps check those components as
> well, and cleanup
> the allocation/cleanup in general.
> 
> 
>>> diff --git a/drivers/net/wireless/rt2x00/rt2x00link.c b/drivers/net/wireless/rt2x00/rt2x00link.c
>>> index bfda60e..c975b0a 100644
>>> --- a/drivers/net/wireless/rt2x00/rt2x00link.c
>>> +++ b/drivers/net/wireless/rt2x00/rt2x00link.c
>>> @@ -417,7 +417,8 @@ void rt2x00link_start_watchdog(struct rt2x00_dev *rt2x00dev)
>>>           !test_bit(DRIVER_SUPPORT_WATCHDOG, &rt2x00dev->flags))
>>>               return;
>>>
>>> -     schedule_delayed_work(&link->watchdog_work, WATCHDOG_INTERVAL);
>>> +     ieee80211_queue_delayed_work(rt2x00dev->hw,
>>> +                                  &link->watchdog_work, WATCHDOG_INTERVAL);
>>>  }
>>>
>>>  void rt2x00link_stop_watchdog(struct rt2x00_dev *rt2x00dev)
>>> @@ -441,7 +442,9 @@ static void rt2x00link_watchdog(struct work_struct *work)
>>>       rt2x00dev->ops->lib->watchdog(rt2x00dev);
>>>
>>>       if (test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
>>> -             schedule_delayed_work(&link->watchdog_work, WATCHDOG_INTERVAL);
>>> +             ieee80211_queue_delayed_work(rt2x00dev->hw,
>>> +                                          &link->watchdog_work,
>>> +                                          WATCHDOG_INTERVAL);
>>>  }
>>>
>>>  void rt2x00link_register(struct rt2x00_dev *rt2x00dev)
>>
>>
>> The above changes seem to be unrelated to the introduction of the rt2x00 workqueue.
>> Do they have to be in this patch?
> 
> Well this patch is about having a clear separation between the TX/RX
> work structures
> and the rest. So in that respect, it does belong in this patch,.
> 

OK. Thanks for the clarifications.

You can add my

Acked-by: Gertjan van Wingerde <gwingerde at gmail.com>

--
Gertjan.



More information about the users mailing list