From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nbd.name (nbd.name [IPv6:2a01:4f8:131:30e2::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E6B4C3B260 for ; Wed, 6 Jul 2016 14:13:57 -0400 (EDT) To: =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= , linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, ath9k-devel@lists.ath9k.org References: <20160618190505.24038-1-toke@toke.dk> <20160706161612.16597-1-toke@toke.dk> Cc: Tim Shepard From: Felix Fietkau Message-ID: <85779557-6c5a-fcd3-60cf-4d3c79aec652@nbd.name> User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160706161612.16597-1-toke@toke.dk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Mailman-Approved-At: Mon, 28 Nov 2016 08:47:10 -0500 Subject: Re: [Make-wifi-fast] [PATCH v2] ath9k: Switch to using 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: , Date: Wed, 06 Jul 2016 18:13:58 -0000 X-Original-Date: Wed, 6 Jul 2016 20:13:55 +0200 X-List-Received-Date: Wed, 06 Jul 2016 18:13:58 -0000 On 2016-07-06 18:16, Toke Høiland-Jørgensen wrote: > This switches ath9k over to using the mac80211 intermediate software > queueing mechanism for data packets. It removes the queueing inside the > driver, except for the retry queue, and instead pulls from mac80211 when > a packet is needed. The retry queue is used to store a packet that was > pulled but can't be sent immediately. > > The old code path in ath_tx_start that would queue packets has been > removed completely, as has the qlen limit tunables (since there's no > longer a queue in the driver to limit). > > Based on Tim's original patch set, but reworked quite thoroughly. > > Cc: Tim Shepard > Cc: Felix Fietkau > Signed-off-by: Toke Høiland-Jørgensen > --- > Changes since v1: > - Remove the old intermediate queueing logic completely instead of > just disabling it. > - Remove the qlen debug tunables. > - Remove the force_channel parameter from struct txctl (since we just > removed the code path that was using it). > > drivers/net/wireless/ath/ath9k/ath9k.h | 12 +- > drivers/net/wireless/ath/ath9k/channel.c | 2 - > drivers/net/wireless/ath/ath9k/debug.c | 14 +- > drivers/net/wireless/ath/ath9k/debug.h | 2 - > drivers/net/wireless/ath/ath9k/debug_sta.c | 4 +- > drivers/net/wireless/ath/ath9k/init.c | 2 +- > drivers/net/wireless/ath/ath9k/main.c | 1 + > drivers/net/wireless/ath/ath9k/xmit.c | 307 +++++++++++------------------ > 8 files changed, 130 insertions(+), 214 deletions(-) Nice work! > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c > index fe795fc..4077eeb 100644 > --- a/drivers/net/wireless/ath/ath9k/xmit.c > +++ b/drivers/net/wireless/ath/ath9k/xmit.c > @@ -912,8 +923,16 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq, > seqno = bf->bf_state.seqno; > > /* do not step over block-ack window */ > - if (!BAW_WITHIN(tid->seq_start, tid->baw_size, seqno)) > + if (!BAW_WITHIN(tid->seq_start, tid->baw_size, seqno)) { > + __skb_queue_tail(&tid->retry_q, skb); > + > + /* If there are other skbs in the retry q, they are > + * probably within the BAW, so loop immediately to get > + * one of them. Otherwise the queue can get stuck. */ > + if (!skb_queue_is_first(&tid->retry_q, skb)) > + continue; Not sure if this can happen, but if we ever somehow end up with two skbs in the retry queue that do not fit into the Block-Ack window, there's potential for an infinite loop here. - Felix