On Thu, Jul 26, 2012 at 8:04 PM, Arik Nemtsov <arik@xxxxxxxxxx> wrote: > On Thu, Jul 26, 2012 at 7:57 PM, Arik Nemtsov <arik@xxxxxxxxxx> wrote: >> On Thu, Jul 26, 2012 at 7:31 PM, Thomas Huehn >> <thomas@xxxxxxxxxxxxxxxxxxxxxxx> 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. Better to just wait some time. I wrote up something to make it much easier for you. Waiting for some internal review before sending it up. Arik -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
