[Cerowrt-devel] Fwd: [bug #442] ath9k queue hang

[Dave Taht]

should I have said "de-protected"? in

linux-3.14/drivers/net/wireless/ath/ath9k/xmit.c

        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q] &&
            ++txq->pending_frames > sc->tx.txq_max_pending[q] &&
            !txq->stopped) {
                ieee80211_stop_queue(sc->hw, q);
                txq->stopped = true;
        }

        if (txctl->an && ieee80211_is_data_present(hdr->frame_control))
                tid = ath_get_skb_tid(sc, txctl->an, skb);

        if (info->flags & IEEE80211_TX_CTL_PS_RESPONSE) {
                ath_txq_unlock(sc, txq);
                txq = sc->tx.uapsdq;
^^^^^^
                ath_txq_lock(sc, txq);
        } else if (txctl->an &&



On Wed, Apr 16, 2014 at 9:55 AM, Felix Fietkau <nbd at openwrt.org> wrote:
> On 2014-04-16 17:34, Dave Taht wrote:
>> On Wed, Apr 16, 2014 at 6:11 AM, Felix Fietkau <nbd at openwrt.org> wrote:
>>> On 2014-04-15 21:00, Dave Taht wrote:
>>>> Thx felix!
>>>>
>>>> Given that there seems to be a potential race in the code
>>>> review I did at:
>>>>
>>>> http://www.bufferbloat.net/issues/442#note-22
>>>>
>>>> another thought is to make the increment and decrement of
>>>>
>>>> txq->pending_frame atomic, or to do a flush before the unlock
>>> I'm not convinced that there's a race that involves txq->pending_frames.
>>> There is no need to make the increment/decrement atomic, because that
>>> variable is already protected by the txq lock.
>>
>> It and "stopped" are briefly unprotected along that code path.
> Where?
>
> - Felix



-- 
Dave Täht

NSFW: https://w2.eff.org/Censorship/Internet_censorship_bills/russell_0296_indecent.article