Il 29/04/2013 10:26, Conor O'Gorman ha scritto:
On Sun, 2013-04-28 at 16:09 +0200, Luca dariz wrote:
Il 28/04/2013 13:45, Luca dariz ha scritto:
Il 26/04/2013 22:00, Luca dariz ha scritto:
Il 26/04/2013 16:35, Conor O'Gorman ha scritto:
On Fri, 2013-04-26 at 14:13 +0200, Luca dariz wrote:
Use a tasklet to handle incoming packets. Fix #12917.

Incoming packets are now processes in a tasklet instead of in the
irq handler; this should improve latency.

This patch is based on a previous version of ltq-atm driver, which
did implement a tasklet.

It has been tested on a arv4518pw with a
Lantiq Danube for about a month and it seems to work well.

And how much 'better' is it?


I did't measure latency with this patch and without it, so i can't tell
exactly.

Luca

A quick test with cyclictest
(https://rt.wiki.kernel.org/index.php/Cyclictest):

without the patch:

cmdline: ./cyclictest  -n
# /dev/cpu_dma_latency set to 0us
policy: other/other: loadavg: 0.51 0.33 0.27 1/43 3241
T: 0 ( 3241) P: 0 I:1000 C: 144288 Min: 40 Act: 93 Avg: 163 Max: 4456

with the patch:

cmdline: ./cyclictest -n
# /dev/cpu_dma_latency set to 0us
policy: other/other: loadavg: 0.78 0.44 0.22 1/44 4981
T: 0 ( 3452) P: 0 I:1000 C:  82427 Min: 39 Act: 199 Avg: 252 Max: 18179

All latencies are measures in microseconds.

Luca

I forgot: during the test, a big download was in progress, to stress the
rx path a bit.


Thank you for providing more information. I'll first say that I am not a
gatekeeper on this code, but a user, so I try to review Lantiq specific
changes and make hopefully relevant comments.

I am a user too, i started looking at the code to find the origin of the warning reported in the ticket :)


As I understand the results, the average and maximum latency statistics
have increased, with a small decrease to minimum?

Sorry there is a mistake in my previous mail, the average amd maximum latencies *decrease* with the patch.


Looking at the system as a whole, it is a router whose primary function
will be to move packets from the adsl (atm) interface to ethernet or
pci-wifi. Given that under load the ethernet/wifi will most likely use
napi interrupt moderation, the vast majority of interrupts firing will
be the adsl/atm. Which latencies, therefore, are you concerned about?


The latencies measured with cyclictest are timer latencies. They decrease since under high load this patch reduces the amount of code that runs with interrupts disabled.
This patch reduces the number of atm interrupts too, under high load.

Ideally the adsl/atm driver on lantiq would use napi method, but I
haven't investigated it's structure.

I haven't found any atm drivers using napi yet, i should do a bit more research to implement it. I think too that would be the ideal approach, this patch does something similar in the sense that the code in the tasklet keeps polling for incoming packets before re-enabling the interrupt. However IIRC the napi tasklet runs with higher priority than normal tasklet.


I have looked at the bug report you reference, but feel you have side
stepped the original problem with an alternative structure.

The original problem was that the atm code runs in the irq handler when it should run in the bottom half. This patch addresses exactly this problem.


Again, just my thoughts.

Conor


Thanks for your comments,
Luca
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to