From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail2.tohojo.dk (mail2.tohojo.dk [77.235.48.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id CE1DD3B260 for ; Fri, 17 Jun 2016 09:43:30 -0400 (EDT) X-Virus-Scanned: amavisd-new at mail2.tohojo.dk DKIM-Filter: OpenDKIM Filter v2.10.3 mail2.tohojo.dk 7F30F40A3A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=201310; t=1466171008; bh=/o/kaEad5xSqLZADYdHi2Yn7y7KWBvTeNxNYrv231v0=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=SdIsltRYr50cxKrgDikJITet80AWuYKRYPR3BuW8Ydo49LexwMUXLfdI2+oQojk9o 77rHze0RX9nAF031PaUJdTnO07QAFMd7bh8wHagkp/yiShiSw+QVLEYN7H9EeQn010 jjrHGJhLN8DxQynz2u2WGfsAa21JXHOCP/yM97Xk= Sender: toke@toke.dk Received: by alrua-kau.kau.toke.dk (Postfix, from userid 1000) id 96576C40172; Fri, 17 Jun 2016 15:43:27 +0200 (CEST) From: =?utf-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= To: Felix Fietkau Cc: linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, ath9k-devel@lists.ath9k.org, Tim Shepard References: <20160617090929.31606-1-toke@toke.dk> <20160617090929.31606-2-toke@toke.dk> <30beabff-a8c6-5431-be09-8f2f83ffc974@nbd.name> Date: Fri, 17 Jun 2016 15:43:27 +0200 In-Reply-To: <30beabff-a8c6-5431-be09-8f2f83ffc974@nbd.name> (Felix Fietkau's message of "Fri, 17 Jun 2016 15:28:26 +0200") X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87d1ng3p8g.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues X-BeenThere: make-wifi-fast@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Jun 2016 13:43:31 -0000 Felix Fietkau writes: > On 2016-06-17 11:09, Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> This patch leaves the code for ath9k's internal per-node per-tid >> queues in place and just modifies the driver to also pull from >> the new mac80211 intermediate software queues, and implements >> the .wake_tx_queue method, which will cause mac80211 to deliver >> packets to be sent via the new intermediate queue. >>=20 >> Signed-off-by: Tim Shepard >>=20 >> Reworked to not require the global variable renaming in ath9k. >>=20 >> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen >> --- >> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++- >> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +- >> drivers/net/wireless/ath/ath9k/init.c | 1 + >> drivers/net/wireless/ath/ath9k/main.c | 1 + >> drivers/net/wireless/ath/ath9k/xmit.c | 119 ++++++++++++++++++++++= +++---- >> 5 files changed, 125 insertions(+), 19 deletions(-) >>=20 >> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wirele= ss/ath/ath9k/ath9k.h >> index 93b3793..caeae10 100644 >> --- a/drivers/net/wireless/ath/ath9k/ath9k.h >> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h >> @@ -145,8 +145,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct a= th_descdma *dd, >> #define BAW_WITHIN(_start, _bawsz, _seqno) \ >> ((((_seqno) - (_start)) & 4095) < (_bawsz)) >>=20=20 >> -#define ATH_AN_2_TID(_an, _tidno) (&(_an)->tid[(_tidno)]) >> - >> #define IS_HT_RATE(rate) (rate & 0x80) >> #define IS_CCK_RATE(rate) ((rate >=3D 0x18) && (rate <=3D 0x1e)) >> #define IS_OFDM_RATE(rate) ((rate >=3D 0x8) && (rate <=3D 0xf)) >> @@ -232,8 +230,10 @@ struct ath_buf { >>=20=20 >> struct ath_atx_tid { >> struct list_head list; >> + struct sk_buff_head i_q; > Do we really need a third queue here? Instead of adding yet another > layer of queueing here, I think we should even get rid of buf_q. This is definitely something that needs to be improved. One other sticking point related to this: in the current version of this patch ath_tid_has_buffered() gains a side effect of pulling from the mac80211 txq, which is obviously not so nice. The obvious way to get rid of this is to export a txq_has_buffered() function at the mac80211 layer. But avoiding that may be possible; the sticking point is what to do with the code paths that do not dequeue packets, but check ath_tid_has_buffered() to decide whether to schedule the queue and/or to tell ieee80211_sta_set_buffered() about it (these are for instance ath_tx_aggr_sleep/wakeup(). Can those just be removed (i.e. don't call into ieee80211, and always schedule the txq on wakeup? I'm not familiar enough with the intermediate queues to make that call... > Channel context based queue handling can be dealt with by > stopping/starting relevant queues on channel context changes. Noted. > buf_q becomes unnecessary when you remove all code in the drv_tx > codepath that moves frames to the intermediate queue. > > Any frame that was pulled from the intermediate queue and prepared for > tx, but which can't be sent right now can simply be queued to retry_q. Right. > This will also help with getting the diffstat insertion/deletion ratio > under control ;) Yes, that would be good ;) >> struct sk_buff_head buf_q; >> struct sk_buff_head retry_q; >> + struct ieee80211_txq *swq; > No need for this pointer, you can use container_of. Ah, cool, thanks! -Toke