From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-x22d.google.com (mail-wi0-x22d.google.com [IPv6:2a00:1450:400c:c05::22d]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 9FF0921F1F3; Tue, 15 Apr 2014 12:00:33 -0700 (PDT) Received: by mail-wi0-f173.google.com with SMTP id z2so248919wiv.12 for ; Tue, 15 Apr 2014 12:00:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=dt2Y6mQ6cn1dzGymLK6jFtD/H3GHhvfjtF+8b8G03po=; b=TN10iWNMhaefM/Y0aBgGW78vFD+nZ8DVtfdOm2qo9ymcwpUWAJDzlfIp5JAYRTW//1 mvE0IioKWn38h+b1d4XVrSHbEV3g+NCPZt5ZP7UmfPHC9vM1nzurcL+irQqDalIwd2ks jHwbRNN1Rk4MFfcVOoaju3wup+Bc/16iuqpZxly6sDcc28HC/9lTnRNTxC3V8IIL8CLf E9lW289Ah9p6GZCLaHlZcJ0RX+9VMP02orUhsXlsgyC2VP43r3LBgV586R80/nVpZMNT dI1rBvdRAwfynCdSqWiYnPq5lMSjTCVIJrYM0v7sTqYD1/IUCMJu0QCZsYgueXV/JElt 1rGQ== MIME-Version: 1.0 X-Received: by 10.180.19.167 with SMTP id g7mr3862223wie.46.1397588431678; Tue, 15 Apr 2014 12:00:31 -0700 (PDT) Received: by 10.216.177.10 with HTTP; Tue, 15 Apr 2014 12:00:31 -0700 (PDT) Date: Tue, 15 Apr 2014 12:00:31 -0700 Message-ID: From: Dave Taht To: Felix Fietkau Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: cerowrt@lists.bufferbloat.net, "cerowrt-devel@lists.bufferbloat.net" Subject: Re: [Cerowrt-devel] Fwd: [bug #442] ath9k queue hang X-BeenThere: cerowrt-devel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: Development issues regarding the cerowrt test router project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Apr 2014 19:00:35 -0000 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 What tree is this patch against? On Tue, Apr 15, 2014 at 11:46 AM, Felix Fietkau wrote: > On 2014-04-15 06:06, Dave Taht wrote: >> regrettably I am too wiped to look this over further right now, but the = patchset >> seems very promising. >> >> I will review on a fresh brain in the morning. Other eyeballs desired >> - this will have to get patched on top of 3.14 and then backported to >> the 3.10 backport.... > The patch is a rather crude workaround which unfortunately will not > help with narrowing down the cause. Also, doing a chip reset because a > software queue is stuck is overkill. > > Please test if this patch helps. The tid->paused flag is no longer > necessary since my rework of the tx path. > --- > --- a/drivers/net/wireless/ath/ath9k/ath9k.h > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h > @@ -254,7 +254,6 @@ struct ath_atx_tid { > > s8 bar_index; > bool sched; > - bool paused; > bool active; > }; > > --- a/drivers/net/wireless/ath/ath9k/xmit.c > +++ b/drivers/net/wireless/ath/ath9k/xmit.c > @@ -107,9 +107,6 @@ static void ath_tx_queue_tid(struct ath_ > { > struct ath_atx_ac *ac =3D tid->ac; > > - if (tid->paused) > - return; > - > if (tid->sched) > return; > > @@ -1407,7 +1404,6 @@ int ath_tx_aggr_start(struct ath_softc * > ath_tx_tid_change_state(sc, txtid); > > txtid->active =3D true; > - txtid->paused =3D true; > *ssn =3D txtid->seq_start =3D txtid->seq_next; > txtid->bar_index =3D -1; > > @@ -1427,7 +1423,6 @@ void ath_tx_aggr_stop(struct ath_softc * > > ath_txq_lock(sc, txq); > txtid->active =3D false; > - txtid->paused =3D false; > ath_tx_flush_tid(sc, txtid); > ath_tx_tid_change_state(sc, txtid); > ath_txq_unlock_complete(sc, txq); > @@ -1487,7 +1482,7 @@ void ath_tx_aggr_wakeup(struct ath_softc > ath_txq_lock(sc, txq); > ac->clear_ps_filter =3D true; > > - if (!tid->paused && ath_tid_has_buffered(tid)) { > + if (ath_tid_has_buffered(tid)) { > ath_tx_queue_tid(txq, tid); > ath_txq_schedule(sc, txq); > } > @@ -1510,7 +1505,6 @@ void ath_tx_aggr_resume(struct ath_softc > ath_txq_lock(sc, txq); > > tid->baw_size =3D IEEE80211_MIN_AMPDU_BUF << sta->ht_cap.ampdu_fa= ctor; > - tid->paused =3D false; > > if (ath_tid_has_buffered(tid)) { > ath_tx_queue_tid(txq, tid); > @@ -1544,8 +1538,6 @@ void ath9k_release_buffered_frames(struc > continue; > > tid =3D ATH_AN_2_TID(an, i); > - if (tid->paused) > - continue; > > ath_txq_lock(sc, tid->ac->txq); > while (nframes > 0) { > @@ -1844,9 +1836,6 @@ void ath_txq_schedule(struct ath_softc * > list_del(&tid->list); > tid->sched =3D false; > > - if (tid->paused) > - continue; > - > if (ath_tx_sched_aggr(sc, txq, tid, &stop)) > sent =3D true; > > @@ -2698,7 +2687,6 @@ void ath_tx_node_init(struct ath_softc * > tid->baw_size =3D WME_MAX_BA; > tid->baw_head =3D tid->baw_tail =3D 0; > tid->sched =3D false; > - tid->paused =3D false; > tid->active =3D false; > __skb_queue_head_init(&tid->buf_q); > __skb_queue_head_init(&tid->retry_q); > --=20 Dave T=C3=A4ht NSFW: https://w2.eff.org/Censorship/Internet_censorship_bills/russell_0296_= indecent.article