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 7942B3B2A4 for ; Wed, 12 Sep 2018 12:23:51 -0400 (EDT) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 83BDA607DD; Wed, 12 Sep 2018 16:23:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1536769430; bh=24FRP0mNKmZX+KjxX5/xJ5n+9a01xSzZw0vKy2vVsLU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lPdk0jFbHk/3ZvPT2LFS5m/uiGi9L1Akz3smxieFnZ3FlZJyk4+0rE1v8zO0lheYS W+NpyQYxV6WcWrlL1Y3k+sBxo/y+pPn+iAbrH4bmwCJZBLd9jon2ovlLfBYOy+C3O3 OMV6XxkGD/o1Hn8KUKdDW53rV95gdmr60duMn0ls= 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 5FD48602A7; Wed, 12 Sep 2018 16:23:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1536769429; bh=24FRP0mNKmZX+KjxX5/xJ5n+9a01xSzZw0vKy2vVsLU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=TD7iYZfhWWqBT7zUfx6pe/wtwHWqqluBzdq6oNWlFEKzQ+5lUhdgko7NEi5nINbNT vrg/uyYahINR2SIG35Tc93wLzbTnLW/C9vh447Qb7l3S0r9cKIZae3Uw84X7CN21Sr rv/2VtrUG0Lh3T77Zlyr6NUsdB5FSihxD/L9TZZ8= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Wed, 12 Sep 2018 09:23:49 -0700 From: Rajkumar Manoharan To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Cc: Johannes Berg , linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, Felix Fietkau , linux-wireless-owner@vger.kernel.org In-Reply-To: <87o9d3hsys.fsf@toke.dk> References: <153635803319.14170.10011969968767927187.stgit@alrua-x1> <153635897061.14170.17999956909203163571.stgit@alrua-x1> <1536567496.3224.36.camel@sipsolutions.net> <87h8ixli4s.fsf@toke.dk> <87o9d3hsys.fsf@toke.dk> Message-ID: X-Sender: rmanohar@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 Subject: Re: [Make-wifi-fast] [PATCH RFC v3 2/4] mac80211: Add airtime accounting and scheduling to TXQs 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, 12 Sep 2018 16:23:51 -0000 On 2018-09-12 04:10, Toke Høiland-Jørgensen wrote: > Rajkumar Manoharan writes: > >> On 2018-09-10 04:13, Toke Høiland-Jørgensen wrote: >>> Johannes Berg writes: >>>>> - txqi->flags & (1<>>>> - txqi->flags & (1<>>>> - txqi->flags & (1<>>>> : >>>>> ""); >>>>> + txqi->flags & (1 << IEEE80211_TXQ_STOP) ? "STOP" : >>>>> "RUN", >>>>> + txqi->flags & (1 << IEEE80211_TXQ_AMPDU) ? " AMPDU" : >>>>> "", >>>>> + txqi->flags & (1 << IEEE80211_TXQ_NO_AMSDU) ? " >>>>> NO-AMSDU" >>>>> : ""); >>>> >>>> consider BIT() instead as a cleanup? :) >>>> >>>> (or maybe this is just a leftover from merging some other patches?) >>> >>> Yeah, this is a merging artifact; Rajkumar's patch added another >>> flag, >>> that I removed again. Didn't notice that there was still a whitespace >>> change in this patch... >>> >> I added the flag based on our last discussion. The driver needs to >> check >> txq status for each tx_dequeue(). One time txq check is not sufficient >> as it allows the driver to dequeue all frames from txq. >> >> drv_func() { >> while (ieee80211_airtime_may_transmit(txq) && >> hw_has_space() && >> (pkt = ieee80211_tx_dequeue(hw, txq))) >> push_to_hw(pkt); >> } > > Yeah, but with airtime only being recorded on TX completion, the odds > of > the value changing within that loop are quite low; so it's not going to > work, which is why I removed it. > > However, after reading Kan's patches I get where you're coming from; a > check in tx_dequeue() is needed for the BQL-style queue limiting. Will > try to incorporate a version of that in the next series so you can see > what I mean when I say it should be orthogonal; and I'm still not sure > it needs a flag :) > Got it.. Will wait for next version.. thanks. >>>>> +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw, >>>>> + struct ieee80211_txq *txq) >>>>> +{ >>>>> + struct ieee80211_local *local = hw_to_local(hw); >>>>> + struct txq_info *txqi = to_txq_info(txq); >>>>> + bool may_tx = false; >>>>> + >>>>> + spin_lock_bh(&local->active_txq_lock); >>>>> + >>>>> + if (ieee80211_txq_check_deficit(local, txqi)) { >>>>> + may_tx = true; >>>>> + list_del_init(&txqi->schedule_order); >>>> >> >> To handle above case, may_transmit should remove the node only >> when it is in list. >> >> if (list_empty(&txqi->schedule_order)) >> list_del_init(&txqi->schedule_order); > > I assume you missed a ! in that if, right? :) > Oops.. yes it should be ! :) -Rajkumar