[PATCH net-next 00/10] korina cleanups/optimizations

[PATCH net-next 00/10] korina cleanups/optimizations

[Roman Yeryomin]

TX optimizations have led to ~15% performance increase (35->40Mbps)
in local tx usecase (tested with iperf v3.2).

Roman Yeryomin (10):
  net: korina: optimize korina_send_packet logic
  net: korina: reorder functions
  net: korina: introduce and use interrupt disable/enable helpers
  net: korina: optimize tx/rx interrupt handlers
  net: korina: remove unused korina_private members
  net: korina: optimize tx descriptor flags processing
  net: korina: move tx to napi context
  net: korina: optimize tx_full condition
  net: korina: use dma api instead of dma_cache_* functions
  net: korina: bump version

 drivers/net/ethernet/korina.c | 503 +++++++++++++++++-------------------------
 1 file changed, 204 insertions(+), 299 deletions(-)

-- 
2.11.0

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[Florian Fainelli]

On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv> wrote:
>TX optimizations have led to ~15% performance increase (35->40Mbps)
>in local tx usecase (tested with iperf v3.2).

Could you avoid empty commit messages and write a paragraph or two for each commit that explains what and why are you changing? The changes look fine but they lack any explanation.

>
>Roman Yeryomin (10):
>  net: korina: optimize korina_send_packet logic
>  net: korina: reorder functions
>  net: korina: introduce and use interrupt disable/enable helpers
>  net: korina: optimize tx/rx interrupt handlers
>  net: korina: remove unused korina_private members
>  net: korina: optimize tx descriptor flags processing
>  net: korina: move tx to napi context
>  net: korina: optimize tx_full condition
>  net: korina: use dma api instead of dma_cache_* functions
>  net: korina: bump version
>
>drivers/net/ethernet/korina.c | 503
>+++++++++++++++++-------------------------
> 1 file changed, 204 insertions(+), 299 deletions(-)


-- 
Florian

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[Roman Yeryomin]

On 2017-10-15 19:38, Florian Fainelli wrote:
> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv> 
> wrote:
>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>> in local tx usecase (tested with iperf v3.2).
> 
> Could you avoid empty commit messages and write a paragraph or two for
> each commit that explains what and why are you changing? The changes
> look fine but they lack any explanation.

I thought that short descriptions are already self explanatory and just 
didn't know what to write more.
To me they are very obvious.
Do you really want me to make up something more?

>> 
>> Roman Yeryomin (10):
>>  net: korina: optimize korina_send_packet logic
>>  net: korina: reorder functions
>>  net: korina: introduce and use interrupt disable/enable helpers
>>  net: korina: optimize tx/rx interrupt handlers
>>  net: korina: remove unused korina_private members
>>  net: korina: optimize tx descriptor flags processing
>>  net: korina: move tx to napi context
>>  net: korina: optimize tx_full condition
>>  net: korina: use dma api instead of dma_cache_* functions
>>  net: korina: bump version
>> 
>> drivers/net/ethernet/korina.c | 503
>> +++++++++++++++++-------------------------
>> 1 file changed, 204 insertions(+), 299 deletions(-)

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[Florian Fainelli]

On October 15, 2017 9:46:02 AM PDT, Roman Yeryomin <roman@advem.lv> wrote:
>On 2017-10-15 19:38, Florian Fainelli wrote:
>> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv> 
>> wrote:
>>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>>> in local tx usecase (tested with iperf v3.2).
>> 
>> Could you avoid empty commit messages and write a paragraph or two
>for
>> each commit that explains what and why are you changing? The changes
>> look fine but they lack any explanation.
>
>I thought that short descriptions are already self explanatory and just
>
>didn't know what to write more.
>To me they are very obvious.

You can't assume what is obvious to you will be to the reviewers.

>Do you really want me to make up something more?

It's up to David as a maintainer to decide, but if this was up to me I would ask you to respin with a least a paragraph for each commit (except the version bump) why you did these changes.

>
>>> 
>>> Roman Yeryomin (10):
>>>  net: korina: optimize korina_send_packet logic
>>>  net: korina: reorder functions
>>>  net: korina: introduce and use interrupt disable/enable helpers
>>>  net: korina: optimize tx/rx interrupt handlers
>>>  net: korina: remove unused korina_private members
>>>  net: korina: optimize tx descriptor flags processing
>>>  net: korina: move tx to napi context
>>>  net: korina: optimize tx_full condition
>>>  net: korina: use dma api instead of dma_cache_* functions
>>>  net: korina: bump version
>>> 
>>> drivers/net/ethernet/korina.c | 503
>>> +++++++++++++++++-------------------------
>>> 1 file changed, 204 insertions(+), 299 deletions(-)


-- 
Florian

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[David Miller]

From: Roman Yeryomin <roman@advem.lv>
Date: Sun, 15 Oct 2017 19:46:02 +0300

> On 2017-10-15 19:38, Florian Fainelli wrote:
>> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv>
>> wrote:
>>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>>> in local tx usecase (tested with iperf v3.2).
>> Could you avoid empty commit messages and write a paragraph or two for
>> each commit that explains what and why are you changing? The changes
>> look fine but they lack any explanation.
> 
> I thought that short descriptions are already self explanatory and
> just didn't know what to write more.

"Optimize TX handlers."

In what way?  Why?  How are things improved?  Is it measurable?
etc.

RE: [PATCH net-next 00/10] korina cleanups/optimizations

[David Laight]

From: Roman Yeryomin
> Sent: 15 October 2017 17:22
> TX optimizations have led to ~15% performance increase (35->40Mbps)
> in local tx usecase (tested with iperf v3.2).

Indicate which patches give the improvement.
IIRC some just changed the source without changing what the code really did.
(Not a problem in itself.)

...
>   net: korina: optimize tx/rx interrupt handlers

You'd probably get a noticeable improvement from caching the
value of the interrupt mask - instead of reading it from the hardware.
...

	David

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[Roman Yeryomin]

On 2017-10-16 00:05, David Miller wrote:
> From: Roman Yeryomin <roman@advem.lv>
> Date: Sun, 15 Oct 2017 19:46:02 +0300
> 
>> On 2017-10-15 19:38, Florian Fainelli wrote:
>>> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv>
>>> wrote:
>>>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>>>> in local tx usecase (tested with iperf v3.2).
>>> Could you avoid empty commit messages and write a paragraph or two 
>>> for
>>> each commit that explains what and why are you changing? The changes
>>> look fine but they lack any explanation.
>> 
>> I thought that short descriptions are already self explanatory and
>> just didn't know what to write more.
> 
> "Optimize TX handlers."
> 
> In what way?  Why?  How are things improved?  Is it measurable?
> etc.

OK, got the idea.
However I think I would need some help with measuring performance 
difference reliably.
On this CPU iperf3 tx takes most of the time (like 80-90%), thus even 
well optimized changes will be hard to see with iperf3 alone.
I've tried using pktgen module. Although it shows much better numbers 
than iperf3 (~95Mbps vs. 40), results don't look like very 
stable/reliable, pps may differ by 10-15% easily between different runs.
perf. I have limited experience with it but if I understand correctly, 
this CPU doesn't support neither cycles nor instructions counters. So 
not sure if perf would be useful here.

  Performance counter stats for 'system wide':

       10387.717082      cpu-clock (msec)          #    1.000 CPUs 
utilized
               2941      context-switches          #    0.283 K/sec
                  0      cpu-migrations            #    0.000 K/sec
                 60      page-faults               #    0.006 K/sec
    <not supported>      cycles
    <not supported>      instructions
    <not supported>      branches
    <not supported>      branch-misses

       10.388087500 seconds time elapsed


What are the suggestions?


Regards,
Roman

RE: [PATCH net-next 00/10] korina cleanups/optimizations

[Roman Yeryomin]

On 2017-10-16 15:59, David Laight wrote:
> From: Roman Yeryomin
>> Sent: 15 October 2017 17:22
>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>> in local tx usecase (tested with iperf v3.2).
> 
> Indicate which patches give the improvement.
> IIRC some just changed the source without changing what the code really 
> did.
> (Not a problem in itself.)

Yes, this was mostly a cleanup.

> ...
>>   net: korina: optimize tx/rx interrupt handlers
> 
> You'd probably get a noticeable improvement from caching the
> value of the interrupt mask - instead of reading it from the hardware.
> ...
> 

Yeah, tried that, but have some problems measuring the results (that is 
don't see any improvements with iperf3, see other email)
But thanks for comment!

Regards,
Roman

Re: [PATCH net-next 00/10] korina cleanups/optimizations

[Roman Yeryomin]

On 2017-10-22 23:57, Roman Yeryomin wrote:
> On 2017-10-16 00:05, David Miller wrote:
>> From: Roman Yeryomin <roman@advem.lv>
>> Date: Sun, 15 Oct 2017 19:46:02 +0300
>> 
>>> On 2017-10-15 19:38, Florian Fainelli wrote:
>>>> On October 15, 2017 9:22:26 AM PDT, Roman Yeryomin <roman@advem.lv>
>>>> wrote:
>>>>> TX optimizations have led to ~15% performance increase (35->40Mbps)
>>>>> in local tx usecase (tested with iperf v3.2).
>>>> Could you avoid empty commit messages and write a paragraph or two 
>>>> for
>>>> each commit that explains what and why are you changing? The changes
>>>> look fine but they lack any explanation.
>>> 
>>> I thought that short descriptions are already self explanatory and
>>> just didn't know what to write more.
>> 
>> "Optimize TX handlers."
>> 
>> In what way?  Why?  How are things improved?  Is it measurable?
>> etc.
> 
> OK, got the idea.
> However I think I would need some help with measuring performance
> difference reliably.
> On this CPU iperf3 tx takes most of the time (like 80-90%), thus even
> well optimized changes will be hard to see with iperf3 alone.
> I've tried using pktgen module. Although it shows much better numbers
> than iperf3 (~95Mbps vs. 40), results don't look like very
> stable/reliable, pps may differ by 10-15% easily between different
> runs.
> perf. I have limited experience with it but if I understand correctly,
> this CPU doesn't support neither cycles nor instructions counters. So
> not sure if perf would be useful here.
> 
>  Performance counter stats for 'system wide':
> 
>       10387.717082      cpu-clock (msec)          #    1.000 CPUs 
> utilized
>               2941      context-switches          #    0.283 K/sec
>                  0      cpu-migrations            #    0.000 K/sec
>                 60      page-faults               #    0.006 K/sec
>    <not supported>      cycles
>    <not supported>      instructions
>    <not supported>      branches
>    <not supported>      branch-misses
> 
>       10.388087500 seconds time elapsed
> 
> 
> What are the suggestions?

Any ideas?
Or I can just comment on the patch(es) which gave apparent performance 
improvement (as seen with iperf3) and others mark as cleanup.

Regards,
Roman