Re: [LTP] [PATCH 1/2] Rewrite ftrace_regression01.sh with new C API

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi lufei,

A few issues need fixing before this can be merged.

[PATCH 1/2]

> +/*\
> + * [Description]
> + *
> + * Regression test for panic while using userstacktrace.

Drop the [Description] tag, it's deprecated.

> +#define FTRACE_FILE_CONTAINS_BUF_SIZE (1024 * 1024)
[...]
> +static inline int file_contains(const char *path, const char *str)
> +{
> +	char buf[FTRACE_FILE_CONTAINS_BUF_SIZE];

This puts 1 MiB on the stack. Use SAFE_MALLOC/free or a static buffer
instead.

> +		if (!file_contains(TRACE_OPTIONS, "userstacktrace"))
> +			tst_brk(TBROK, "Failed to set userstacktrace");

strstr("nouserstacktrace", "userstacktrace") is true, so this check
always passes even when the option is disabled. The original shell
version used grep -q "^userstacktrace" which anchored to line start.

[PATCH 2/2]

> --- /dev/null
> +++ b/testcases/kernel/tracing/ftrace_test/ftrace_regression02.c

The old ftrace_regression02.sh is not deleted by this patch. You need
to remove it, same as you did for ftrace_regression01.sh in patch 1/2.

> +	.save_restore = (const struct tst_path_val[]) {
> +		{"/proc/sys/kernel/stack_tracer_enabled", NULL, TST_SR_TCONF},
> +		{}
> +	},

This test doesn't use stack_tracer_enabled. Looks like copy-paste from
ftrace_regression01. Remove it.

The original shell test gated on kernel >= 3.2. Add .min_kver = "3.2"
to preserve that check.

[Both patches]

The runtest/tracing entries still reference the .sh files:

  ftrace_regression01	ftrace_regression01.sh
  ftrace_regression02	ftrace_regression02.sh

Update them to point to the C binaries.

The Makefile has INSTALL_TARGETS := *.sh ftrace_stress/* which won't
install the new C binaries. It needs updating as well.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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