diff mbox series

[1/2] ftrace/selftests: workaround cgroup RT scheduling issues

Message ID 1581001760-29831-2-git-send-email-alan.maguire@oracle.com (mailing list archive)
State New
Headers show
Series ftrace/selftests: clean up failure cases | expand

Commit Message

Alan Maguire Feb. 6, 2020, 3:09 p.m. UTC
wakeup_rt.tc and wakeup.tc tests in tracers/ subdirectory
fail due to the chrt command returning:

 chrt: failed to set pid 0's policy: Operation not permitted.

To work around this, temporarily disable grout RT scheduling
during ftracetest execution.  Restore original value on
test run completion.  With these changes in place, both
tests consistently pass.

Fixes: c575dea2c1a5 ("selftests/ftrace: Add wakeup_rt tracer testcase")
Fixes: c1edd060b413 ("selftests/ftrace: Add wakeup tracer testcase")
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/ftrace/ftracetest | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Masami Hiramatsu (Google) Feb. 7, 2020, 6:14 a.m. UTC | #1
On Thu,  6 Feb 2020 15:09:19 +0000
Alan Maguire <alan.maguire@oracle.com> wrote:

> wakeup_rt.tc and wakeup.tc tests in tracers/ subdirectory
> fail due to the chrt command returning:
> 
>  chrt: failed to set pid 0's policy: Operation not permitted.
> 
> To work around this, temporarily disable grout RT scheduling
> during ftracetest execution.  Restore original value on
> test run completion.  With these changes in place, both
> tests consistently pass.

OK, this looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> 
> Fixes: c575dea2c1a5 ("selftests/ftrace: Add wakeup_rt tracer testcase")
> Fixes: c1edd060b413 ("selftests/ftrace: Add wakeup tracer testcase")
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/testing/selftests/ftrace/ftracetest | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 063ecb2..3207bbf 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -29,8 +29,26 @@ err_ret=1
>  # kselftest skip code is 4
>  err_skip=4
>  
> +# cgroup RT scheduling prevents chrt commands from succeeding, which
> +# induces failures in test wakeup tests.  Disable for the duration of
> +# the tests.
> +sched_rt_runtime=$(sysctl -n kernel.sched_rt_runtime_us)

OK, but can you 

> +
> +set_sysctl() {
> +  sysctl -qw ${1}=${2} >/dev/null 2>&1
> +}
> +
> +setup() {
> +  set_sysctl kernel.sched_rt_runtime_us -1
> +}
> +
> +cleanup() {
> +  set_sysctl kernel.sched_rt_runtime_us $sched_rt_runtime
> +}
> +
>  errexit() { # message
>    echo "Error: $1" 1>&2
> +  cleanup
>    exit $err_ret
>  }
>  
> @@ -39,6 +57,8 @@ if [ `id -u` -ne 0 ]; then
>    errexit "this must be run by root user"
>  fi
>  
> +setup
> +
>  # Utilities
>  absdir() { # file_path
>    (cd `dirname $1`; pwd)
> @@ -235,6 +255,7 @@ TOTAL_RESULT=0
>  
>  INSTANCE=
>  CASENO=0
> +
>  testcase() { # testfile
>    CASENO=$((CASENO+1))
>    desc=`grep "^#[ \t]*description:" $1 | cut -f2 -d:`
> @@ -406,5 +427,7 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w`
>  prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w`
>  prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w`
>  
> +cleanup
> +
>  # if no error, return 0
>  exit $TOTAL_RESULT
> -- 
> 1.8.3.1
>
Steven Rostedt Feb. 10, 2020, 10:18 p.m. UTC | #2
On Fri, 7 Feb 2020 15:14:56 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Thu,  6 Feb 2020 15:09:19 +0000
> Alan Maguire <alan.maguire@oracle.com> wrote:
> 
> > wakeup_rt.tc and wakeup.tc tests in tracers/ subdirectory
> > fail due to the chrt command returning:
> > 
> >  chrt: failed to set pid 0's policy: Operation not permitted.
> > 
> > To work around this, temporarily disable grout RT scheduling
> > during ftracetest execution.  Restore original value on
> > test run completion.  With these changes in place, both
> > tests consistently pass.  
> 
> OK, this looks good to me.
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Thanks!
> 
> > 
> > Fixes: c575dea2c1a5 ("selftests/ftrace: Add wakeup_rt tracer testcase")
> > Fixes: c1edd060b413 ("selftests/ftrace: Add wakeup tracer testcase")
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> >  tools/testing/selftests/ftrace/ftracetest | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 063ecb2..3207bbf 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -29,8 +29,26 @@ err_ret=1
> >  # kselftest skip code is 4
> >  err_skip=4
> >  
> > +# cgroup RT scheduling prevents chrt commands from succeeding, which
> > +# induces failures in test wakeup tests.  Disable for the duration of
> > +# the tests.
> > +sched_rt_runtime=$(sysctl -n kernel.sched_rt_runtime_us)  
> 
> OK, but can you 

??

Masami?

-- Steve

> 
> > +
> > +set_sysctl() {
> > +  sysctl -qw ${1}=${2} >/dev/null 2>&1
> > +}
> > +
Masami Hiramatsu (Google) Feb. 10, 2020, 10:53 p.m. UTC | #3
On Mon, 10 Feb 2020 17:18:01 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Fri, 7 Feb 2020 15:14:56 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Thu,  6 Feb 2020 15:09:19 +0000
> > Alan Maguire <alan.maguire@oracle.com> wrote:
> > 
> > > wakeup_rt.tc and wakeup.tc tests in tracers/ subdirectory
> > > fail due to the chrt command returning:
> > > 
> > >  chrt: failed to set pid 0's policy: Operation not permitted.
> > > 
> > > To work around this, temporarily disable grout RT scheduling
> > > during ftracetest execution.  Restore original value on
> > > test run completion.  With these changes in place, both
> > > tests consistently pass.  
> > 
> > OK, this looks good to me.
> > 
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > 
> > Thanks!
> > 
> > > 
> > > Fixes: c575dea2c1a5 ("selftests/ftrace: Add wakeup_rt tracer testcase")
> > > Fixes: c1edd060b413 ("selftests/ftrace: Add wakeup tracer testcase")
> > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > > ---
> > >  tools/testing/selftests/ftrace/ftracetest | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > > index 063ecb2..3207bbf 100755
> > > --- a/tools/testing/selftests/ftrace/ftracetest
> > > +++ b/tools/testing/selftests/ftrace/ftracetest
> > > @@ -29,8 +29,26 @@ err_ret=1
> > >  # kselftest skip code is 4
> > >  err_skip=4
> > >  
> > > +# cgroup RT scheduling prevents chrt commands from succeeding, which
> > > +# induces failures in test wakeup tests.  Disable for the duration of
> > > +# the tests.
> > > +sched_rt_runtime=$(sysctl -n kernel.sched_rt_runtime_us)  
> > 
> > OK, but can you 
> 
> ??
> 
> Masami?

Oops, I missed to fill the comment. I meant

"but can you consider to use /proc/sys directly instead of sysctl command,
because other test cases uses /proc/sys (ftrace/fgraph-filter-stack.tc and
ftrace/func_stack_tracer.tc)?"

Thank you,

> 
> -- Steve
> 
> > 
> > > +
> > > +set_sysctl() {
> > > +  sysctl -qw ${1}=${2} >/dev/null 2>&1
> > > +}
> > > +
diff mbox series

Patch

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 063ecb2..3207bbf 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -29,8 +29,26 @@  err_ret=1
 # kselftest skip code is 4
 err_skip=4
 
+# cgroup RT scheduling prevents chrt commands from succeeding, which
+# induces failures in test wakeup tests.  Disable for the duration of
+# the tests.
+sched_rt_runtime=$(sysctl -n kernel.sched_rt_runtime_us)
+
+set_sysctl() {
+  sysctl -qw ${1}=${2} >/dev/null 2>&1
+}
+
+setup() {
+  set_sysctl kernel.sched_rt_runtime_us -1
+}
+
+cleanup() {
+  set_sysctl kernel.sched_rt_runtime_us $sched_rt_runtime
+}
+
 errexit() { # message
   echo "Error: $1" 1>&2
+  cleanup
   exit $err_ret
 }
 
@@ -39,6 +57,8 @@  if [ `id -u` -ne 0 ]; then
   errexit "this must be run by root user"
 fi
 
+setup
+
 # Utilities
 absdir() { # file_path
   (cd `dirname $1`; pwd)
@@ -235,6 +255,7 @@  TOTAL_RESULT=0
 
 INSTANCE=
 CASENO=0
+
 testcase() { # testfile
   CASENO=$((CASENO+1))
   desc=`grep "^#[ \t]*description:" $1 | cut -f2 -d:`
@@ -406,5 +427,7 @@  prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w`
 prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w`
 prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w`
 
+cleanup
+
 # if no error, return 0
 exit $TOTAL_RESULT