From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E4E853B29E for ; Thu, 12 Jul 2018 20:33:55 -0400 (EDT) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 2A07E60271; Fri, 13 Jul 2018 00:33:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1531442035; bh=zYHISlCAdkn7fCgMaF0D6bqs2EEUqqChP7bfoevFZ6Y=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NFkyiKC+wtp4q0jO1+m2/Rhn7TvhbtpBCgBBWbuodPC3oOB9++SJELpLlfIBaa6+G h20NZeQYQK5geDzJlMfDIJSzZCKqRrjOYaV5eT+XoNjR15hoOnh8CIaHkBuOVE5JUM onySJDE/fgkj16h/qGva2EdS3Dz5+i//gmBE68tM= X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.8 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,T_DKIM_INVALID autolearn=no autolearn_force=no version=3.4.0 Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 4DF5360271; Fri, 13 Jul 2018 00:33:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1531442034; bh=zYHISlCAdkn7fCgMaF0D6bqs2EEUqqChP7bfoevFZ6Y=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=bv5JPmCVLXWkH58zEn+ks6KW4wlYkvCrK8jQEuv68jVY6M4BdDQEw5j3TdWgGTwXo Hi9tgztNdALtoitHxiOciKEQAYRBY6kOrrpJbisoAHNUDoi4lcR07FrfiEzEhxSHOQ 4/IqU6nM+5JcyExt2fbJuVpCx2XHXxrgfFOxBnYs= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Thu, 12 Jul 2018 17:33:54 -0700 From: Rajkumar Manoharan To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Cc: linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, Felix Fietkau , linux-wireless-owner@vger.kernel.org In-Reply-To: <87k1q09hf1.fsf@toke.dk> References: <153115421866.7447.6363834356268564403.stgit@alrua-x1> <153115422491.7447.12479559048433925372.stgit@alrua-x1> <361a221dd15e44028fd35440df657a3d@codeaurora.org> <87lgahbisu.fsf@toke.dk> <8d8160cd9c804d1b00ba4e234c8f1520@codeaurora.org> <87k1q09hf1.fsf@toke.dk> Message-ID: X-Sender: rmanohar@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 X-Mailman-Approved-At: Fri, 13 Jul 2018 07:52:04 -0400 Subject: Re: [Make-wifi-fast] [RFC v2 1/4] mac80211: Add TXQ scheduling API 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, 13 Jul 2018 00:33:56 -0000 On 2018-07-12 16:13, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan writes: > >> On 2018-07-11 13:48, Toke Høiland-Jørgensen wrote: >>> Rajkumar Manoharan writes: >>> >>>> On 2018-07-09 09:37, Toke Høiland-Jørgensen wrote: >>>> [...] >>>>> +/** [...] >>> Erm, how would this prevent an infinite loop? With this scheme, at >>> some >>> point, ieee80211_next_txq() removes the last txq from activeq, and >>> returns that. Then, when it is called again the next time the driver >>> loops, it's back to the first case (activeq empty, waitq non-empty); >>> so >>> it'll move waitq back as activeq and start over... Only the driver >>> really knows when it is starting a logical "loop through all active >>> TXQs". >>> >> Oops.. My bad.. The idea is that ieee80211_next_txq process txq from >> activeq only and keep processed txqs separately. Having single list >> eventually needs tracking mechanism. The point is that once activeq >> becomes empty, splice waitq list and return NULL. So that driver can >> break from the loop. >> >> ieee80211_next_txq >> - if activeq empty, >> - move waitq list into activeq >> - return NULL >> >> - if activeq not empty >> - fetch appropriate txq from activeq >> - remove txq from activeq list. >> >> - If txq found, return txq else return NULL > > Right, so this would ensure the driver only sees each TXQ once *if it > keeps looping*. But it doesn't necessarily; if the hardware queues fill > up, for instance, it might abort earlier. In that case it would need to > signal mac80211 that it is done for now, which is equivalent to > signalling when it starts a scheduler round. > Hmm... I thought driver will call ieee80211_schedule_txq when it runs out of hardware descriptor and break the loop. The serving txq will be added back to head of activeq list. no? >>> Also, for airtime fairness, the queues are not scheduled strictly >>> round-robin, so the dual-list scheme wouldn't work there either... >>> >> As you know, ath10k driver will operate in two tx modes (push-only, >> push-pull). These modes will be switched dynamically depends on >> firmware load or resource availability. In push-pull mode, firmware >> will query N number of frames for set of STA, TID. > > Ah, so it will look up the TXQ without looping through the list of > pending queues at all? Didn't realise that is what it's doing; yeah, > that would be problematic for airtime fairness :) > >> So the driver will directly dequeue N number of frames from given txq. >> In this case txq ordering alone wont help. I am planning to add below >> changes in exiting API and add new API ieee80211_reorder_txq. >> >> ieee80211_txq_get_depth >> - return deficit status along with frm_cnt >> >> ieee80211_reorder_txq >> - if txq deficit > 0 >> - return; >> - if txq is last >> - return >> - delete txq from list >> - move it to tail >> - update deficit by quantum >> >> ath10k_htt_rx_tx_fetch_ind >> - get txq deficit status >> - if txq deficit > 0 >> - dequeue skb >> - else if deficit < 0 >> - return NULL >> >> Please share your thoughts. > > Hmm, not sure exactly how this would work; seems a little complicated? > Also, I'd rather if drivers were completely oblivious to the deficit; > that is a bit of an implementation detail... > Agree.. Initially I thought of adding deficit check in ieee80211_tx_dequeue. But It will be overhead of taking activeq_lock for every skbs. Perhaps it can be renamed as allowed_to_dequeue instead of deficit. > We could have an ieee80211_txq_may_pull(); or maybe just have > ieee80211_tx_dequeue() return NULL if the deficit is negative? > As I said earlier, checking deficit for every skb will be an overhead. It should be done once before accessing txq. > the reasonable thing for the driver to do, then, would be to ask > ieee80211_next_txq() for another TXQ to pull from if the current one > doesn't work for whatever reason. > > Would that work for push-pull mode? > Not really. Driver shouldn't send other txq data instead of asked one. In MU-MIMO, firmware will query N packets from given set of {STA,TID}. So the driver not supposed to send other txq's data. -Rajkumar