On Sun, Nov 27, 2011 at 18:55, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > On Sun, 2011-11-27 at 11:12 -0500, Nikolay Martynov wrote: > >> > - del_timer(&tid_tx->addba_resp_timer); >> > + del_timer_sync(&tid_tx->addba_resp_timer); >> > >> > #ifdef CONFIG_MAC80211_HT_DEBUG >> > printk(KERN_DEBUG "switched off addBA timer for tid %d\n", tid); >> > #endif >> > + >> > + /* >> > + * addba_resp_timer may have fired before we got here, and >> > + * caused WANT_STOP to be set. STOPPING should not be set >> > + * as we're under the mutex, but check it anyway. >> > + */ >> > + if (test_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state) || >> > + test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) { >> >> Just a small comment about comment :). >> If I understand correctly process of stopping tx agg looks as following: >> 1) Call to ___ieee80211_stop_tx_ba_session. This removes >> HT_AGG_STATE_WANT_STOP (in ht.c) and adds HT_AGG_STATE_STOPPING. It >> holds mutex during duration of the call. >> 2) Call to ieee80211_stop_tx_ba_cb. This destroys the actual tx_tid. >> It holds mutex during duration of the call. > >> But between these two calls we have HT_AGG_STATE_STOPPING and no >> mutex. I think at this time we can receive addBA resp and that's why I >> was checking for HT_AGG_STATE_STOPPING in my original patch. > > Indeed, I missed that. It's due to the driver callback again. > > How about this? > > /* > * addba_resp_timer may have fired before we got here, and > * caused WANT_STOP to be set. If the stop then was already > * processed further, STOPPING might be set. > */ > > > Did you notice that I moved this code to after the dialog token check? > Don't you think we should also send a delBA ? The AP thinks we will Tx in Agg and basically we are now out of sync. -- 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