[LEDE-DEV] [PATCH] ag71xx: Add some unlikely calls + rearange some stuff in hard_start_xmit.

[Roman Yeryomin]

On 2018-02-15 13:29, Felix Fietkau wrote:
> On 2018-02-14 16:20, Rosen Penev wrote:
>> On Tue, Feb 13, 2018 at 11:10 PM, Felix Fietkau <nbd at nbd.name> wrote:
>>> On 2018-02-13 23:53, Rosen Penev wrote:
>>>> Based on Qualcomm driver. Improves iperf3 throughput by ~20mbps on 
>>>> transmit on Archer C7v4.
>>>> 
>>>> Signed-off-by: Rosen Penev <rosenp at gmail.com>
>>>> ---
>>>>  .../drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c      | 14 
>>>> +++++++-------
>>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>> 
>>>> diff --git 
>>>> a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c 
>>>> b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
>>>> index 95682b7641..d32f220178 100644
>>>> --- 
>>>> a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
>>>> +++ 
>>>> b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
>>>> @@ -797,11 +797,14 @@ static netdev_tx_t 
>>>> ag71xx_hard_start_xmit(struct sk_buff *skb,
>>>>       if (ag71xx_has_ar8216(ag))
>>>>               ag71xx_add_ar8216_header(ag, skb);
>>>> 
>>>> -     if (skb->len <= 4) {
>>>> +     dma_cache_sync (NULL, skb->data, skb->len, DMA_TO_DEVICE);
>>> The use of dma_cache_sync here makes no sense, since it's the wrong 
>>> API.
>>> Also, effectively it results in the same kind of cache flush as the 
>>> one
>>> that's done by the DMA mapping done later.
>> 
>> From my reading of dma_cache_flush, I agree. However that's the part
>> of this patch that gives the biggest speedup. Before this patch, I
>> tested just adding that and it worked. I can back this up with
>> benchmarks later on.
> In my test I quite often encountered big differences in throughput from
> minor (and often very much unrelated) changes. I haven't found the real
> source of these differences yet, so it's hard to know which changes
> really help in the long run. My best guess is that some changes affect
> the alignment of critical functions and thus affect the icache 
> footprint
> somehow.

Exactly my impression I got when have made a mistake and jumped into 
this rabbit hole some time ago.
Trying to port the changes QCA did I was getting random tput changes, 
often not logical at all. Then I started to suspect it's related to code 
alignment and MIPS architecture features.
I realized that after I got down to (ar8327) switch port buffers and 
wrote few additional functions to control them. And got +50Mbps stable 
right away, not even touching default buffer settings.
Functions were never run, not even at switch init.

> Until I see a reasonable explanation of how this change helps, I'm 
> going
> to assume it's the same kind of random change fluctuation that I've 
> seen
> so often and NACK this change.
> 
> - Felix
> 
> _______________________________________________
> Lede-dev mailing list
> Lede-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev