* [PATCH 1/3] perf test: Remove unused argument
@ 2021-03-10 20:41 Ian Rogers
2021-03-10 20:41 ` [PATCH 2/3] perf test: Cleanup daemon if test is interrupted Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ian Rogers @ 2021-03-10 20:41 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel
Cc: Stephane Eranian, Ian Rogers
Remove unused argument from daemon_exit.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/daemon.sh | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 5ad3ca8d681b..66ad56b4e0a5 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -115,8 +115,7 @@ daemon_start()
daemon_exit()
{
- local base=$1
- local config=$2
+ local config=$1
local line=`perf daemon --config ${config} -x: | head -1`
local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
@@ -171,7 +170,7 @@ EOF
${base}/session-time/ack "0"
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
rm -rf ${base}
rm -f ${config}
@@ -288,7 +287,7 @@ EOF
done
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
rm -rf ${base}
rm -f ${config}
@@ -333,7 +332,7 @@ EOF
fi
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
# check that sessions are gone
if [ -d "/proc/${pid_size}" ]; then
@@ -374,7 +373,7 @@ EOF
perf daemon signal --config ${config}
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
# count is 2 perf.data for signals and 1 for perf record finished
count=`ls ${base}/session-test/ | grep perf.data | wc -l`
@@ -420,7 +419,7 @@ EOF
fi
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
rm -rf ${base}
rm -f ${config}
@@ -457,7 +456,7 @@ EOF
fi
# stop daemon
- daemon_exit ${base} ${config}
+ daemon_exit ${config}
rm -rf ${base}
rm -f ${config}
--
2.30.1.766.gb4fecdf3b7-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/3] perf test: Cleanup daemon if test is interrupted.
2021-03-10 20:41 [PATCH 1/3] perf test: Remove unused argument Ian Rogers
@ 2021-03-10 20:41 ` Ian Rogers
2021-03-11 10:37 ` Jiri Olsa
2021-03-10 20:41 ` [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start Ian Rogers
2021-03-11 10:38 ` [PATCH 1/3] perf test: Remove unused argument Jiri Olsa
2 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2021-03-10 20:41 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel
Cc: Stephane Eranian, Ian Rogers
Reorder daemon_start and daemon_exit as the trap handler is added in
daemon_start referencing daemon_exit.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/daemon.sh | 34 +++++++++++++++++++-------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 66ad56b4e0a5..a02cedc76de6 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -98,6 +98,23 @@ check_line_other()
fi
}
+daemon_exit()
+{
+ local config=$1
+
+ local line=`perf daemon --config ${config} -x: | head -1`
+ local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+
+ # Reset trap handler.
+ trap - SIGINT SIGTERM
+
+ # stop daemon
+ perf daemon stop --config ${config}
+
+ # ... and wait for the pid to go away
+ tail --pid=${pid} -f /dev/null
+}
+
daemon_start()
{
local config=$1
@@ -105,6 +122,9 @@ daemon_start()
perf daemon start --config ${config}
+ # Clean up daemon if interrupted.
+ trap "daemon_exit ${config}; exit 4" SIGINT SIGTERM
+
# wait for the session to ping
local state="FAIL"
while [ "${state}" != "OK" ]; do
@@ -113,20 +133,6 @@ daemon_start()
done
}
-daemon_exit()
-{
- local config=$1
-
- local line=`perf daemon --config ${config} -x: | head -1`
- local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
-
- # stop daemon
- perf daemon stop --config ${config}
-
- # ... and wait for the pid to go away
- tail --pid=${pid} -f /dev/null
-}
-
test_list()
{
echo "test daemon list"
--
2.30.1.766.gb4fecdf3b7-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] perf test: Cleanup daemon if test is interrupted.
2021-03-10 20:41 ` [PATCH 2/3] perf test: Cleanup daemon if test is interrupted Ian Rogers
@ 2021-03-11 10:37 ` Jiri Olsa
2021-03-11 16:18 ` Ian Rogers
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2021-03-11 10:37 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-kernel,
Stephane Eranian
On Wed, Mar 10, 2021 at 12:41:17PM -0800, Ian Rogers wrote:
> Reorder daemon_start and daemon_exit as the trap handler is added in
> daemon_start referencing daemon_exit.
makes sense, minor comments below
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/shell/daemon.sh | 34 +++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> index 66ad56b4e0a5..a02cedc76de6 100755
> --- a/tools/perf/tests/shell/daemon.sh
> +++ b/tools/perf/tests/shell/daemon.sh
> @@ -98,6 +98,23 @@ check_line_other()
> fi
> }
>
> +daemon_exit()
> +{
> + local config=$1
> +
> + local line=`perf daemon --config ${config} -x: | head -1`
> + local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> +
> + # Reset trap handler.
> + trap - SIGINT SIGTERM
please keep the 'tabs' in here
> +
> + # stop daemon
> + perf daemon stop --config ${config}
> +
> + # ... and wait for the pid to go away
> + tail --pid=${pid} -f /dev/null
> +}
> +
> daemon_start()
> {
> local config=$1
> @@ -105,6 +122,9 @@ daemon_start()
>
> perf daemon start --config ${config}
>
> + # Clean up daemon if interrupted.
> + trap "daemon_exit ${config}; exit 4" SIGINT SIGTERM
ditto, plus let's document exit values somewhere in comments,
I think the next patch is adding exit 62, so that one as well
thanks,
jirka
> +
> # wait for the session to ping
> local state="FAIL"
> while [ "${state}" != "OK" ]; do
> @@ -113,20 +133,6 @@ daemon_start()
> done
> }
>
> -daemon_exit()
> -{
> - local config=$1
> -
> - local line=`perf daemon --config ${config} -x: | head -1`
> - local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> -
> - # stop daemon
> - perf daemon stop --config ${config}
> -
> - # ... and wait for the pid to go away
> - tail --pid=${pid} -f /dev/null
> -}
> -
> test_list()
> {
> echo "test daemon list"
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] perf test: Cleanup daemon if test is interrupted.
2021-03-11 10:37 ` Jiri Olsa
@ 2021-03-11 16:18 ` Ian Rogers
2021-03-11 19:03 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2021-03-11 16:18 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, LKML,
Stephane Eranian
On Thu, Mar 11, 2021 at 2:38 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, Mar 10, 2021 at 12:41:17PM -0800, Ian Rogers wrote:
> > Reorder daemon_start and daemon_exit as the trap handler is added in
> > daemon_start referencing daemon_exit.
>
> makes sense, minor comments below
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/tests/shell/daemon.sh | 34 +++++++++++++++++++-------------
> > 1 file changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> > index 66ad56b4e0a5..a02cedc76de6 100755
> > --- a/tools/perf/tests/shell/daemon.sh
> > +++ b/tools/perf/tests/shell/daemon.sh
> > @@ -98,6 +98,23 @@ check_line_other()
> > fi
> > }
> >
> > +daemon_exit()
> > +{
> > + local config=$1
> > +
> > + local line=`perf daemon --config ${config} -x: | head -1`
> > + local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> > +
> > + # Reset trap handler.
> > + trap - SIGINT SIGTERM
>
> please keep the 'tabs' in here
>
> > +
> > + # stop daemon
> > + perf daemon stop --config ${config}
> > +
> > + # ... and wait for the pid to go away
> > + tail --pid=${pid} -f /dev/null
> > +}
> > +
> > daemon_start()
> > {
> > local config=$1
> > @@ -105,6 +122,9 @@ daemon_start()
> >
> > perf daemon start --config ${config}
> >
> > + # Clean up daemon if interrupted.
> > + trap "daemon_exit ${config}; exit 4" SIGINT SIGTERM
>
> ditto, plus let's document exit values somewhere in comments,
> I think the next patch is adding exit 62, so that one as well
Thanks, it might be easier to just have these as exit 1 - I pulled
values from the errno list. Do you have any preferences?
Ian
> thanks,
> jirka
>
> > +
> > # wait for the session to ping
> > local state="FAIL"
> > while [ "${state}" != "OK" ]; do
> > @@ -113,20 +133,6 @@ daemon_start()
> > done
> > }
> >
> > -daemon_exit()
> > -{
> > - local config=$1
> > -
> > - local line=`perf daemon --config ${config} -x: | head -1`
> > - local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> > -
> > - # stop daemon
> > - perf daemon stop --config ${config}
> > -
> > - # ... and wait for the pid to go away
> > - tail --pid=${pid} -f /dev/null
> > -}
> > -
> > test_list()
> > {
> > echo "test daemon list"
> > --
> > 2.30.1.766.gb4fecdf3b7-goog
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] perf test: Cleanup daemon if test is interrupted.
2021-03-11 16:18 ` Ian Rogers
@ 2021-03-11 19:03 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2021-03-11 19:03 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, LKML,
Stephane Eranian
On Thu, Mar 11, 2021 at 08:18:27AM -0800, Ian Rogers wrote:
> On Thu, Mar 11, 2021 at 2:38 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Wed, Mar 10, 2021 at 12:41:17PM -0800, Ian Rogers wrote:
> > > Reorder daemon_start and daemon_exit as the trap handler is added in
> > > daemon_start referencing daemon_exit.
> >
> > makes sense, minor comments below
> >
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > tools/perf/tests/shell/daemon.sh | 34 +++++++++++++++++++-------------
> > > 1 file changed, 20 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> > > index 66ad56b4e0a5..a02cedc76de6 100755
> > > --- a/tools/perf/tests/shell/daemon.sh
> > > +++ b/tools/perf/tests/shell/daemon.sh
> > > @@ -98,6 +98,23 @@ check_line_other()
> > > fi
> > > }
> > >
> > > +daemon_exit()
> > > +{
> > > + local config=$1
> > > +
> > > + local line=`perf daemon --config ${config} -x: | head -1`
> > > + local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> > > +
> > > + # Reset trap handler.
> > > + trap - SIGINT SIGTERM
> >
> > please keep the 'tabs' in here
> >
> > > +
> > > + # stop daemon
> > > + perf daemon stop --config ${config}
> > > +
> > > + # ... and wait for the pid to go away
> > > + tail --pid=${pid} -f /dev/null
> > > +}
> > > +
> > > daemon_start()
> > > {
> > > local config=$1
> > > @@ -105,6 +122,9 @@ daemon_start()
> > >
> > > perf daemon start --config ${config}
> > >
> > > + # Clean up daemon if interrupted.
> > > + trap "daemon_exit ${config}; exit 4" SIGINT SIGTERM
> >
> > ditto, plus let's document exit values somewhere in comments,
> > I think the next patch is adding exit 62, so that one as well
>
> Thanks, it might be easier to just have these as exit 1 - I pulled
> values from the errno list. Do you have any preferences?
yep, exit 1 seems enough
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start.
2021-03-10 20:41 [PATCH 1/3] perf test: Remove unused argument Ian Rogers
2021-03-10 20:41 ` [PATCH 2/3] perf test: Cleanup daemon if test is interrupted Ian Rogers
@ 2021-03-10 20:41 ` Ian Rogers
2021-03-11 10:37 ` Jiri Olsa
2021-03-11 10:38 ` [PATCH 1/3] perf test: Remove unused argument Jiri Olsa
2 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2021-03-10 20:41 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-kernel
Cc: Stephane Eranian, Ian Rogers
Retry the ping loop upto 600 times, or approximately 30 seconds, to make
sure the test does hang at start up.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/daemon.sh | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index a02cedc76de6..cb831ff2c185 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -127,9 +127,16 @@ daemon_start()
# wait for the session to ping
local state="FAIL"
+ local retries=0
while [ "${state}" != "OK" ]; do
state=`perf daemon ping --config ${config} --session ${session} | awk '{ print $1 }'`
sleep 0.05
+ retries=$((${retries} +1))
+ if [ ${retries} -ge 600 ]; then
+ echo "Timeout waiting for daemon to ping"
+ daemon_exit ${config}
+ exit 62
+ fi
done
}
--
2.30.1.766.gb4fecdf3b7-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start.
2021-03-10 20:41 ` [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start Ian Rogers
@ 2021-03-11 10:37 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2021-03-11 10:37 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-kernel,
Stephane Eranian
On Wed, Mar 10, 2021 at 12:41:18PM -0800, Ian Rogers wrote:
> Retry the ping loop upto 600 times, or approximately 30 seconds, to make
> sure the test does hang at start up.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/shell/daemon.sh | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> index a02cedc76de6..cb831ff2c185 100755
> --- a/tools/perf/tests/shell/daemon.sh
> +++ b/tools/perf/tests/shell/daemon.sh
> @@ -127,9 +127,16 @@ daemon_start()
>
> # wait for the session to ping
> local state="FAIL"
> + local retries=0
> while [ "${state}" != "OK" ]; do
> state=`perf daemon ping --config ${config} --session ${session} | awk '{ print $1 }'`
> sleep 0.05
> + retries=$((${retries} +1))
> + if [ ${retries} -ge 600 ]; then
> + echo "Timeout waiting for daemon to ping"
> + daemon_exit ${config}
> + exit 62
> + fi
so it broke in here for you? ;-) makes sense, please keep
the 'FAILED: ...' prefix so it's obvious it's an error
thanks,
jirka
> done
> }
>
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] perf test: Remove unused argument
2021-03-10 20:41 [PATCH 1/3] perf test: Remove unused argument Ian Rogers
2021-03-10 20:41 ` [PATCH 2/3] perf test: Cleanup daemon if test is interrupted Ian Rogers
2021-03-10 20:41 ` [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start Ian Rogers
@ 2021-03-11 10:38 ` Jiri Olsa
2 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2021-03-11 10:38 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-kernel,
Stephane Eranian
On Wed, Mar 10, 2021 at 12:41:16PM -0800, Ian Rogers wrote:
> Remove unused argument from daemon_exit.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> ---
> tools/perf/tests/shell/daemon.sh | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> index 5ad3ca8d681b..66ad56b4e0a5 100755
> --- a/tools/perf/tests/shell/daemon.sh
> +++ b/tools/perf/tests/shell/daemon.sh
> @@ -115,8 +115,7 @@ daemon_start()
>
> daemon_exit()
> {
> - local base=$1
> - local config=$2
> + local config=$1
>
> local line=`perf daemon --config ${config} -x: | head -1`
> local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
> @@ -171,7 +170,7 @@ EOF
> ${base}/session-time/ack "0"
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> rm -rf ${base}
> rm -f ${config}
> @@ -288,7 +287,7 @@ EOF
> done
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> rm -rf ${base}
> rm -f ${config}
> @@ -333,7 +332,7 @@ EOF
> fi
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> # check that sessions are gone
> if [ -d "/proc/${pid_size}" ]; then
> @@ -374,7 +373,7 @@ EOF
> perf daemon signal --config ${config}
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> # count is 2 perf.data for signals and 1 for perf record finished
> count=`ls ${base}/session-test/ | grep perf.data | wc -l`
> @@ -420,7 +419,7 @@ EOF
> fi
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> rm -rf ${base}
> rm -f ${config}
> @@ -457,7 +456,7 @@ EOF
> fi
>
> # stop daemon
> - daemon_exit ${base} ${config}
> + daemon_exit ${config}
>
> rm -rf ${base}
> rm -f ${config}
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-03-11 19:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 20:41 [PATCH 1/3] perf test: Remove unused argument Ian Rogers
2021-03-10 20:41 ` [PATCH 2/3] perf test: Cleanup daemon if test is interrupted Ian Rogers
2021-03-11 10:37 ` Jiri Olsa
2021-03-11 16:18 ` Ian Rogers
2021-03-11 19:03 ` Jiri Olsa
2021-03-10 20:41 ` [PATCH 3/3] perf test: Add 30s timeout for wait for daemon start Ian Rogers
2021-03-11 10:37 ` Jiri Olsa
2021-03-11 10:38 ` [PATCH 1/3] perf test: Remove unused argument Jiri Olsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.