Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bf6f9d8707021dc6017c7cb17805d63edea63333.camel@sipsolutions.net>
Subject: Re: [PATCH] wifi: mac80211: Use internal TX queues for all drivers
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Toke =?ISO-8859-1?Q?H=F8iland-J=F8rgensen?= <toke@kernel.org>,
        Alexander Wetzel <alexander@wetzel-home.de>,
        linux-wireless@vger.kernel.org
Date:   Wed, 05 Oct 2022 14:40:01 +0200
In-Reply-To: <875ygyihhm.fsf@toke.dk>
References: <20220926161303.13035-1-alexander@wetzel-home.de>
         <96e9ad692842853cfe92a7e5de18136baf20a492.camel@sipsolutions.net>
         <875ygyihhm.fsf@toke.dk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-10-05 at 14:26 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:

> > void ieee80211_handle_wake_tx_queue(struct ieee80211_hw *hw,
> > 				    struct ieee80211_txq *txq)
> > {
> > 	... *local =3D from_hw(hw);
> > 	... *sdata =3D from_vif(txq->vif);
> >=20
> > 	wake_tx_push_queue(local, sdata, txq);
> > }
> >=20
> > Actually ... I wonder why you'd here - in waking a single TXQ - use
> > ieee80211_next_txq() at all, Toke, what do you think?
>=20
> Well, this patch does almost exactly the same as the ath9k driver does,
> for instance. Really, the wake_tx_queue() is a signal to the driver to
> start transmitting on the *hardware* queue that the txq points to. For
> some drivers (like Intel, right?) that's a 1-to-1 mapping, for others
> there are multiple TXQs being scheduled on the same HW-TXQ. So I think
> it's probably the right thing to do to just call next_txq(); if there's
> only a single TXQ scheduled it should be pretty cheap to do so.

Oh OK. So then the logic Alexander had makes sense.

>=20
> This logic has implications for putting "urgent" frames (like PS(?)) on
> TXQs as well, of course, but that needs to be handled somehow anyway...

But that probably then anyway needs to be handled in next_txq()?

johannes