Re: [LTP] ftrace: Add common library for C implementation

[linuxtestproject.agent@gmail.com]

Hi Praveen,

On Tue, Jun 9, 2026, Praveen K Pandey wrote:
> ftrace: Add common library for C implementation

--- [PATCH 1/5] ---

> -INSTALL_TARGETS         := *.sh ftrace_stress/*
> +# Filter out ftrace_lib from standalone build targets
> +FILTER_OUT_MAKE_TARGETS := ftrace_lib

Removing INSTALL_TARGETS here means the ftrace_stress/* helper
scripts are no longer installed, but ftrace_stress_test.sh still
exists and depends on them (it is not converted until patch 4/5).
This breaks the shell-based stress test at this intermediate
commit.

Moving this removal to patch 5/5 (where the helper scripts are
actually deleted) would preserve bisectability.

--- [PATCH 2/5] ---

> -ftrace_regression01	ftrace_regression01.sh
> -ftrace_regression02	ftrace_regression02.sh
> -ftrace-stress-test	ftrace_stress_test.sh 90
> +ftrace_regression01	ftrace_regression01
> +ftrace_regression02	ftrace_regression02
> +ftrace-stress-test	ftrace_stress_test

This patch converts only ftrace_regression01, but all three
runtest/tracing entries are updated to reference C binaries.
At this point ftrace_regression02 and ftrace_stress_test binaries
do not exist -- their .c files are added in patches 3/5 and 4/5
respectively.

After this commit, running ftrace_regression02 or ftrace-stress-test
from the runtest file will look for binaries that cannot be built.
Each conversion patch should only update its own runtest entry.

--- [PATCH 4/5] ---

> +static struct stress_test stress_tests[] = {
> +	{"current_tracer", "current_tracer", stress_current_tracer, 0},
> +	{"trace_pipe", "trace_pipe", stress_trace_pipe, 0},
> +	{"set_event", "events/enable", stress_set_event, 0},
> +	{"buffer_size_kb", "buffer_size_kb", stress_buffer_size, 0},
> +	{"tracing_on", "tracing_on", stress_tracing_on, 0},
> +	{"trace_options", "trace_options", stress_trace_options, 0},
> +	{"set_ftrace_filter", "set_ftrace_filter", stress_set_ftrace_filter, 0},

The original shell test exercised 17 ftrace interfaces concurrently:
trace_pipe, current_tracer, ftrace_enabled,
function_profile_enabled, set_event, set_ftrace_pid,
stack_max_size, stack_trace, trace, trace_clock, trace_options,
trace_stat, tracing_enabled, tracing_max_latency, tracing_on,
buffer_size_kb, tracing_cpumask, and set_ftrace_filter.

The C version implements 7 of these, dropping 10 stress targets.
Is the reduced coverage intentional? If so, it would be good to
note it in the commit message.

>  * Converted to C by: Praveen K Pandey <praveen@linux.ibm.com>
>  */
>
> +/*\
>  * ...
>  *    The test creates multiple threads that stress test different
>  *    ftrace interfaces concurrently for a specified duration.
>  *
>  *    Signed-off-by: Praveen K Pandey <praveen@linux.ibm.com>

The commit message states "Test timeout set to 300 seconds", but
the code has `.timeout = DEFAULT_TEST_DURATION + 30` which
evaluates to 90, not 300.

Verdict: Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp