| Message ID | 20240124095814.1882509-2-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: bonding: use busy/slowwait when waiting | expand |
On 1/24/24 10:58, Hangbin Liu wrote: > Add slowwait functions to wait for some operations that may need a long time > to finish. The busywait executes the cmd too fast, which is kind of wasting > cpu in this scenario. At the same time, if shell debugging is enabled with > `set -x`. the busywait will output too much logs. > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 8a61464ab6eb..07faedc2071b 100644 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -41,6 +41,7 @@ fi > # Kselftest framework requirement - SKIP code is 4. > ksft_skip=4 > > +# timeout in milliseconds > busywait() > { > local timeout=$1; shift > @@ -64,6 +65,32 @@ busywait() > done > } > > +# timeout in seconds > +slowwait() > +{ > + local timeout=$1; shift > + > + local start_time="$(date -u +%s)" > + while true > + do > + local out > + out=$("$@") > + local ret=$? > + if ((!ret)); then it would be nice to have some exit code used (or just reserved) for "operation failed, no need to wait, fail the test please" similar to the xargs, eg: 126 if the command cannot be run > + echo -n "$out" > + return 0 > + fi > + > + local current_time="$(date -u +%s)" > + if ((current_time - start_time > timeout)); then > + echo -n "$out" > + return 1 > + fi > + > + sleep 1 I see that `sleep 1` is simplest correct impl, but it's tempting to suggest exponential back-off, perhaps with saturation at 15 (but then you will have to cap last sleep to don't exceed timeout by more than 1). > + done > +} > + > ############################################################################## > # Sanity checks > > @@ -505,6 +532,15 @@ busywait_for_counter() > busywait "$timeout" until_counter_is ">= $((base + delta))" "$@" > } > > +slowwait_for_counter() > +{ > + local timeout=$1; shift > + local delta=$1; shift > + > + local base=$("$@") > + slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@" > +} > + > setup_wait_dev() > { > local dev=$1; shift just nitpicks so I will provide my RB in case you want to ignore ;)
Hi Przemek, Thanks for your review. On Wed, Jan 24, 2024 at 02:25:57PM +0100, Przemek Kitszel wrote: > > +# timeout in seconds > > +slowwait() > > +{ > > + local timeout=$1; shift > > + > > + local start_time="$(date -u +%s)" > > + while true > > + do > > + local out > > + out=$("$@") > > + local ret=$? > > + if ((!ret)); then > > it would be nice to have some exit code used (or just reserved) for > "operation failed, no need to wait, fail the test please" > similar to the xargs, eg: > 126 if the command cannot be run Return directly instead of wait may confuse the caller. Maybe we can add a parameter and let user decide whether to wait if return some value. e.g. slowwait nowait 126 $timeout $commands > > > + echo -n "$out" > > + return 0 > > + fi > > + > > + local current_time="$(date -u +%s)" > > + if ((current_time - start_time > timeout)); then > > + echo -n "$out" > > + return 1 > > + fi > > + > > + sleep 1 > > I see that `sleep 1` is simplest correct impl, but it's tempting to > suggest exponential back-off, perhaps with saturation at 15 > > (but then you will have to cap last sleep to don't exceed timeout by > more than 1). Do you mean sleep longer when cmd exec failed? I'm not sure if it's a good idea as the caller still wants to return ASAP when cmd exec succeeds. Thanks Hangbin
On Fri, 2024-01-26 at 17:22 +0800, Hangbin Liu wrote: > On Wed, Jan 24, 2024 at 02:25:57PM +0100, Przemek Kitszel wrote: > > > > > + echo -n "$out" > > > + return 0 > > > + fi > > > + > > > + local current_time="$(date -u +%s)" > > > + if ((current_time - start_time > timeout)); then > > > + echo -n "$out" > > > + return 1 > > > + fi > > > + > > > + sleep 1 > > > > I see that `sleep 1` is simplest correct impl, but it's tempting to > > suggest exponential back-off, perhaps with saturation at 15 > > > > (but then you will have to cap last sleep to don't exceed timeout by > > more than 1). > > Do you mean sleep longer when cmd exec failed? I'm not sure if it's a good > idea as the caller still wants to return ASAP when cmd exec succeeds. I think exponential backoff is not needed here: we don't care about minimizing the CPU usage overhead, and there should not be 'collision' issues. Instead I *think* you could use a smaller sleep, e.g. sleep 0.1 and hopefully reduce the latency even further. Cheers, Paolo
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 8a61464ab6eb..07faedc2071b 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -41,6 +41,7 @@ fi # Kselftest framework requirement - SKIP code is 4. ksft_skip=4 +# timeout in milliseconds busywait() { local timeout=$1; shift @@ -64,6 +65,32 @@ busywait() done } +# timeout in seconds +slowwait() +{ + local timeout=$1; shift + + local start_time="$(date -u +%s)" + while true + do + local out + out=$("$@") + local ret=$? + if ((!ret)); then + echo -n "$out" + return 0 + fi + + local current_time="$(date -u +%s)" + if ((current_time - start_time > timeout)); then + echo -n "$out" + return 1 + fi + + sleep 1 + done +} + ############################################################################## # Sanity checks @@ -505,6 +532,15 @@ busywait_for_counter() busywait "$timeout" until_counter_is ">= $((base + delta))" "$@" } +slowwait_for_counter() +{ + local timeout=$1; shift + local delta=$1; shift + + local base=$("$@") + slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@" +} + setup_wait_dev() { local dev=$1; shift
Add slowwait functions to wait for some operations that may need a long time to finish. The busywait executes the cmd too fast, which is kind of wasting cpu in this scenario. At the same time, if shell debugging is enabled with `set -x`. the busywait will output too much logs. Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- tools/testing/selftests/net/forwarding/lib.sh | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+)