From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [IPv6:2001:470:dc45:1000::1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 6CF1A3CB35 for ; Mon, 10 Sep 2018 11:00:13 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1536591612; bh=OGU2ZUa3XUNp3JFhECNQARQwDSkVpxD+uZohJ7E8bOk=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=MSMSmfAaJTihtZN/EZ+NAH0LJ++qvrKarZvb1gfKxTa6wAdOmhZfewO2KGHhiBrSa 7GNUgVs/BQ6TaKFkZlzlZgi//aFcMAG76VD2LpgsN+qMCEh9jhtGLDll0sWJGh34Vl P24+tKzb5ihoT2MHg6oUfhk6nMIZBuoshn5g6IBckxQM8i8k0Ux39+QqLiJvhW1pxy yC1mK7fPI06kmyKyBBhrP7DOKnHBivGbyJ4qkMWPEpZwaiw98bOGlDXldrD64VeyYd vUnMkoiS0mHejagw7FrNNkj4kOsXYDYX5e2KWhCa00QuXdMtDAVOz1/YcR6JZCHBtr GmfC3Ft4k1H9g== To: Johannes Berg , linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net Cc: Rajkumar Manoharan , Felix Fietkau In-Reply-To: <1536591110.3224.76.camel@sipsolutions.net> References: <153635803319.14170.10011969968767927187.stgit@alrua-x1> <153635897010.14170.2992498632345986102.stgit@alrua-x1> <1536565717.3224.12.camel@sipsolutions.net> <87musplivy.fsf@toke.dk> <1536577419.3224.50.camel@sipsolutions.net> <87zhwpjzme.fsf@toke.dk> <1536583587.3224.71.camel@sipsolutions.net> <87wortjy8n.fsf@toke.dk> <1536585051.3224.72.camel@sipsolutions.net> <87r2i1jxse.fsf@toke.dk> <1536591110.3224.76.camel@sipsolutions.net> Date: Mon, 10 Sep 2018 17:00:11 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87muspjt38.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 RFC v3 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: Mon, 10 Sep 2018 15:00:13 -0000 Johannes Berg writes: > On Mon, 2018-09-10 at 15:18 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > >> If we have the start_schedule() / end_schedule() pair anyway, the latter >> could notify any TXQs that became eligible during the scheduling round. > > Do we even need end_schedule()? It's hard to pass multiple things to a > single call (do you build a list?), so having > > start_schedule(), get_txq(), return_txq() > > would be sufficient? Well, start_schedule() / end_schedule() would be needed if we are going to add locking in mac80211? >> Also, instead of having the three different API functions >> (next_txq()/may_tx()/schedule_txq()), we could have get_txq(txq)/put_tx= q(txq) >> which would always need to be paired; but the argument to get_txq() >> could be optional, and if the driver passes NULL it means "give me the >> next available TXQ". > > I can't say I like this. It makes the meaning totally different: > > * with NULL: use the internal scheduler to determine which one is good > to use next > * non-NULL: essentially equivalent to may_tx() Yeah, it'll be two completely different uses for the same function; but there wouldn't be two different APIs to keep track of. I'm fine with keeping them as separately named functions. :) >> So for ath9k it would be: >>=20 >>=20 >> start_schedule(ac); >> while ((txq =3D get_txq(NULL)) { >> queue_aggregate(txq); >> put_txq(txq); >> } >> end_schedule(ac); >>=20 >> And for ath10k/iwlwifi it would be: >>=20 >> on_hw_notify(txq) { >> start_schedule(ac); >> if (txq =3D get_txq(txq)) { >> queue_packets(txq); >> put_txq(txq); >> } >> end_schedule(ac); >> } >>=20 >>=20 >> I think that would be simpler, API-wise? > > I can't say I see much point in overloading get_txq() that way. You'd > never use it the same way. > > Also, would you really start_schedule(ac) in the hw-managed case? If we decide mac80211 needs to do locking to prevent two threads from scheduling the same ac, that would also be needed for the hw-managed case? > It seems like not? Basically it seems to me that in the hw-managed > case all you need is may_tx()? And in fact, once you opt in you don't > even really need *that* since mac80211 can just return NULL from > get_skb()? Yeah, we could just throttle in get_skb(); the separate call was to avoid the overhead of the check for every packet. Typically, you'll pick a TXQ, then dequeue multiple packets from it in succession; with a separate call to may_tx(), you only do the check once, not for every packet... -Toke