Re: [PATCH 05/11] selftests: net/fcnal: kill_procs via spin instead of sleep

[Leonard Crestez]

On 06.10.2021 17:45, David Ahern wrote:
> On 10/6/21 5:47 AM, Leonard Crestez wrote:
> > Sleeping for one second after a kill is not necessary and adds up quite
> > quickly. Replace with a fast loop spinning until pidof returns nothing.
> >
> > Signed-off-by: Leonard Crestez <cdleonard@xxxxxxxxx>
> > ---
> >   tools/testing/selftests/net/fcnal-test.sh | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
> > index 0bd60cd3bc06..b7fda51deb3f 100755
> > --- a/tools/testing/selftests/net/fcnal-test.sh
> > +++ b/tools/testing/selftests/net/fcnal-test.sh
> > @@ -176,12 +176,19 @@ show_hint()
> >   	fi
> >   }
> >   
> >   kill_procs()
> >   {
> > -	killall nettest ping ping6 >/dev/null 2>&1
> > -	sleep 1
> > +	local pids
> > +	while true; do
> > +		pids=$(pidof nettest ping ping6)
> > +		if [[ -z $pids ]]; then
> > +			break
> > +		fi
> > +		kill $pids
> > +		sleep 0.01
> > +	done
> >   }
> >   
> >   do_run_cmd()
> >   {
> >   	local cmd="$*"
>
> ideally the script keeps track of processes it launches and only kills
> those. The original killall was just a stop gap until the process
> tracking was added.

That's harder to do. This is much faster and not in any way worse than 
killall + sleep.

Some sort of a wrapper would have to added for each process running the 
background, for each run_ping_bg.

If nettest forks by itself then $! won't work, maybe some sort of 
--pid-file switch would be required?

--
Regards,
Leonard