by Jesper Dangaard Brouer

[permalink] [raw]

Subject: Re: [PATCH] mm: page_alloc: High-order per-cpu page allocator v7

On Thu, 8 Dec 2016 15:11:01 +0000
Mel Gorman <[email protected]> wrote:

> On Thu, Dec 08, 2016 at 03:48:13PM +0100, Jesper Dangaard Brouer wrote:
> > On Thu, 8 Dec 2016 11:06:56 +0000
> > Mel Gorman <[email protected]> wrote:
> >
> > > On Thu, Dec 08, 2016 at 11:43:08AM +0100, Jesper Dangaard Brouer wrote:
> > > > > That's expected. In the initial sniff-test, I saw negligible packet loss.
> > > > > I'm waiting to see what the full set of network tests look like before
> > > > > doing any further adjustments.
> > > >
> > > > For netperf I will not recommend adjusting the global default
> > > > /proc/sys/net/core/rmem_default as netperf have means of adjusting this
> > > > value from the application (which were the options you setup too low
> > > > and just removed). I think you should keep this as the default for now
> > > > (unless Eric says something else), as this should cover most users.
> > > >
> > >
> > > Ok, the current state is that buffer sizes are only set for netperf
> > > UDP_STREAM and only when running over a real network. The values selected
> > > were specific to the network I had available so milage may vary.
> > > localhost is left at the defaults.
> >
> > Looks like you made a mistake when re-implementing using buffer sizes
> > for netperf.
>
> We appear to have a disconnect. This was reintroduced in response to your
> comment "For netperf I will not recommend adjusting the global default
> /proc/sys/net/core/rmem_default as netperf have means of adjusting this
> value from the application".
>
> My understanding was that netperfs means was the -s and -S switches for
> send and recv buffers so I reintroduced them and avoided altering
> [r|w]mem_default.
>
> Leaving the defaults resulted in some UDP packet loss on a 10GbE network
> so some upward adjustment.
>
> From my perspective, either adjusting [r|w]mem_default or specifying -s
> -S works for the UDP_STREAM issue but using the switches meant only this
> is affected and other loads like sockperf and netpipe will need to be
> evaluated separately which I don't mind doing.
>
> > See patch below signature.
> >
> > Besides I think you misunderstood me, you can adjust:
> > sysctl net.core.rmem_max
> > sysctl net.core.wmem_max
> >
> > And you should if you plan to use/set 851968 as socket size for UDP
> > remote tests, else you will be limited to the "max" values (212992 well
> > actually 425984 2x default value, for reasons I cannot remember)
> >
>
> The intent is to use the larger values to avoid packet loss on
> UDP_STREAM.

We do seem to misunderstand each-other.
I was just pointing out two things:

1. Notice the difference between "max" and "default" proc setting.
Only adjust the "max" setting.

2. There was simple BASH-shell script error in your commit.
Patch below fix it.

[PATCH] mmtests: actually use variable SOCKETSIZE_OPT

From: Jesper Dangaard Brouer <[email protected]>

commit 7f16226577b2 ("netperf: Set remote and local socket max buffer
sizes") removed netperf's setting of the socket buffer sizes and
instead used global /proc/sys settings.

commit de9f8cdb7146 ("netperf: Only adjust socket sizes for
UDP_STREAM") re-added explicit netperf setting socket buffer sizes for
remote-host testing (saved in SOCKETSIZE_OPT). Only problem is this
variable is not used after commit 7f16226577b2.

Simply use $SOCKETSIZE_OPT when invoking netperf command.

Signed-off-by: Jesper Dangaard Brouer <[email protected]>
---
shellpack_src/src/netperf/netperf-bench | 2 +-
shellpacks/shellpack-bench-netperf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/shellpack_src/src/netperf/netperf-bench b/shellpack_src/src/netperf/netperf-bench
index 8e7d02864c4a..b2820610936e 100755
--- a/shellpack_src/src/netperf/netperf-bench
+++ b/shellpack_src/src/netperf/netperf-bench
@@ -93,7 +93,7 @@ mmtests_server_ctl start --serverside-name $PROTOCOL-$SIZE
-t $PROTOCOL \
-i 3,3 -I 95,5 \
-H $SERVER_HOST \
- -- $MSGSIZE_OPT $EXTRA \
+ -- $SOCKETSIZE_OPT $MSGSIZE_OPT $EXTRA \
2>&1 | tee $LOGDIR_RESULTS/$PROTOCOL-${SIZE}.$ITERATION \
|| die Failed to run netperf
monitor_post_hook $LOGDIR_RESULTS $SIZE
diff --git a/shellpacks/shellpack-bench-netperf b/shellpacks/shellpack-bench-netperf
index 2ce26ba39f1b..7356082d5a78 100755
--- a/shellpacks/shellpack-bench-netperf
+++ b/shellpacks/shellpack-bench-netperf
@@ -190,7 +190,7 @@ for ITERATION in `seq 1 $ITERATIONS`; do
-t $PROTOCOL \
-i 3,3 -I 95,5 \
-H $SERVER_HOST \
- -- $MSGSIZE_OPT $EXTRA \
+ -- $SOCKETSIZE_OPT $MSGSIZE_OPT $EXTRA \
2>&1 | tee $LOGDIR_RESULTS/$PROTOCOL-${SIZE}.$ITERATION \
|| die Failed to run netperf
monitor_post_hook $LOGDIR_RESULTS $SIZE

--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer

2016-12-08 17:39:32

by Mel Gorman

[permalink] [raw]

Subject: Re: [PATCH] mm: page_alloc: High-order per-cpu page allocator v7

On Thu, Dec 08, 2016 at 06:19:51PM +0100, Jesper Dangaard Brouer wrote:
> > > See patch below signature.
> > >
> > > Besides I think you misunderstood me, you can adjust:
> > > sysctl net.core.rmem_max
> > > sysctl net.core.wmem_max
> > >
> > > And you should if you plan to use/set 851968 as socket size for UDP
> > > remote tests, else you will be limited to the "max" values (212992 well
> > > actually 425984 2x default value, for reasons I cannot remember)
> > >
> >
> > The intent is to use the larger values to avoid packet loss on
> > UDP_STREAM.
>
> We do seem to misunderstand each-other.
> I was just pointing out two things:
>
> 1. Notice the difference between "max" and "default" proc setting.
> Only adjust the "max" setting.
>
> 2. There was simple BASH-shell script error in your commit.
> Patch below fix it.
>

Understood now.

> [PATCH] mmtests: actually use variable SOCKETSIZE_OPT
>
> From: Jesper Dangaard Brouer <[email protected]>
>

Applied, thanks!

--
Mel Gorman
SUSE Labs