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;
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Tue, 24 Sep 2019 16:22:43 +0800
From:   Yibo Zhao <yiboz@codeaurora.org>
To:     =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
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: <87impj5lkm.fsf@toke.dk>
References: <1569223201-1490-1-git-send-email-yiboz@codeaurora.org>
 <1569223201-1490-4-git-send-email-yiboz@codeaurora.org>
 <87impj5lkm.fsf@toke.dk>
Message-ID: <2aab0bd944ee34751304a5f92b885113@codeaurora.org>
User-Agent: Roundcube Webmail/1.2.5
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 2019-09-23 18:55, Toke Høiland-Jørgensen wrote:
> Yibo Zhao <yiboz@codeaurora.org> writes:
> 
>> Not long after the start of multi-clients test, not a single station 
>> is
>> an eligible candidate for transmission since global virtual time(g_vt) 
>> is
>> smaller than the virtual airtime(s_vt) of all the stations. As a 
>> result,
>> the Tx has been blocked and throughput is quite low.
>> 
>> This may mainly due to sync mechanism and accumulative deviation from 
>> the
>> devision calculation of g_vt.
>> 
>> For example:
>> Suppose we have 50 clients in first round.
>> Round 1:
>> STA	weight	Tx_time_round  wt_sum	s_vt	g_vt  valid_for_next_Tx
>> 1	256	2048		12800	2048	2000	N
>> 2	256	2048			2048		N
>> .	.	.			.		.
>> .	.	.			.		.
>> .	.	.			.		.
>> 50	256	2048			2048		N
>> 
>> After this round, all the stations are not valid for next transmission 
>> due to
>> accumulative deviation.
>> 
>> And if we add a new #51,
>> STA	weight	Tx_time_round  wt_sum	s_vt	g_vt  valid_for_next_Tx
>> 1	256	2048		13056	2048	2020	N
>> 2	256	2048			2048		N
>> .	.	.			.
>> .	.	.			.
>> .	.	.			.
>> 50	256	2048			2048		N
>> 51	256	1024			2524		N
> 
> That's better :)
> 
>> Sync is done by:
>> max(g_vt of last round - grace period, s_vt)
>> and s_vt of #51 = max(2000 - 500, 0) + 1024 = 2524, and it is more 
>> than the final
>> g_vt of this round.
>> 
>> After this round, no more station is valid for transmission.
>> 
>> The real situation can be more complicate, above is one of the 
>> extremely case.
>> 
>> To avoid this situation to occur, the new proposal is:
>> 
>> - Increase the airtime grace period a little more to reduce the
>>   unexpected sync
>> 
>> - If global virtual time is less than the virtual airtime of any 
>> station,
>>   sync it to the airtime of first station in the red-black tree
>> 
>> - Round the division result
> 
> I can see why we need the second part (basically, this happens because 
> I
> forgot to add a check for "no eligible stations" in may_transmit(), 
> 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 
to FW has deep queue and few packets in TXQ at that time. So the TXQ is 
removed from the rbtree after dequeuing. When it is about to added back 
very soon after the removal, the g_vt might have gone a little far away 
from sta vt where sync is needed. With this sync, the station is forced 
to catch up with the g_vt, however, its chance for transmission has been 
reduced. I think 500us is quite a short period in multi-clients case.
> 
> -Toke

-- 
Yibo