linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Introduce perf build subcommand
@ 2023-08-25  6:11 Aditya Gupta
  2023-08-25  6:11 ` [PATCH 1/4] perf build: introduce " Aditya Gupta
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-25  6:11 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
	linux-kernel

The Problem
===========

Currently the presence of a feature is checked with a combination of
perf version --build-options and greps, such as:

    perf version --build-options | grep " on .* HAVE_FEATURE"

Proposed solution
=================

As suggested by contributors in:
https://lore.kernel.org/linux-perf-users/ZMPWk5K63tadmDlU@kernel.org/

Introduce a subcommand "perf build --has", with which
scripts can test for presence of a feature, such as:

    perf build --has HAVE_FEATURE

The usage of "perf version --build-options | grep" has been replaced in two
tests, with "perf build --has" command

Also, to not duplicate the same feature list at multiple places, a new global
'supported_features' array has been introduced in builtin.h, so both commands
'perf build --has' and 'perf version --build-options' use the same array

'supported_features' feature is an array of 'struct feature_support', which
also has the name of the feature, macro used to test it's presence, and a
is_builtin member, which will be 0 if feature not built-in, and 1 if built-in

Architectures Tested
====================
* x86_64
* ppc64le

Git tree
========

Git tree with this patch series applied for testing:
https://github.com/adi-g15-ibm/linux/tree/perf-build-has

Aditya Gupta (3):
  perf build: introduce build subcommand
  perf version: update --build-options to use 'supported_features' array
  perf tests task_analyzer: check perf build for libtraceevent support

Athira Rajeev (1):
  tools/perf/tests: Update probe_vfs_getname.sh script to use perf build
    --has

 tools/perf/Build                              |  1 +
 tools/perf/builtin-build.c                    | 94 +++++++++++++++++++
 tools/perf/builtin-version.c                  | 39 ++------
 tools/perf/builtin.h                          | 47 ++++++++++
 tools/perf/perf.c                             |  1 +
 .../perf/tests/shell/lib/probe_vfs_getname.sh |  4 +-
 .../shell/record+probe_libc_inet_pton.sh      |  5 +-
 .../shell/record+script_probe_vfs_getname.sh  |  5 +-
 tools/perf/tests/shell/test_task_analyzer.sh  |  4 +-
 9 files changed, 163 insertions(+), 37 deletions(-)
 create mode 100644 tools/perf/builtin-build.c

-- 
2.41.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] perf build: introduce build subcommand
  2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
@ 2023-08-25  6:11 ` Aditya Gupta
  2023-08-25  6:11 ` [PATCH 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-25  6:11 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
	linux-kernel

Currently the presence of a feature is checked with a combination of
perf version --build-options and greps, such as:

    perf version --build-options | grep " on .* HAVE_FEATURE"

Instead of this, introduce a subcommand "perf build --has", with which
scripts can test for presence of a feature, such as:

    perf build --has HAVE_FEATURE

'perf build --has' command is expected to have exit status of 1 if feature is
built-in, and 0 if not, -2 if feature is not known.

A global array 'supported_features' has also been introduced that can be
used by other commands like 'perf version --build-options', so that
new features can be added in one place, with the array

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/Build           |  1 +
 tools/perf/builtin-build.c | 94 ++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h       | 47 +++++++++++++++++++
 tools/perf/perf.c          |  1 +
 4 files changed, 143 insertions(+)
 create mode 100644 tools/perf/builtin-build.c

diff --git a/tools/perf/Build b/tools/perf/Build
index aa7623622834..b15294919e75 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -5,6 +5,7 @@ perf-y += builtin-diff.o
 perf-y += builtin-evlist.o
 perf-y += builtin-ftrace.o
 perf-y += builtin-help.o
+perf-y += builtin-build.o
 perf-y += builtin-buildid-list.o
 perf-y += builtin-buildid-cache.o
 perf-y += builtin-kallsyms.o
diff --git a/tools/perf/builtin-build.c b/tools/perf/builtin-build.c
new file mode 100644
index 000000000000..60af38c3bc64
--- /dev/null
+++ b/tools/perf/builtin-build.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "builtin.h"
+#include "color.h"
+#include "util/debug.h"
+#include "util/header.h"
+#include <tools/config.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <subcmd/parse-options.h>
+
+struct build {
+	const char *has;
+};
+
+static struct build build;
+
+static struct option build_options[] = {
+	OPT_STRING(0, "has", &build.has, NULL, "check if a feature is built in"),
+	OPT_END(),
+};
+
+static const char * const build_usage[] = {
+	"perf build [<options>]",
+	NULL
+};
+
+static void on_off_print(const char *status)
+{
+	printf("[ ");
+
+	if (!strcmp(status, "OFF"))
+		color_fprintf(stdout, PERF_COLOR_RED, "%-3s", status);
+	else
+		color_fprintf(stdout, PERF_COLOR_GREEN, "%-3s", status);
+
+	printf(" ]");
+}
+
+static void status_print(const char *name, const char *macro,
+			 const char *status)
+{
+	printf("%22s: ", name);
+	on_off_print(status);
+	printf("  # %s\n", macro);
+}
+
+#define STATUS(feature)                                   \
+do {                                                      \
+	if (feature.is_builtin)                               \
+		status_print(feature.name, feature.macro, "on");  \
+	else                                                  \
+		status_print(feature.name, feature.macro, "OFF"); \
+} while (0)
+
+/**
+ * check whether "feature" is built-in with perf
+ * returns:
+ *   -1: Feature not known
+ *    0: Built-in
+ *    1: NOT Built in
+ */
+static int has_support(const char *feature)
+{
+	int res = -1;
+
+	for (int i = 0; supported_features[i].name; ++i) {
+		if (strcmp(feature, supported_features[i].name) == 0) {
+			res = supported_features[i].is_builtin;
+			STATUS(supported_features[i]);
+			break;
+		}
+	}
+
+	if (res == -1) {
+		color_fprintf(stdout, PERF_COLOR_RED, "Feature not known: %s", feature);
+		return -2;
+	}
+
+	return !res;
+}
+
+int cmd_build(int argc, const char **argv)
+{
+	argc = parse_options(argc, argv, build_options, build_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	printf("perf build %s\n", perf_version_string);
+
+	if (build.has)
+		return has_support(build.has);
+
+	return 0;
+}
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index f2ab5bae2150..f5b2b5d809ce 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -2,11 +2,58 @@
 #ifndef BUILTIN_H
 #define BUILTIN_H
 
+#include <stddef.h>
+#include <linux/compiler.h>
+#include <tools/config.h>
+
+struct feature_support {
+	const char *name;
+	const char *macro;
+	int is_builtin;
+};
+
+#define FEATURE_SUPPORT(name_, macro_) { \
+	.name = name_,                       \
+	.macro = #macro_,                    \
+	.is_builtin = IS_BUILTIN(macro_) }
+
+static struct feature_support supported_features[] __maybe_unused = {
+	FEATURE_SUPPORT("dwarf", HAVE_DWARF_SUPPORT),
+	FEATURE_SUPPORT("dwarf_getlocations", HAVE_DWARF_GETLOCATIONS_SUPPORT),
+#ifndef HAVE_SYSCALL_TABLE_SUPPORT
+	FEATURE_SUPPORT("libaudit", HAVE_LIBAUDIT_SUPPORT),
+#endif
+	FEATURE_SUPPORT("syscall_table", HAVE_SYSCALL_TABLE_SUPPORT),
+	FEATURE_SUPPORT("libbfd", HAVE_LIBBFD_SUPPORT),
+	FEATURE_SUPPORT("debuginfod", HAVE_DEBUGINFOD_SUPPORT),
+	FEATURE_SUPPORT("libelf", HAVE_LIBELF_SUPPORT),
+	FEATURE_SUPPORT("libnuma", HAVE_LIBNUMA_SUPPORT),
+	FEATURE_SUPPORT("numa_num_possible_cpus", HAVE_LIBNUMA_SUPPORT),
+	FEATURE_SUPPORT("libperl", HAVE_LIBPERL_SUPPORT),
+	FEATURE_SUPPORT("libpython", HAVE_LIBPYTHON_SUPPORT),
+	FEATURE_SUPPORT("libslang", HAVE_SLANG_SUPPORT),
+	FEATURE_SUPPORT("libcrypto", HAVE_LIBCRYPTO_SUPPORT),
+	FEATURE_SUPPORT("libunwind", HAVE_LIBUNWIND_SUPPORT),
+	FEATURE_SUPPORT("libdw-dwarf-unwind", HAVE_DWARF_SUPPORT),
+	FEATURE_SUPPORT("zlib", HAVE_ZLIB_SUPPORT),
+	FEATURE_SUPPORT("lzma", HAVE_LZMA_SUPPORT),
+	FEATURE_SUPPORT("get_cpuid", HAVE_AUXTRACE_SUPPORT),
+	FEATURE_SUPPORT("bpf", HAVE_LIBBPF_SUPPORT),
+	FEATURE_SUPPORT("aio", HAVE_AIO_SUPPORT),
+	FEATURE_SUPPORT("zstd", HAVE_ZSTD_SUPPORT),
+	FEATURE_SUPPORT("libpfm4", HAVE_LIBPFM),
+	FEATURE_SUPPORT("libtraceevent", HAVE_LIBTRACEEVENT),
+
+	// this should remain at end, to know the array end
+	FEATURE_SUPPORT(NULL, _)
+};
+
 void list_common_cmds_help(void);
 const char *help_unknown_cmd(const char *cmd);
 
 int cmd_annotate(int argc, const char **argv);
 int cmd_bench(int argc, const char **argv);
+int cmd_build(int argc, const char **argv);
 int cmd_buildid_cache(int argc, const char **argv);
 int cmd_buildid_list(int argc, const char **argv);
 int cmd_config(int argc, const char **argv);
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index d3fc8090413c..95a4a91b1144 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -48,6 +48,7 @@ struct cmd_struct {
 
 static struct cmd_struct commands[] = {
 	{ "archive",	NULL,	0 },
+	{ "build", cmd_build, 0 },
 	{ "buildid-cache", cmd_buildid_cache, 0 },
 	{ "buildid-list", cmd_buildid_list, 0 },
 	{ "config",	cmd_config,	0 },
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] perf version: update --build-options to use 'supported_features' array
  2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
  2023-08-25  6:11 ` [PATCH 1/4] perf build: introduce " Aditya Gupta
@ 2023-08-25  6:11 ` Aditya Gupta
  2023-08-25  6:11 ` [PATCH 3/4] perf tests task_analyzer: check perf build for libtraceevent support Aditya Gupta
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-25  6:11 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
	linux-kernel

Now that the feature list has been duplicated in a global
'supported_features' array, use that array instead of manually checking
status of built-in features.

This helps in being consistent with commands such as 'perf build --has',
so commands can use the same array, and any new feature can be added at
one place, in the 'supported_features' array

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/builtin-version.c | 39 ++++++++----------------------------
 1 file changed, 8 insertions(+), 31 deletions(-)

diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index e5859c70e195..e149d96c6dc5 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -46,41 +46,18 @@ static void status_print(const char *name, const char *macro,
 	printf("  # %s\n", macro);
 }
 
-#define STATUS(__d, __m)				\
-do {							\
-	if (IS_BUILTIN(__d))				\
-		status_print(#__m, #__d, "on");		\
-	else						\
-		status_print(#__m, #__d, "OFF");	\
+#define STATUS(feature)                                   \
+do {                                                      \
+	if (feature.is_builtin)                               \
+		status_print(feature.name, feature.macro, "on");  \
+	else                                                  \
+		status_print(feature.name, feature.macro, "OFF"); \
 } while (0)
 
 static void library_status(void)
 {
-	STATUS(HAVE_DWARF_SUPPORT, dwarf);
-	STATUS(HAVE_DWARF_GETLOCATIONS_SUPPORT, dwarf_getlocations);
-#ifndef HAVE_SYSCALL_TABLE_SUPPORT
-	STATUS(HAVE_LIBAUDIT_SUPPORT, libaudit);
-#endif
-	STATUS(HAVE_SYSCALL_TABLE_SUPPORT, syscall_table);
-	STATUS(HAVE_LIBBFD_SUPPORT, libbfd);
-	STATUS(HAVE_DEBUGINFOD_SUPPORT, debuginfod);
-	STATUS(HAVE_LIBELF_SUPPORT, libelf);
-	STATUS(HAVE_LIBNUMA_SUPPORT, libnuma);
-	STATUS(HAVE_LIBNUMA_SUPPORT, numa_num_possible_cpus);
-	STATUS(HAVE_LIBPERL_SUPPORT, libperl);
-	STATUS(HAVE_LIBPYTHON_SUPPORT, libpython);
-	STATUS(HAVE_SLANG_SUPPORT, libslang);
-	STATUS(HAVE_LIBCRYPTO_SUPPORT, libcrypto);
-	STATUS(HAVE_LIBUNWIND_SUPPORT, libunwind);
-	STATUS(HAVE_DWARF_SUPPORT, libdw-dwarf-unwind);
-	STATUS(HAVE_ZLIB_SUPPORT, zlib);
-	STATUS(HAVE_LZMA_SUPPORT, lzma);
-	STATUS(HAVE_AUXTRACE_SUPPORT, get_cpuid);
-	STATUS(HAVE_LIBBPF_SUPPORT, bpf);
-	STATUS(HAVE_AIO_SUPPORT, aio);
-	STATUS(HAVE_ZSTD_SUPPORT, zstd);
-	STATUS(HAVE_LIBPFM, libpfm4);
-	STATUS(HAVE_LIBTRACEEVENT, libtraceevent);
+	for (int i = 0; supported_features[i].name; ++i)
+		STATUS(supported_features[i]);
 }
 
 int cmd_version(int argc, const char **argv)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] perf tests task_analyzer: check perf build for libtraceevent support
  2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
  2023-08-25  6:11 ` [PATCH 1/4] perf build: introduce " Aditya Gupta
  2023-08-25  6:11 ` [PATCH 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
@ 2023-08-25  6:11 ` Aditya Gupta
  2023-08-25  6:11 ` [PATCH 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf build --has Aditya Gupta
  2023-08-25 17:07 ` [PATCH 0/4] Introduce perf build subcommand Namhyung Kim
  4 siblings, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-25  6:11 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
	linux-kernel

Currently we depend on output of 'perf record -e "sched:sched_switch"', to
check whether perf was built with libtraceevent support.

Instead, a more straightforward approach can be to check the build options,
using 'perf build --has libtraceevent', to check for libtraceevent support.

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/tests/shell/test_task_analyzer.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
index 92d15154ba79..b4803e373fe5 100755
--- a/tools/perf/tests/shell/test_task_analyzer.sh
+++ b/tools/perf/tests/shell/test_task_analyzer.sh
@@ -52,8 +52,8 @@ find_str_or_fail() {
 
 # check if perf is compiled with libtraceevent support
 skip_no_probe_record_support() {
-	perf version --build-options | grep -q " OFF .* HAVE_LIBTRACEEVENT" && return 2
-	return 0
+	perf build --has libtraceevent >/dev/null && return 0
+	return 2
 }
 
 prepare_perf_data() {
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf build --has
  2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
                   ` (2 preceding siblings ...)
  2023-08-25  6:11 ` [PATCH 3/4] perf tests task_analyzer: check perf build for libtraceevent support Aditya Gupta
@ 2023-08-25  6:11 ` Aditya Gupta
  2023-08-25 17:07 ` [PATCH 0/4] Introduce perf build subcommand Namhyung Kim
  4 siblings, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-25  6:11 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, linuxppc-dev, maddy, atrajeev, kjain, disgoel,
	linux-kernel

From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

In probe_vfs_getname.sh, current we use "perf record --dry-run"
to check for libtraceevent and skip the test if perf is not
build with libtraceevent. Change the check to use "perf build --has"
option

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/tests/shell/lib/probe_vfs_getname.sh           | 4 ++--
 tools/perf/tests/shell/record+probe_libc_inet_pton.sh     | 5 ++++-
 tools/perf/tests/shell/record+script_probe_vfs_getname.sh | 5 ++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/shell/lib/probe_vfs_getname.sh b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
index bf4c1fb71c4b..a5da9f035afe 100644
--- a/tools/perf/tests/shell/lib/probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
@@ -27,7 +27,7 @@ skip_if_no_debuginfo() {
 # check if perf is compiled with libtraceevent support
 skip_no_probe_record_support() {
 	if [ $had_vfs_getname -eq 1 ] ; then
-		perf record --dry-run -e $1 2>&1 | grep "libtraceevent is necessary for tracepoint support" && return 2
-		return 1
+		perf build --has libtraceevent >/dev/null && return 1
+		return 2
 	fi
 }
diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
index 89214a6d9951..0c00e94b83ff 100755
--- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
@@ -58,7 +58,10 @@ trace_libc_inet_pton_backtrace() {
 
 	# Check presence of libtraceevent support to run perf record
 	skip_no_probe_record_support "$event_name/$eventattr/"
-	[ $? -eq 2 ] && return 2
+	if [ $? -eq 2 ]; then
+		echo "WARN: Skipping test trace_libc_inet_pton_backtrace. No libtraceevent support."
+		return 2
+	fi
 
 	perf record -e $event_name/$eventattr/ -o $perf_data ping -6 -c 1 ::1 > /dev/null 2>&1
 	# check if perf data file got created in above step.
diff --git a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
index 7f664f1889d9..c7416d21fc92 100755
--- a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
@@ -19,7 +19,10 @@ record_open_file() {
 	echo "Recording open file:"
 	# Check presence of libtraceevent support to run perf record
 	skip_no_probe_record_support "probe:vfs_getname*"
-	[ $? -eq 2 ] && return 2
+	if [ $? -eq 2 ]; then
+		echo "WARN: Skipping test record_open_file. No libtraceevent support"
+		return 2
+	fi
 	perf record -o ${perfdata} -e probe:vfs_getname\* touch $file
 }
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] Introduce perf build subcommand
  2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
                   ` (3 preceding siblings ...)
  2023-08-25  6:11 ` [PATCH 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf build --has Aditya Gupta
@ 2023-08-25 17:07 ` Namhyung Kim
  2023-08-27  7:17   ` Aditya Gupta
  2023-09-03 11:53   ` Aditya Gupta
  4 siblings, 2 replies; 9+ messages in thread
From: Namhyung Kim @ 2023-08-25 17:07 UTC (permalink / raw)
  To: Aditya Gupta
  Cc: acme, jolsa, irogers, linux-perf-users, linuxppc-dev, maddy,
	atrajeev, kjain, disgoel, linux-kernel

Hello,

On Thu, Aug 24, 2023 at 11:11 PM Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> The Problem
> ===========
>
> Currently the presence of a feature is checked with a combination of
> perf version --build-options and greps, such as:
>
>     perf version --build-options | grep " on .* HAVE_FEATURE"
>
> Proposed solution
> =================
>
> As suggested by contributors in:
> https://lore.kernel.org/linux-perf-users/ZMPWk5K63tadmDlU@kernel.org/
>
> Introduce a subcommand "perf build --has", with which
> scripts can test for presence of a feature, such as:
>
>     perf build --has HAVE_FEATURE
>
> The usage of "perf version --build-options | grep" has been replaced in two
> tests, with "perf build --has" command

I'm not sure 'perf build' is a good name, it sounds like it needs to build
something.  Maybe 'perf check --feature XXX' ?

Then we can extend the perf check command to *check* system
settings like perf_event_paranoid, kptr_restrict, nmi_watchdog
and so on, and possibly provides some advice or even change
the values easily.

What do you think?

Thanks,
Namhyung


>
> Also, to not duplicate the same feature list at multiple places, a new global
> 'supported_features' array has been introduced in builtin.h, so both commands
> 'perf build --has' and 'perf version --build-options' use the same array
>
> 'supported_features' feature is an array of 'struct feature_support', which
> also has the name of the feature, macro used to test it's presence, and a
> is_builtin member, which will be 0 if feature not built-in, and 1 if built-in
>
> Architectures Tested
> ====================
> * x86_64
> * ppc64le
>
> Git tree
> ========
>
> Git tree with this patch series applied for testing:
> https://github.com/adi-g15-ibm/linux/tree/perf-build-has
>
> Aditya Gupta (3):
>   perf build: introduce build subcommand
>   perf version: update --build-options to use 'supported_features' array
>   perf tests task_analyzer: check perf build for libtraceevent support
>
> Athira Rajeev (1):
>   tools/perf/tests: Update probe_vfs_getname.sh script to use perf build
>     --has
>
>  tools/perf/Build                              |  1 +
>  tools/perf/builtin-build.c                    | 94 +++++++++++++++++++
>  tools/perf/builtin-version.c                  | 39 ++------
>  tools/perf/builtin.h                          | 47 ++++++++++
>  tools/perf/perf.c                             |  1 +
>  .../perf/tests/shell/lib/probe_vfs_getname.sh |  4 +-
>  .../shell/record+probe_libc_inet_pton.sh      |  5 +-
>  .../shell/record+script_probe_vfs_getname.sh  |  5 +-
>  tools/perf/tests/shell/test_task_analyzer.sh  |  4 +-
>  9 files changed, 163 insertions(+), 37 deletions(-)
>  create mode 100644 tools/perf/builtin-build.c
>
> --
> 2.41.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] Introduce perf build subcommand
  2023-08-25 17:07 ` [PATCH 0/4] Introduce perf build subcommand Namhyung Kim
@ 2023-08-27  7:17   ` Aditya Gupta
  2023-09-03 11:53   ` Aditya Gupta
  1 sibling, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-08-27  7:17 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: acme, jolsa, irogers, linux-perf-users, linuxppc-dev, maddy,
	atrajeev, kjain, disgoel, linux-kernel

Hello,

On Fri, Aug 25, 2023 at 10:07:28AM -0700, Namhyung Kim wrote:
> Hello,
> 
> On Thu, Aug 24, 2023 at 11:11 PM Aditya Gupta <adityag@linux.ibm.com> wrote:
> >
> > <...>
> >
> > Proposed solution
> > =================
> >
> > As suggested by contributors in:
> > https://lore.kernel.org/linux-perf-users/ZMPWk5K63tadmDlU@kernel.org/
> >
> > Introduce a subcommand "perf build --has", with which
> > scripts can test for presence of a feature, such as:
> >
> >     perf build --has HAVE_FEATURE
> >
> > <...>
> 
> I'm not sure 'perf build' is a good name, it sounds like it needs to build
> something.  Maybe 'perf check --feature XXX' ?
> 
> Then we can extend the perf check command to *check* system
> settings like perf_event_paranoid, kptr_restrict, nmi_watchdog
> and so on, and possibly provides some advice or even change
> the values easily.
> 
> What do you think?
> 

Sure, the said confusion due to 'build' makes sense. I will discuss with athira
also and post a V2 with suggested change.

And, since a new subcommand is being introduced, where do I add a
documentation, such that it can be visible in a manpage etc, similar to
perf-version(1) ?

Thanks,
Aditya Gupta


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] perf version: update --build-options to use 'supported_features' array
  2023-09-03 11:47 [PATCH 0/4] Introduce perf check subcommand Aditya Gupta
@ 2023-09-03 11:47 ` Aditya Gupta
  0 siblings, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-09-03 11:47 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, maddy, atrajeev, kjain, disgoel, linux-kernel

Now that the feature list has been duplicated in a global
'supported_features' array, use that array instead of manually checking
status of built-in features.

This helps in being consistent with commands such as 'perf check --feature',
so commands can use the same array, and any new feature can be added at
one place, in the 'supported_features' array

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/builtin-version.c | 39 ++++++++----------------------------
 1 file changed, 8 insertions(+), 31 deletions(-)

diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index e5859c70e195..e149d96c6dc5 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -46,41 +46,18 @@ static void status_print(const char *name, const char *macro,
 	printf("  # %s\n", macro);
 }
 
-#define STATUS(__d, __m)				\
-do {							\
-	if (IS_BUILTIN(__d))				\
-		status_print(#__m, #__d, "on");		\
-	else						\
-		status_print(#__m, #__d, "OFF");	\
+#define STATUS(feature)                                   \
+do {                                                      \
+	if (feature.is_builtin)                               \
+		status_print(feature.name, feature.macro, "on");  \
+	else                                                  \
+		status_print(feature.name, feature.macro, "OFF"); \
 } while (0)
 
 static void library_status(void)
 {
-	STATUS(HAVE_DWARF_SUPPORT, dwarf);
-	STATUS(HAVE_DWARF_GETLOCATIONS_SUPPORT, dwarf_getlocations);
-#ifndef HAVE_SYSCALL_TABLE_SUPPORT
-	STATUS(HAVE_LIBAUDIT_SUPPORT, libaudit);
-#endif
-	STATUS(HAVE_SYSCALL_TABLE_SUPPORT, syscall_table);
-	STATUS(HAVE_LIBBFD_SUPPORT, libbfd);
-	STATUS(HAVE_DEBUGINFOD_SUPPORT, debuginfod);
-	STATUS(HAVE_LIBELF_SUPPORT, libelf);
-	STATUS(HAVE_LIBNUMA_SUPPORT, libnuma);
-	STATUS(HAVE_LIBNUMA_SUPPORT, numa_num_possible_cpus);
-	STATUS(HAVE_LIBPERL_SUPPORT, libperl);
-	STATUS(HAVE_LIBPYTHON_SUPPORT, libpython);
-	STATUS(HAVE_SLANG_SUPPORT, libslang);
-	STATUS(HAVE_LIBCRYPTO_SUPPORT, libcrypto);
-	STATUS(HAVE_LIBUNWIND_SUPPORT, libunwind);
-	STATUS(HAVE_DWARF_SUPPORT, libdw-dwarf-unwind);
-	STATUS(HAVE_ZLIB_SUPPORT, zlib);
-	STATUS(HAVE_LZMA_SUPPORT, lzma);
-	STATUS(HAVE_AUXTRACE_SUPPORT, get_cpuid);
-	STATUS(HAVE_LIBBPF_SUPPORT, bpf);
-	STATUS(HAVE_AIO_SUPPORT, aio);
-	STATUS(HAVE_ZSTD_SUPPORT, zstd);
-	STATUS(HAVE_LIBPFM, libpfm4);
-	STATUS(HAVE_LIBTRACEEVENT, libtraceevent);
+	for (int i = 0; supported_features[i].name; ++i)
+		STATUS(supported_features[i]);
 }
 
 int cmd_version(int argc, const char **argv)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] Introduce perf build subcommand
  2023-08-25 17:07 ` [PATCH 0/4] Introduce perf build subcommand Namhyung Kim
  2023-08-27  7:17   ` Aditya Gupta
@ 2023-09-03 11:53   ` Aditya Gupta
  1 sibling, 0 replies; 9+ messages in thread
From: Aditya Gupta @ 2023-09-03 11:53 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: acme, jolsa, irogers, linux-perf-users, linuxppc-dev, maddy,
	atrajeev, kjain, disgoel, linux-kernel

Hello Namhyung,

On Fri, Aug 25, 2023 at 10:07:28AM -0700, Namhyung Kim wrote:
> On Thu, Aug 24, 2023 at 11:11 PM Aditya Gupta <adityag@linux.ibm.com> wrote:
> >
> > The Problem
> > ===========
> >
> > Currently the presence of a feature is checked with a combination of
> > perf version --build-options and greps, such as:
> >
> >     perf version --build-options | grep " on .* HAVE_FEATURE"
> >
> > Proposed solution
> > =================
> >
> > As suggested by contributors in:
> > https://lore.kernel.org/linux-perf-users/ZMPWk5K63tadmDlU@kernel.org/
> >
> > Introduce a subcommand "perf build --has", with which
> > scripts can test for presence of a feature, such as:
> >
> >     perf build --has HAVE_FEATURE
> >
> > The usage of "perf version --build-options | grep" has been replaced in two
> > tests, with "perf build --has" command
> 
> I'm not sure 'perf build' is a good name, it sounds like it needs to build
> something.  Maybe 'perf check --feature XXX' ?
> 
> Then we can extend the perf check command to *check* system
> settings like perf_event_paranoid, kptr_restrict, nmi_watchdog
> and so on, and possibly provides some advice or even change
> the values easily.
> 
> What do you think?

Thanks for the suggestion, we agree with it.
I have sent the updated patch series, but it's again from V1 since the subcommand changed.

Patch series: https://lore.kernel.org/linux-perf-users/20230903114721.190733-1-adityag@linux.ibm.com

Thanks,
Aditya Gupta


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-09-03 11:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-25  6:11 [PATCH 0/4] Introduce perf build subcommand Aditya Gupta
2023-08-25  6:11 ` [PATCH 1/4] perf build: introduce " Aditya Gupta
2023-08-25  6:11 ` [PATCH 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
2023-08-25  6:11 ` [PATCH 3/4] perf tests task_analyzer: check perf build for libtraceevent support Aditya Gupta
2023-08-25  6:11 ` [PATCH 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf build --has Aditya Gupta
2023-08-25 17:07 ` [PATCH 0/4] Introduce perf build subcommand Namhyung Kim
2023-08-27  7:17   ` Aditya Gupta
2023-09-03 11:53   ` Aditya Gupta
  -- strict thread matches above, loose matches on Subject: below --
2023-09-03 11:47 [PATCH 0/4] Introduce perf check subcommand Aditya Gupta
2023-09-03 11:47 ` [PATCH 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).