[PATCH v6] mac80211: Remove control.sta from struct ieee80211_tx_info and restructure tx-path

[Arik Nemtsov]

On Thu, Jul 26, 2012 at 7:57 PM, Arik Nemtsov <arik at wizery.com> wrote:
> On Thu, Jul 26, 2012 at 7:31 PM, Thomas Huehn
> <thomas at net.t-labs.tu-berlin.de> wrote:
>> Hi Johannes,
>>
>> Johannes Berg schrieb:
>>
>>>>
>>>> /home/johannes/sys/wireless/drivers/net/wireless/ti/wlcore/tx.c: In
>>>> function ‘wl1271_skb_queue_head’:
>>>> /home/johannes/sys/wireless/drivers/net/wireless/ti/wlcore/tx.c:622:48:
>>>> warning: ‘hlid’ may be used uninitialized in this function
>>>> [-Wuninitialized]
>>>
>>> Those changes make no sense anyway -- you should be able to pass the
>>> station pointer through if it previously used info->control.sta, instead
>>> of doing an (expensive) lookup.
>>
>>
>> This is the call path
>>
>> in main.c ... INIT_WORK(&wl->tx_work, wl1271_tx_work);
>>  jumps to tx.c...wl1271_tx_work(struct work_struct *work)
>>   calls wl1271_tx_work(struct work_struct *work)
>>    calls wlcore_tx_work_locked(struct wl1271 *wl)
>>     calls wl1271_skb_queue_head()
>>      calls wl12xx_tx_get_hlid()
>>       calls wl12xx_tx_get_hlid_ap() .... where sta is needed.
>>
>> But I could not find any point in the call path where I could grab the
>> sta pointer and pass it through. I will move the rcu into
>> wl12xx_tx_get_hlid_ap(), but I do not see any cheaper way here as using rcu.
>
> I agree with Johannes. The find_sta() in the Tx path is unacceptable.
>
> A relatively simple change here would be to put the sta pointer in the
> portion of the struct reserved for rate control during op_tx(). wlcore
> doesn't use software rate control anyway.
> This would avoid from changing the code too much.

Johannes correctly pointed out that the sta may no longer be valid
when the skb is dequeued (which is a bug today as well). It's best to
just store the hlid in the rate control portion.