From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x144.google.com (mail-lf1-x144.google.com [IPv6:2a00:1450:4864:20::144]) (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 5D1803B2A4 for ; Wed, 9 Oct 2019 18:44:50 -0400 (EDT) Received: by mail-lf1-x144.google.com with SMTP id u28so2862503lfc.5 for ; Wed, 09 Oct 2019 15:44:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=K74FSLibgRox92Ke/og8To0X7TZ2VBykrYPKh9Z5pcY=; b=OFzsCFUKTEmsMDknSMDxoYlbwIb7GydlLPMKg2T7ObARVcE80qslnzBR91P7DoYc3l LGczTTg6nnxjA5GRPpHuBcw9ZBqUEBD4tvERUHqHZ65gFMBnZ4hHilyY+F3bTFpndQa1 ZHiNeC0+dGKGssHwt8dgOHpb4GzHA636aT7RiDrU5h+HHm1fQLCXgqFawHbCSnvGOK1R Z2mkd5W8/kVjsp0WyF9CFggzncP2OYg32kAH0b/nlzsRBagRmZrutVQLg1EDZV+cem3+ S/AOygxjUKZWc/f7uf8zmHAnWq9hkQYkfT8vL5tCk8Eo/Z60MKSWEo3QBOBwAh6z0j7h hgrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=K74FSLibgRox92Ke/og8To0X7TZ2VBykrYPKh9Z5pcY=; b=jp1r6K+DpJwCDG8OpZ/9CkOg9rTROdOA14IYrPrQov+q7A2ZbhJu79u/+l6w8s6srg wJ5z3aRFf8DI+BgMmORKg7zjzzaXof2e98Ui0PJM3atr060sB9jjjAiW7Tr6DmhvlE8o OSsxKqP50b2ry3uOTxx1RSiFbSWjn+FDp8WF+r9hMNS2DQnSSx25Sgomtltra5AEqHEA rPxy5Wx+tBpMuJU2a7TdHzWDt1NCiIbCSsdyeo+V5vaEeyH4LvODy1QM9eGwi1Sw90hg bjVJnaF3drnxO8/zlo+Po9w8x8cs0b4a+13DYnTQysXRpiTrocDgP1U9r4xXq6Q8JSQK DRVw== X-Gm-Message-State: APjAAAX623rSF4+5JWmVKO+j89MARa3SrJ8DiOBiMbtN2aEjakRvvhlh uPr1AGeYZvN9urxsS7tidvmm1tHyWB/+a2og0whVWg== X-Google-Smtp-Source: APXvYqw8TxfNud+WAkN3jF6GWM7cpQopC1R+YRlW1JfifDNbx3oPxFQDYFKeZzY2DCGsB2iZqSRopVKloav1TwNXDcM= X-Received: by 2002:a19:7b08:: with SMTP id w8mr3422573lfc.95.1570661088819; Wed, 09 Oct 2019 15:44:48 -0700 (PDT) MIME-Version: 1.0 References: <20191007043120.67567-1-kyan@google.com> <20191007043120.67567-2-kyan@google.com> <18630c07d0aa46d16cf660d013f96b3d@codeaurora.org> In-Reply-To: <18630c07d0aa46d16cf660d013f96b3d@codeaurora.org> From: Kan Yan Date: Wed, 9 Oct 2019 15:44:37 -0700 Message-ID: To: Yibo Zhao Cc: Johannes Berg , linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= , Felix Fietkau Content-Type: text/plain; charset="UTF-8" Subject: Re: [Make-wifi-fast] [PATCH v2 1/2] mac80211: Implement Airtime-based Queue Limit (AQL) 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: Wed, 09 Oct 2019 22:44:50 -0000 Hi Johannes, Thanks for the review and will address all issues you pointed out in the next version. Hi Yibo, > > I assume here the only txq in the list that does not meet AQL check will > not be dequeued. Right? Will it affect peak throughput once there is > only one station. Yes, the txq won't be picked for transmitting even if it is the only active txq if the AQL check failed. However, this won't affect peak throughput. The reason why there are two queue limits is address this kind of situation. The higher queue limit ensures the hardware get enough frames. > > > @@ -3748,10 +3785,10 @@ bool ieee80211_txq_may_transmit(struct > > ieee80211_hw *hw, > > struct sta_info *sta; > > u8 ac = txq->ac; > > > > - spin_lock_bh(&local->active_txq_lock[ac]); > > - > > if (!txqi->txq.sta) > > - goto out; > > + return true; > > why return here? I think even a txq without sta info should get removed > from list and added it back later in return_txq() if needed. No? Yes, it should be removed from the active list. I will fix that. Thanks, Kan On Wed, Oct 9, 2019 at 1:18 AM Yibo Zhao wrote: > > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > > index f13eb2f61ccf..dadb643a5498 100644 > > --- a/net/mac80211/tx.c > > +++ b/net/mac80211/tx.c > > @@ -3669,7 +3669,8 @@ struct ieee80211_txq *ieee80211_next_txq(struct > > ieee80211_hw *hw, u8 ac) > > { > > struct ieee80211_local *local = hw_to_local(hw); > > struct ieee80211_txq *ret = NULL; > > - struct txq_info *txqi = NULL; > > + struct txq_info *txqi = NULL, *head = NULL; > > + bool found_eligible_txq = false; > > > > spin_lock_bh(&local->active_txq_lock[ac]); > > > > @@ -3680,20 +3681,32 @@ struct ieee80211_txq > > *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac) > > if (!txqi) > > goto out; > > > > + if (txqi == head && !found_eligible_txq) > > + goto out; > > I assume here the only txq in the list that does not meet AQL check will > not be dequeued. Right? Will it affect peak throughput once there is > only one station. > > How about dequeuing it anyway regardless AQL because it is the only one > active now so it is fine to occupy the rest bandwidth. Otherwise, I am > afraid next_txq() will return NULL in the test only one station is > present. > > > @@ -3748,10 +3785,10 @@ bool ieee80211_txq_may_transmit(struct > > ieee80211_hw *hw, > > struct sta_info *sta; > > u8 ac = txq->ac; > > > > - spin_lock_bh(&local->active_txq_lock[ac]); > > - > > if (!txqi->txq.sta) > > - goto out; > > + return true; > > why return here? I think even a txq without sta info should get removed > from list and added it back later in return_txq() if needed. No? > > > + > > + spin_lock_bh(&local->active_txq_lock[ac]); > > > > if (list_empty(&txqi->schedule_order)) > > goto out; > > > -- > Yibo