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;
From:   Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@kernel.org>
To:     Johannes Berg <johannes@sipsolutions.net>,
        Alexander Wetzel <alexander@wetzel-home.de>,
        linux-wireless@vger.kernel.org
Subject: Re: [PATCH] wifi: mac80211: Use internal TX queues for all drivers
In-Reply-To: <bf6f9d8707021dc6017c7cb17805d63edea63333.camel@sipsolutions.net>
References: <20220926161303.13035-1-alexander@wetzel-home.de>
 <96e9ad692842853cfe92a7e5de18136baf20a492.camel@sipsolutions.net>
 <875ygyihhm.fsf@toke.dk>
 <bf6f9d8707021dc6017c7cb17805d63edea63333.camel@sipsolutions.net>
Date:   Wed, 05 Oct 2022 16:43:05 +0200
Message-ID: <87r0zmgwli.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Johannes Berg <johannes@sipsolutions.net> writes:

> 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.

Yup, I think so :)

>>=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()?

Yeah, just meant that comment as an "for future reference", it doesn't
impact this patch series (I think?)

-Toke