From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nbd.name (unknown [IPv6:2a01:4f8:131:30e2::2]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by huchra.bufferbloat.net (Postfix) with ESMTPS id F3AFD21F208 for ; Wed, 16 Apr 2014 14:06:05 -0700 (PDT) Message-ID: <534EF0B2.7060701@openwrt.org> Date: Wed, 16 Apr 2014 23:05:54 +0200 From: Felix Fietkau User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Dave Taht , Robert Bradley References: <534E8168.50003@openwrt.org> <534EB61C.1080108@openwrt.org> <534EBCFC.9000504@gmail.com> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: "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: Wed, 16 Apr 2014 21:06:06 -0000 On 2014-04-16 19:38, Dave Taht wrote: > On Wed, Apr 16, 2014 at 10:25 AM, Robert Bradley > wrote: >> On 16/04/2014 18:00, Dave Taht wrote: >>> should I have said "de-protected"? in >>> >>> linux-3.14/drivers/net/wireless/ath/ath9k/xmit.c >> >>> 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 && >>> >> >> Isn't the point here that you're potentially switching txq and so need >> to unlock the old one and then lock the new one? > > But it's happening after incrementing the old one and potentially stopping it... > > My take on this bit of code is that it needs to happen before the increment > and potental stop, and it should validate the value of tx.uapsdq. > > but I am by no means the expert here! Felix is da man.... The txq switching handling wrt. pending_frames is correct. Since the uapsd queue is used for powersave responses (both from the drv_tx codepath and internal buffering), it is much better to credit these packets to the queue that similar frames are normally meant to be transmitted on. This is the txq before the switch. To make that work, ath_txq_skb_done has an explicit check for the uapsd queue. - Felix