| Message ID | 20240729150049.1924352-1-paul.barker.ct@bp.renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Backport ravb driver performance improvements | expand |
Hi! > This series includes the performance improvements present in Linux > v6.11-rc1 along with dependencies. That's a bit aggressive timing, is it? -rc1 does not really get much testing. Please avoid blank lines in the signoff area. > RZ/Five: > * 19% increase in TCP TX bandwidth > * 225% increase in TCP RX bandwidth > * >10,000% increase in UDP RX bandwidth > (UDP RX bandwidth was <100kbps and unstable with v6.1.101-cip25 > so it's difficult to get an exact figure here) > * 6% loss in UDP TX bandwidth > > Small losses in UDP TX bandwidth are considered acceptable to achieve > the above improvements in other test cases. If UDP TX bandwidth must be > maximised for a particular use case, NAPI threaded mode can be disabled > at runtime via sysfs writes. And this wories me. One of the configurations works so badly that tests don't even finish. You automatically select "right" configurations in some cases, but that can't be complete fix. It would be good to find out what is going on there, and fix not-threaded so that it works well enough to finish tests. Anyway... this is still not reason to block the patches, so I can apply this if it passes testing and there are no other comments. Best regards, Pavel
On 31/07/2024 08:54, Pavel Machek wrote: > Hi! > >> This series includes the performance improvements present in Linux >> v6.11-rc1 along with dependencies. > > That's a bit aggressive timing, is it? -rc1 does not really get much > testing. Hi Pavel, Please feel free to delay merging these patches for a couple of rc cycles if you need to. > > Please avoid blank lines in the signoff area. > >> RZ/Five: >> * 19% increase in TCP TX bandwidth >> * 225% increase in TCP RX bandwidth >> * >10,000% increase in UDP RX bandwidth >> (UDP RX bandwidth was <100kbps and unstable with v6.1.101-cip25 >> so it's difficult to get an exact figure here) >> * 6% loss in UDP TX bandwidth >> >> Small losses in UDP TX bandwidth are considered acceptable to achieve >> the above improvements in other test cases. If UDP TX bandwidth must be >> maximised for a particular use case, NAPI threaded mode can be disabled >> at runtime via sysfs writes. > > And this wories me. One of the configurations works so badly that > tests don't even finish. You automatically select "right" > configurations in some cases, but that can't be complete fix. It would > be good to find out what is going on there, and fix not-threaded so > that it works well enough to finish tests. These changes were submitted to the mainline kernel after extensive investigation and testing, they represent the best configuration that could be found. The UDP RX results were obtained using `iperf3 -c <server> -b0 -u -R`, where the remote server has a 1Gbps network connection and generates over 80k packets/second. In the default configuration this leads to a very high IRQ & softirq load, on the RZ/Five this load is enough to starve all other processes of CPU time and cause the test to crash. Both SW IRQ coalescing and NAPI threaded mode are needed to avoid the test crash on the RZ/Five, neither of these alone resolves the issue. Thanks,
Hi! > >> This series includes the performance improvements present in Linux > >> v6.11-rc1 along with dependencies. > > > > That's a bit aggressive timing, is it? -rc1 does not really get much > > testing. > > Hi Pavel, > > Please feel free to delay merging these patches for a couple of rc > cycles if you need to. I'm getting: drivers/net/ethernet/renesas/ravb_main.c: In function 'ravb_rx_gbeth': 2534 drivers/net/ethernet/renesas/ravb_main.c:869:33: error: implicit declaration of function 'skb_mark_for_recycle' [-Werror=implicit-function-declaration] 2535 869 | skb_mark_for_recycle(skb); 2536 | ^~~~~~~~~~~~~~~~~~~~ 2537 https://gitlab.com/cip-project/cip-kernel/linux-cip/-/jobs/7484664109 https://gitlab.com/cip-project/cip-kernel/linux-cip/-/pipelines/1397249930 When attempting to test this. Fix is probably as simple as: diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 2a48ffc08464f..25a15cda707a0 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -31,6 +31,7 @@ #include <linux/sys_soc.h> #include <linux/reset.h> #include <linux/math64.h> +#include <linux/skbuff.h> #include <net/ip.h> #include <net/page_pool.h> But... what about this: In about two weeks, you submit new version with blank line replaced by Reviewed-by: Pavel Machek <pavel@denx.de> ? Best regards, Pavel
On 01/08/2024 20:48, Pavel Machek wrote: > Hi! > >>>> This series includes the performance improvements present in Linux >>>> v6.11-rc1 along with dependencies. >>> >>> That's a bit aggressive timing, is it? -rc1 does not really get much >>> testing. >> >> Hi Pavel, >> >> Please feel free to delay merging these patches for a couple of rc >> cycles if you need to. > > I'm getting: > > drivers/net/ethernet/renesas/ravb_main.c: In function 'ravb_rx_gbeth': > 2534 > drivers/net/ethernet/renesas/ravb_main.c:869:33: error: implicit declaration of function 'skb_mark_for_recycle' [-Werror=implicit-function-declaration] > 2535 > 869 | skb_mark_for_recycle(skb); > 2536 > | ^~~~~~~~~~~~~~~~~~~~ > 2537 > > https://gitlab.com/cip-project/cip-kernel/linux-cip/-/jobs/7484664109 > https://gitlab.com/cip-project/cip-kernel/linux-cip/-/pipelines/1397249930 > > When attempting to test this. Fix is probably as simple as: > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c > index 2a48ffc08464f..25a15cda707a0 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -31,6 +31,7 @@ > #include <linux/sys_soc.h> > #include <linux/reset.h> > #include <linux/math64.h> > +#include <linux/skbuff.h> > #include <net/ip.h> > #include <net/page_pool.h> Hi Pavel, I've left this for a while, to my knowledge no issues have been reported with these changes in the v6.11-rc series so far. The issue above is not due to a missing include, it's due to the skb_mark_for_recycle() function only being defined when CONFIG_PAGE_POOL is enabled. I'll open a merge request to enable that in the relevant configs in cip-kernel-config. Once that is merged, we can proceed with this backport. Thanks,
On 27/08/2024 13:54, Paul Barker wrote: > On 01/08/2024 20:48, Pavel Machek wrote: >> Hi! >> >>>>> This series includes the performance improvements present in Linux >>>>> v6.11-rc1 along with dependencies. >>>> >>>> That's a bit aggressive timing, is it? -rc1 does not really get much >>>> testing. >>> >>> Hi Pavel, >>> >>> Please feel free to delay merging these patches for a couple of rc >>> cycles if you need to. >> >> I'm getting: >> >> drivers/net/ethernet/renesas/ravb_main.c: In function 'ravb_rx_gbeth': >> 2534 >> drivers/net/ethernet/renesas/ravb_main.c:869:33: error: implicit declaration of function 'skb_mark_for_recycle' [-Werror=implicit-function-declaration] >> 2535 >> 869 | skb_mark_for_recycle(skb); >> 2536 >> | ^~~~~~~~~~~~~~~~~~~~ >> 2537 >> >> https://gitlab.com/cip-project/cip-kernel/linux-cip/-/jobs/7484664109 >> https://gitlab.com/cip-project/cip-kernel/linux-cip/-/pipelines/1397249930 >> >> When attempting to test this. Fix is probably as simple as: >> >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >> index 2a48ffc08464f..25a15cda707a0 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c >> @@ -31,6 +31,7 @@ >> #include <linux/sys_soc.h> >> #include <linux/reset.h> >> #include <linux/math64.h> >> +#include <linux/skbuff.h> >> #include <net/ip.h> >> #include <net/page_pool.h> > > Hi Pavel, > > I've left this for a while, to my knowledge no issues have been reported > with these changes in the v6.11-rc series so far. > > The issue above is not due to a missing include, it's due to the > skb_mark_for_recycle() function only being defined when CONFIG_PAGE_POOL > is enabled. I'll open a merge request to enable that in the relevant > configs in cip-kernel-config. Once that is merged, we can proceed with > this backport. Actually, the solution is to backport commit 721478fe6a5c [1], then PAGE_POOL is enabled whenever RAVB is enabled. I'll send a v2 series with that included. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=721478fe6a5cd243e289105e3fdd0ecdd9d39ac3
This series includes the performance improvements present in Linux v6.11-rc1 along with dependencies. The patch commit messages have not been changed as part of this backport, so they refer to performance achieved during testing with the upstream kernel. The performance improvement achieved by applying this backport series on top of v6.1.101-cip25 is as follows, measured using iperf3: RZ/G2L: * Similar TX/RX bandwidth * 50% less CPU usage during TCP RX test * 21% less CPU usage during UDP RX test RZ/G2UL: * 6% increase in TCP TX bandwidth * 34% increase in TCP RX bandwidth * 500% increase in UDP RX bandwidth * 24% loss in UDP TX bandwidth RZ/G3S: * 38% increase in TCP TX bandwidth * 48% increase in TCP RX bandwidth * 481% increase in UDP RX bandwidth * 17% loss in UDP TX bandwidth RZ/Five: * 19% increase in TCP TX bandwidth * 225% increase in TCP RX bandwidth * >10,000% increase in UDP RX bandwidth (UDP RX bandwidth was <100kbps and unstable with v6.1.101-cip25 so it's difficult to get an exact figure here) * 6% loss in UDP TX bandwidth Small losses in UDP TX bandwidth are considered acceptable to achieve the above improvements in other test cases. If UDP TX bandwidth must be maximised for a particular use case, NAPI threaded mode can be disabled at runtime via sysfs writes. Heiner Kallweit (1): net: add netdev_sw_irq_coalesce_default_on() Paul Barker (7): net: ravb: Simplify poll & receive functions net: ravb: Align poll function with NAPI docs net: ravb: Refactor RX ring refill net: ravb: Refactor GbEth RX code path net: ravb: Enable SW IRQ Coalescing for GbEth net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP net: ravb: Allocate RX buffers via page pool drivers/net/ethernet/renesas/ravb.h | 14 +- drivers/net/ethernet/renesas/ravb_main.c | 488 +++++++++++++---------- include/linux/netdevice.h | 1 + net/core/dev.c | 16 + 4 files changed, 298 insertions(+), 221 deletions(-)