Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
To:     Yibo Zhao <yiboz@codeaurora.org>
Cc:     linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
        linux-wireless-owner@vger.kernel.org
Subject: Re: [PATCH V3 3/4] mac80211: fix low throughput in multi-clients situation
In-Reply-To: <2aab0bd944ee34751304a5f92b885113@codeaurora.org>
References: <1569223201-1490-1-git-send-email-yiboz@codeaurora.org> <1569223201-1490-4-git-send-email-yiboz@codeaurora.org> <87impj5lkm.fsf@toke.dk> <2aab0bd944ee34751304a5f92b885113@codeaurora.org>
Date:   Tue, 24 Sep 2019 10:48:53 +0200
Message-ID: <87d0fq3wqy.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Yibo Zhao <yiboz@codeaurora.org> writes:

>> I can see why we need the second part (basically, this happens because=
=20
>> I
>> forgot to add a check for "no eligible stations" in may_transmit(),=20
>> like
>> the one in next_txq()). And rounding up the division result doesn't
>> hurt, I guess. But why does it help to change the grace period if we're
>> doing all the other stuff?
> In multi-clients case, it is possible a TXQ sometimes gets drained due=20
> to FW has deep queue and few packets in TXQ at that time. So the TXQ is=
=20
> removed from the rbtree after dequeuing. When it is about to added back=
=20
> very soon after the removal, the g_vt might have gone a little far away=
=20
> from sta vt where sync is needed. With this sync, the station is forced=
=20
> to catch up with the g_vt, however, its chance for transmission has been=
=20
> reduced. I think 500us is quite a short period in multi-clients case.

That's a good point, actually: Having the grace period be too small will
allow stations that leave and re-enter the queue to "skip ahead" and use
more than its share. However, I think it's a separate issue from what
this patch is about; so how about I just increase the grace period in
the next version of the base patch?

-Toke