Re: [PATCH v6 3/3] selftests/tracing: Add hist poll() support test

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

Hi,

On Thu, 19 Dec 2024 11:04:34 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 16 Oct 2024 19:49:41 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-poll.tc
> > @@ -0,0 +1,74 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +# description: event trigger - test poll wait on histogram
> > +# requires: set_event events/sched/sched_process_free/trigger events/sched/sched_process_free/hist
> > +# flags: instance
> > +
> > +POLL=${FTRACETEST_ROOT}/poll
> > +
> > +if [ ! -x ${POLL} ]; then
> > +  echo "poll program is not compiled!"
> > +  exit_unresolved
> > +fi
> > +
> > +EVENT=events/sched/sched_process_free/
> > +
> > +# Check poll ops is supported. Before implementing poll on hist file, it
> > +# returns soon with POLLIN | POLLOUT, but not POLLPRI.
> > +
> > +# This must wait >1 sec and return 1 (timeout).
> > +set +e
> > +${POLL} -I -t 1000 ${EVENT}/hist
> > +ret=$?
> > +set -e
> > +if [ ${ret} != 1 ]; then
> > +  echo "poll on hist file is not supported"
> > +  exit_unsupported
> > +fi
> > +
> > +# Test POLLIN
> > +echo > trace
> > +echo "hist:key=comm" > ${EVENT}/trigger
> > +echo 1 > ${EVENT}/enable
> > +
> > +# This sleep command will exit after 2 seconds.
> > +sleep 2 &
> > +BGPID=$!
> > +# if timeout happens, poll returns 1.
> > +${POLL} -I -t 4000 ${EVENT}/hist
> > +echo 0 > tracing_on
> > +
> > +if [ -d /proc/${BGPID} ]; then
> > +  echo "poll exits too soon"
> > +  kill -KILL ${BGPID} ||:
> > +  exit_fail
> > +fi
> > +
> > +if ! grep -qw "sleep" trace; then
> > +  echo "poll exits before event happens"
> 
> I ran this and it failed here. But it wasn't because the poll failed, it's
> because the test is wrong. If something else exits during the test, then
> the poll function will exit early.

Hmm, so another process exits before sleep exits, it fails, yes.

> 
> What the check should do is simply read the hist file, get the hist count,
> and make sure it's updated after the poll is run, or at least put a filter
> on it:
> 
>   echo 'hist:keys=comm if comm =="sleep"' > /sys/kernel/tracing/events/sched/sched_process_free/trigger
> 
> Which would work as long as no other "sleep" exits during the test.

OK, let me update the series.

Thanks!

> 
> -- Steve
> 
> > +  exit_fail
> > +fi
> > +
> > +# Test POLLPRI
> > +echo > trace
> > +echo 1 > tracing_on
> > +
> > +# This sleep command will exit after 2 seconds.
> > +sleep 2 &
> > +BGPID=$!
> > +# if timeout happens, poll returns 1.
> > +${POLL} -P -t 4000 ${EVENT}/hist
> > +echo 0 > tracing_on
> > +
> > +if [ -d /proc/${BGPID} ]; then
> > +  echo "poll exits too soon"
> > +  kill -KILL ${BGPID} ||:
> > +  exit_fail
> > +fi
> > +
> > +if ! grep -qw "sleep" trace; then
> > +  echo "poll exits before event happens"
> > +  exit_fail
> > +fi
> > +
> > +exit_pass
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>