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

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 check --feature", with which
scripts can test for presence of a feature or multiple features, such as:

    perf check --feature HAVE_LIBTRACEEVENT

    or

    perf check --feature libtraceevent

    or

    perf check --feature libtraceevent,bpf

The usage of "perf version --build-options | grep" has been replaced in two
tests, with "perf check --feature" 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 check --feature' 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-check-feature-v8

Changelog
=========
V8
+ handle return value of 'malloc' in patch #1
+ fix error due to strncpy depending on source string's length

V7
+ modified patch #1 to fix compile issue, and add feature to allow
  multiple comma-separated features

V6
+ rebased to perf-tools-next/perf-tools-next
+ modified patch #1 to include FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL)

V5
+ invert return value of 'has_support', but return value of perf check --feature
according to shell convention

V4
+ invert return value of perf check --feature

V3
+ simplified has_support code in builtin-check.c (patch #1)
+ modified patch #3 and patch #4 according to change in return value in patch #1

V2
+ improved the patch series with suggestions from Namhyung
+ fix incorrect return value, added -q option, and moved array definition to
  perf-check.c

V1
+ changed subcommand name to 'perf check --feature'
+ added documentation for perf check
+ support both macro (eg. HAVE_LIBTRACEEVENT), and name (eg. libtraceevent) as
  input to 'perf check --feature'
+ change subject and descriptions of all patch mentioning perf check instead of
  perf build

V0: Previous patch series: https://lore.kernel.org/linux-perf-users/20230825061125.24312-1-adityag@linux.ibm.com/

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

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

 tools/perf/Build                              |   1 +
 tools/perf/Documentation/perf-check.txt       |  59 +++++++++
 tools/perf/builtin-check.c                    | 122 ++++++++++++++++++
 tools/perf/builtin-version.c                  |  39 ++----
 tools/perf/builtin.h                          |  18 +++
 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 +-
 10 files changed, 221 insertions(+), 37 deletions(-)
 create mode 100644 tools/perf/Documentation/perf-check.txt
 create mode 100644 tools/perf/builtin-check.c

-- 
2.41.0


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

* [PATCH v8 1/4] perf check: introduce check subcommand
  2023-12-04  9:39 [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
@ 2023-12-04  9:39 ` Aditya Gupta
  2024-01-05 23:29   ` Namhyung Kim
  2023-12-04  9:39 ` [PATCH v8 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Aditya Gupta @ 2023-12-04  9:39 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, maddy, atrajeev, kjain, disgoel

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 check --feature", with which
scripts can test for presence of a feature, such as:

    perf check --feature HAVE_FEATURE

'perf check --feature' command is expected to have exit status of 0 if
feature is built-in, and 1 if it's not built-in or if feature is not known.

Multiple features can also be passed as a comma-separated list, in which
case the exit status will be 1 only if all of the passed features are
built-in. For example, with below command, it will have exit status of 1 only
if both libtraceevent and bpf are enabled, else 0 in all other cases

    perf check --feature libtraceevent,bpf

An 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

Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/Build                        |   1 +
 tools/perf/Documentation/perf-check.txt |  65 ++++++++++
 tools/perf/builtin-check.c              | 155 ++++++++++++++++++++++++
 tools/perf/builtin.h                    |  18 +++
 tools/perf/perf.c                       |   1 +
 5 files changed, 240 insertions(+)
 create mode 100644 tools/perf/Documentation/perf-check.txt
 create mode 100644 tools/perf/builtin-check.c

diff --git a/tools/perf/Build b/tools/perf/Build
index aa7623622834..a55a797c1b5f 100644
--- a/tools/perf/Build
+++ b/tools/perf/Build
@@ -1,5 +1,6 @@
 perf-y += builtin-bench.o
 perf-y += builtin-annotate.o
+perf-y += builtin-check.o
 perf-y += builtin-config.o
 perf-y += builtin-diff.o
 perf-y += builtin-evlist.o
diff --git a/tools/perf/Documentation/perf-check.txt b/tools/perf/Documentation/perf-check.txt
new file mode 100644
index 000000000000..dfb1dddd0ac3
--- /dev/null
+++ b/tools/perf/Documentation/perf-check.txt
@@ -0,0 +1,65 @@
+perf-check(1)
+===============
+
+NAME
+----
+perf-check - check features in perf
+
+SYNOPSIS
+--------
+'perf check' [<options>]
+
+DESCRIPTION
+-----------
+With no options given, the 'perf check' just prints the perf version
+on the standard output.
+
+If the option '--feature' is given, then status of feature is printed
+on the standard output (unless '-q' is also passed), ie. whether it is
+compiled-in/built-in or not.
+Also, 'perf check --feature' returns with exit status 0 if the feature
+is built-in, otherwise returns with exit status 1.
+
+OPTIONS
+-------
+-q::
+		Do not print any messages or warnings
+
+--feature::
+        Print whether feature(s) is compiled-in or not. A feature name/macro is
+        required to be passed after this flag
+
+        Multiple features can also be passed as a comma-separated list, in which
+        case the exit status will be 1 only if all of the passed features are
+        compiled-in.
+
+        Example Usage:
+                perf check --feature libtraceevent
+                perf check --feature HAVE_LIBTRACEEVENT
+                perf check --feature libtraceevent,bpf
+
+        Supported feature names/macro:
+                dwarf      /  HAVE_DWARF_SUPPORT
+                dwarf_getlocations  /  HAVE_DWARF_GETLOCATIONS_SUPPORT
+                libaudit   /  HAVE_LIBAUDIT_SUPPORT
+                syscall_table       /  HAVE_SYSCALL_TABLE_SUPPORT
+                libbfd     /  HAVE_LIBBFD_SUPPORT
+                debuginfod /  HAVE_DEBUGINFOD_SUPPORT
+                libelf     /  HAVE_LIBELF_SUPPORT
+                libnuma    /  HAVE_LIBNUMA_SUPPORT
+                numa_num_possible_cpus  /  HAVE_LIBNUMA_SUPPORT
+                libperl    /  HAVE_LIBPERL_SUPPORT
+                libpython  /  HAVE_LIBPYTHON_SUPPORT
+                libslang   /  HAVE_SLANG_SUPPORT
+                libcrypto  /  HAVE_LIBCRYPTO_SUPPORT
+                libunwind  /  HAVE_LIBUNWIND_SUPPORT
+                libdw-dwarf-unwind  /  HAVE_DWARF_SUPPORT
+                zlib       /  HAVE_ZLIB_SUPPORT
+                lzma       /  HAVE_LZMA_SUPPORT
+                get_cpuid  /  HAVE_AUXTRACE_SUPPORT
+                bpf        /  HAVE_LIBBPF_SUPPORT
+                aio        /  HAVE_AIO_SUPPORT
+                zstd       /  HAVE_ZSTD_SUPPORT
+                libpfm4    /  HAVE_LIBPFM
+                libtraceevent      /  HAVE_LIBTRACEEVENT
+                bpf_skeletons      /  HAVE_BPF_SKEL
diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
new file mode 100644
index 000000000000..f121d657a7bc
--- /dev/null
+++ b/tools/perf/builtin-check.c
@@ -0,0 +1,155 @@
+// 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 check {
+	const char *feature;
+};
+
+static struct check check;
+
+static struct option check_options[] = {
+	OPT_STRING(0, "feature", &check.feature, NULL, "check if feature(s) is built in"),
+	OPT_BOOLEAN('q', "quiet", &quiet, "do not show any warnings or messages"),
+	OPT_END(),
+};
+
+static const char * const check_usage[] = {
+	"perf check [<options>]",
+	NULL
+};
+
+struct feature_support supported_features[] = {
+	FEATURE_SUPPORT("dwarf", HAVE_DWARF_SUPPORT),
+	FEATURE_SUPPORT("dwarf_getlocations", HAVE_DWARF_GETLOCATIONS_SUPPORT),
+	FEATURE_SUPPORT("libaudit", HAVE_LIBAUDIT_SUPPORT),
+	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),
+	FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL),
+
+	/* this should remain at end, to know the array end */
+	FEATURE_SUPPORT(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:
+ *    0: NOT built-in or Feature not known
+ *    1: Built-in
+ */
+static int has_support(const char *feature)
+{
+	for (int i = 0; supported_features[i].name; ++i) {
+		if ((strcmp(feature, supported_features[i].name) == 0) ||
+		    (strcmp(feature, supported_features[i].macro) == 0)) {
+			if (!quiet)
+				STATUS(supported_features[i]);
+			return supported_features[i].is_builtin;
+		}
+	}
+
+	if (!quiet)
+		color_fprintf(stdout, PERF_COLOR_RED, "Feature not known: '%s'\n", feature);
+
+	return 0;
+}
+
+int cmd_check(int argc, const char **argv)
+{
+	char *feature_list;
+	char *feature_name;
+	int feature_enabled;
+	int buf_len;
+
+	argc = parse_options(argc, argv, check_options, check_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	if (check.feature) {
+		/* check.feature can be a single feature name, or a comma-separated list
+		 * of feature names.
+		 * eg. check.feature can be "libtraceevent" or
+		 * "libtraceevent,bpf" etc
+		 *
+		 * In case of a comma-separated list, feature_enabled will be 1, only if
+		 * all features passed in the string are supported
+		 * Note that check.feature will get modified due to strtok and str_trim
+		 */
+		feature_enabled = 1;
+		buf_len = strlen(check.feature) + 1;
+		/* feature_list is a non-const copy of 'check.feature' */
+		feature_list = malloc(buf_len * sizeof(char));
+		if (!feature_list) {
+			color_fprintf(stdout, PERF_COLOR_RED,
+					"ERROR: failed to allocate memory for feature list");
+			return -1;
+		}
+
+		memcpy(feature_list, check.feature, buf_len);
+		feature_name = strtok(feature_list, ",");
+
+		while (feature_name) {
+			feature_enabled &= has_support(feature_name);
+			feature_name = strtok(NULL, ",");
+		}
+
+		free(feature_list);
+
+		return !feature_enabled;
+	}
+
+	return 0;
+}
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index f2ab5bae2150..9f895981bc9e 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -2,6 +2,23 @@
 #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_) }
+
+extern struct feature_support supported_features[];
+
 void list_common_cmds_help(void);
 const char *help_unknown_cmd(const char *cmd);
 
@@ -9,6 +26,7 @@ int cmd_annotate(int argc, const char **argv);
 int cmd_bench(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_check(int argc, const char **argv);
 int cmd_config(int argc, const char **argv);
 int cmd_c2c(int argc, const char **argv);
 int cmd_diff(int argc, const char **argv);
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index d3fc8090413c..6514f4121c49 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -50,6 +50,7 @@ static struct cmd_struct commands[] = {
 	{ "archive",	NULL,	0 },
 	{ "buildid-cache", cmd_buildid_cache, 0 },
 	{ "buildid-list", cmd_buildid_list, 0 },
+	{ "check",	cmd_check,	0 },
 	{ "config",	cmd_config,	0 },
 	{ "c2c",	cmd_c2c,	0 },
 	{ "diff",	cmd_diff,	0 },
-- 
2.42.0


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

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

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

Acked-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/builtin-version.c | 40 ++++++++----------------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index ac20c2b9bbc2..e149d96c6dc5 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -46,42 +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);
-	STATUS(HAVE_BPF_SKEL, bpf_skeletons);
+	for (int i = 0; supported_features[i].name; ++i)
+		STATUS(supported_features[i]);
 }
 
 int cmd_version(int argc, const char **argv)
-- 
2.42.0


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

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

Currently we use output of 'perf version --build-options', to check
whether perf was built with libtraceevent support.

Instead, use 'perf check --feature libtraceevent' to check for
libtraceevent support.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
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..f4db68edb2e3 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 check -q --feature libtraceevent && return 0
+	return 2
 }
 
 prepare_perf_data() {
-- 
2.42.0


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

* [PATCH v8 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf check --feature
  2023-12-04  9:39 [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
                   ` (2 preceding siblings ...)
  2023-12-04  9:39 ` [PATCH v8 3/4] perf tests task_analyzer: use perf check for libtraceevent support Aditya Gupta
@ 2023-12-04  9:39 ` Aditya Gupta
  2023-12-06  6:12 ` [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
  4 siblings, 0 replies; 11+ messages in thread
From: Aditya Gupta @ 2023-12-04  9:39 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, maddy, atrajeev, kjain, disgoel

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 check --feature"
option

Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.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..368a55c02134 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 check -q --feature libtraceevent && 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 eebeea6bdc76..758da2a23e26 100755
--- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
@@ -60,7 +60,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 5eedbe29bba1..9a61928e3c9a 100755
--- a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
@@ -21,7 +21,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.42.0


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

* Re: [PATCH v8 0/4] Introduce perf check subcommand
  2023-12-04  9:39 [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
                   ` (3 preceding siblings ...)
  2023-12-04  9:39 ` [PATCH v8 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf check --feature Aditya Gupta
@ 2023-12-06  6:12 ` Aditya Gupta
  2024-01-05  8:00   ` Aditya Gupta
  4 siblings, 1 reply; 11+ messages in thread
From: Aditya Gupta @ 2023-12-06  6:12 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, maddy, atrajeev, kjain, disgoel

Hi Arnaldo, just a ping for this series, this should also fix the error in v7.

Thanks,
Aditya

On Mon, Dec 04, 2023 at 03:09:50PM +0530, Aditya Gupta 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 check --feature", with which
> scripts can test for presence of a feature or multiple features, such as:
> 
>     perf check --feature HAVE_LIBTRACEEVENT
> 
>     or
> 
>     perf check --feature libtraceevent
> 
>     or
> 
>     perf check --feature libtraceevent,bpf
> 
> The usage of "perf version --build-options | grep" has been replaced in two
> tests, with "perf check --feature" 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 check --feature' 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-check-feature-v8
> 
> Changelog
> =========
> V8
> + handle return value of 'malloc' in patch #1
> + fix error due to strncpy depending on source string's length
> 
> V7
> + modified patch #1 to fix compile issue, and add feature to allow
>   multiple comma-separated features
> 
> V6
> + rebased to perf-tools-next/perf-tools-next
> + modified patch #1 to include FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL)
> 
> V5
> + invert return value of 'has_support', but return value of perf check --feature
> according to shell convention
> 
> V4
> + invert return value of perf check --feature
> 
> V3
> + simplified has_support code in builtin-check.c (patch #1)
> + modified patch #3 and patch #4 according to change in return value in patch #1
> 
> V2
> + improved the patch series with suggestions from Namhyung
> + fix incorrect return value, added -q option, and moved array definition to
>   perf-check.c
> 
> V1
> + changed subcommand name to 'perf check --feature'
> + added documentation for perf check
> + support both macro (eg. HAVE_LIBTRACEEVENT), and name (eg. libtraceevent) as
>   input to 'perf check --feature'
> + change subject and descriptions of all patch mentioning perf check instead of
>   perf build
> 
> V0: Previous patch series: https://lore.kernel.org/linux-perf-users/20230825061125.24312-1-adityag@linux.ibm.com/
> 
> Aditya Gupta (3):
>   perf check: introduce check subcommand
>   perf version: update --build-options to use 'supported_features' array
>   perf tests task_analyzer: use perf check for libtraceevent support
> 
> Athira Rajeev (1):
>   tools/perf/tests: Update probe_vfs_getname.sh script to use perf check
>     --feature
> 
>  tools/perf/Build                              |   1 +
>  tools/perf/Documentation/perf-check.txt       |  59 +++++++++
>  tools/perf/builtin-check.c                    | 122 ++++++++++++++++++
>  tools/perf/builtin-version.c                  |  39 ++----
>  tools/perf/builtin.h                          |  18 +++
>  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 +-
>  10 files changed, 221 insertions(+), 37 deletions(-)
>  create mode 100644 tools/perf/Documentation/perf-check.txt
>  create mode 100644 tools/perf/builtin-check.c
> 
> -- 
> 2.41.0
> 

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

* Re: [PATCH v8 0/4] Introduce perf check subcommand
  2023-12-06  6:12 ` [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
@ 2024-01-05  8:00   ` Aditya Gupta
  2024-01-05 15:08     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 11+ messages in thread
From: Aditya Gupta @ 2024-01-05  8:00 UTC (permalink / raw)
  To: acme, jolsa, irogers, namhyung
  Cc: linux-perf-users, maddy, atrajeev, kjain, disgoel

Hi Arnaldo,
Any comments on the series ?

Thanks,
Aditya

> On Mon, Dec 04, 2023 at 03:09:50PM +0530, Aditya Gupta 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 check --feature", with which
> > scripts can test for presence of a feature or multiple features, such as:
> > 
> >     perf check --feature HAVE_LIBTRACEEVENT
> > 
> >     or
> > 
> >     perf check --feature libtraceevent
> > 
> >     or
> > 
> >     perf check --feature libtraceevent,bpf
> > 
> > The usage of "perf version --build-options | grep" has been replaced in two
> > tests, with "perf check --feature" 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 check --feature' 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-check-feature-v8
> > 
> > Changelog
> > =========
> > V8
> > + handle return value of 'malloc' in patch #1
> > + fix error due to strncpy depending on source string's length
> > 
> > V7
> > + modified patch #1 to fix compile issue, and add feature to allow
> >   multiple comma-separated features
> > 
> > V6
> > + rebased to perf-tools-next/perf-tools-next
> > + modified patch #1 to include FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL)
> > 
> > V5
> > + invert return value of 'has_support', but return value of perf check --feature
> > according to shell convention
> > 
> > V4
> > + invert return value of perf check --feature
> > 
> > V3
> > + simplified has_support code in builtin-check.c (patch #1)
> > + modified patch #3 and patch #4 according to change in return value in patch #1
> > 
> > V2
> > + improved the patch series with suggestions from Namhyung
> > + fix incorrect return value, added -q option, and moved array definition to
> >   perf-check.c
> > 
> > V1
> > + changed subcommand name to 'perf check --feature'
> > + added documentation for perf check
> > + support both macro (eg. HAVE_LIBTRACEEVENT), and name (eg. libtraceevent) as
> >   input to 'perf check --feature'
> > + change subject and descriptions of all patch mentioning perf check instead of
> >   perf build
> > 
> > V0: Previous patch series: https://lore.kernel.org/linux-perf-users/20230825061125.24312-1-adityag@linux.ibm.com/
> > 
> > Aditya Gupta (3):
> >   perf check: introduce check subcommand
> >   perf version: update --build-options to use 'supported_features' array
> >   perf tests task_analyzer: use perf check for libtraceevent support
> > 
> > Athira Rajeev (1):
> >   tools/perf/tests: Update probe_vfs_getname.sh script to use perf check
> >     --feature
> > 
> >  tools/perf/Build                              |   1 +
> >  tools/perf/Documentation/perf-check.txt       |  59 +++++++++
> >  tools/perf/builtin-check.c                    | 122 ++++++++++++++++++
> >  tools/perf/builtin-version.c                  |  39 ++----
> >  tools/perf/builtin.h                          |  18 +++
> >  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 +-
> >  10 files changed, 221 insertions(+), 37 deletions(-)
> >  create mode 100644 tools/perf/Documentation/perf-check.txt
> >  create mode 100644 tools/perf/builtin-check.c
> > 
> > -- 
> > 2.41.0
> > 

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

* Re: [PATCH v8 0/4] Introduce perf check subcommand
  2024-01-05  8:00   ` Aditya Gupta
@ 2024-01-05 15:08     ` Arnaldo Carvalho de Melo
  2024-01-06  5:48       ` Aditya Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-01-05 15:08 UTC (permalink / raw)
  To: Aditya Gupta
  Cc: jolsa, irogers, namhyung, linux-perf-users, maddy, atrajeev,
	kjain, disgoel

Em Fri, Jan 05, 2024 at 01:30:19PM +0530, Aditya Gupta escreveu:
> Hi Arnaldo,
> Any comments on the series ?

I'm trying now to finish the pull req for v6.8, trying to avoid adding
new features, Namhyung will take care for patches for v6.9, where he
will most likely consider your latest v8 kit.

Thanks,

- Arnaldo
 
> Thanks,
> Aditya
> 
> > On Mon, Dec 04, 2023 at 03:09:50PM +0530, Aditya Gupta 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 check --feature", with which
> > > scripts can test for presence of a feature or multiple features, such as:
> > > 
> > >     perf check --feature HAVE_LIBTRACEEVENT
> > > 
> > >     or
> > > 
> > >     perf check --feature libtraceevent
> > > 
> > >     or
> > > 
> > >     perf check --feature libtraceevent,bpf
> > > 
> > > The usage of "perf version --build-options | grep" has been replaced in two
> > > tests, with "perf check --feature" 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 check --feature' 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-check-feature-v8
> > > 
> > > Changelog
> > > =========
> > > V8
> > > + handle return value of 'malloc' in patch #1
> > > + fix error due to strncpy depending on source string's length
> > > 
> > > V7
> > > + modified patch #1 to fix compile issue, and add feature to allow
> > >   multiple comma-separated features
> > > 
> > > V6
> > > + rebased to perf-tools-next/perf-tools-next
> > > + modified patch #1 to include FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL)
> > > 
> > > V5
> > > + invert return value of 'has_support', but return value of perf check --feature
> > > according to shell convention
> > > 
> > > V4
> > > + invert return value of perf check --feature
> > > 
> > > V3
> > > + simplified has_support code in builtin-check.c (patch #1)
> > > + modified patch #3 and patch #4 according to change in return value in patch #1
> > > 
> > > V2
> > > + improved the patch series with suggestions from Namhyung
> > > + fix incorrect return value, added -q option, and moved array definition to
> > >   perf-check.c
> > > 
> > > V1
> > > + changed subcommand name to 'perf check --feature'
> > > + added documentation for perf check
> > > + support both macro (eg. HAVE_LIBTRACEEVENT), and name (eg. libtraceevent) as
> > >   input to 'perf check --feature'
> > > + change subject and descriptions of all patch mentioning perf check instead of
> > >   perf build
> > > 
> > > V0: Previous patch series: https://lore.kernel.org/linux-perf-users/20230825061125.24312-1-adityag@linux.ibm.com/
> > > 
> > > Aditya Gupta (3):
> > >   perf check: introduce check subcommand
> > >   perf version: update --build-options to use 'supported_features' array
> > >   perf tests task_analyzer: use perf check for libtraceevent support
> > > 
> > > Athira Rajeev (1):
> > >   tools/perf/tests: Update probe_vfs_getname.sh script to use perf check
> > >     --feature
> > > 
> > >  tools/perf/Build                              |   1 +
> > >  tools/perf/Documentation/perf-check.txt       |  59 +++++++++
> > >  tools/perf/builtin-check.c                    | 122 ++++++++++++++++++
> > >  tools/perf/builtin-version.c                  |  39 ++----
> > >  tools/perf/builtin.h                          |  18 +++
> > >  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 +-
> > >  10 files changed, 221 insertions(+), 37 deletions(-)
> > >  create mode 100644 tools/perf/Documentation/perf-check.txt
> > >  create mode 100644 tools/perf/builtin-check.c
> > > 
> > > -- 
> > > 2.41.0
> > > 

-- 

- Arnaldo

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

* Re: [PATCH v8 1/4] perf check: introduce check subcommand
  2023-12-04  9:39 ` [PATCH v8 1/4] perf check: introduce " Aditya Gupta
@ 2024-01-05 23:29   ` Namhyung Kim
  2024-01-06 20:52     ` Aditya Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2024-01-05 23:29 UTC (permalink / raw)
  To: Aditya Gupta
  Cc: acme, jolsa, irogers, linux-perf-users, maddy, atrajeev, kjain,
	disgoel

Hello,

On Mon, Dec 4, 2023 at 1:40 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> 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 check --feature", with which
> scripts can test for presence of a feature, such as:
>
>     perf check --feature HAVE_FEATURE

Can you please change it to a sub-command instead of an option?
Like "perf check feature HAVE_FEATURE" (without dashes).

I think it's more appropriate since it's intended to return a result.
IMHO options should be used to control (optional) behaviors.
Later, we might add other checking behavior then using it
together with this won't make sense.  Making it a subcommand
would prevent that.

>
> 'perf check --feature' command is expected to have exit status of 0 if
> feature is built-in, and 1 if it's not built-in or if feature is not known.
>
> Multiple features can also be passed as a comma-separated list, in which
> case the exit status will be 1 only if all of the passed features are
> built-in. For example, with below command, it will have exit status of 1 only
> if both libtraceevent and bpf are enabled, else 0 in all other cases
>
>     perf check --feature libtraceevent,bpf
>
> An 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
>
> Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>  tools/perf/Build                        |   1 +
>  tools/perf/Documentation/perf-check.txt |  65 ++++++++++
>  tools/perf/builtin-check.c              | 155 ++++++++++++++++++++++++
>  tools/perf/builtin.h                    |  18 +++
>  tools/perf/perf.c                       |   1 +
>  5 files changed, 240 insertions(+)
>  create mode 100644 tools/perf/Documentation/perf-check.txt
>  create mode 100644 tools/perf/builtin-check.c
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index aa7623622834..a55a797c1b5f 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -1,5 +1,6 @@
>  perf-y += builtin-bench.o
>  perf-y += builtin-annotate.o
> +perf-y += builtin-check.o
>  perf-y += builtin-config.o
>  perf-y += builtin-diff.o
>  perf-y += builtin-evlist.o
> diff --git a/tools/perf/Documentation/perf-check.txt b/tools/perf/Documentation/perf-check.txt
> new file mode 100644
> index 000000000000..dfb1dddd0ac3
> --- /dev/null
> +++ b/tools/perf/Documentation/perf-check.txt
> @@ -0,0 +1,65 @@
> +perf-check(1)
> +===============
> +
> +NAME
> +----
> +perf-check - check features in perf
> +
> +SYNOPSIS
> +--------
> +'perf check' [<options>]
> +
> +DESCRIPTION
> +-----------
> +With no options given, the 'perf check' just prints the perf version
> +on the standard output.
> +
> +If the option '--feature' is given, then status of feature is printed
> +on the standard output (unless '-q' is also passed), ie. whether it is
> +compiled-in/built-in or not.
> +Also, 'perf check --feature' returns with exit status 0 if the feature
> +is built-in, otherwise returns with exit status 1.
> +
> +OPTIONS
> +-------
> +-q::
> +               Do not print any messages or warnings
> +
> +--feature::
> +        Print whether feature(s) is compiled-in or not. A feature name/macro is
> +        required to be passed after this flag
> +
> +        Multiple features can also be passed as a comma-separated list, in which
> +        case the exit status will be 1 only if all of the passed features are

Do you mean 0 instead of 1?

> +        compiled-in.
> +
> +        Example Usage:
> +                perf check --feature libtraceevent
> +                perf check --feature HAVE_LIBTRACEEVENT
> +                perf check --feature libtraceevent,bpf
> +
> +        Supported feature names/macro:
> +                dwarf      /  HAVE_DWARF_SUPPORT
> +                dwarf_getlocations  /  HAVE_DWARF_GETLOCATIONS_SUPPORT
> +                libaudit   /  HAVE_LIBAUDIT_SUPPORT
> +                syscall_table       /  HAVE_SYSCALL_TABLE_SUPPORT
> +                libbfd     /  HAVE_LIBBFD_SUPPORT
> +                debuginfod /  HAVE_DEBUGINFOD_SUPPORT
> +                libelf     /  HAVE_LIBELF_SUPPORT
> +                libnuma    /  HAVE_LIBNUMA_SUPPORT
> +                numa_num_possible_cpus  /  HAVE_LIBNUMA_SUPPORT
> +                libperl    /  HAVE_LIBPERL_SUPPORT
> +                libpython  /  HAVE_LIBPYTHON_SUPPORT
> +                libslang   /  HAVE_SLANG_SUPPORT
> +                libcrypto  /  HAVE_LIBCRYPTO_SUPPORT
> +                libunwind  /  HAVE_LIBUNWIND_SUPPORT
> +                libdw-dwarf-unwind  /  HAVE_DWARF_SUPPORT
> +                zlib       /  HAVE_ZLIB_SUPPORT
> +                lzma       /  HAVE_LZMA_SUPPORT
> +                get_cpuid  /  HAVE_AUXTRACE_SUPPORT
> +                bpf        /  HAVE_LIBBPF_SUPPORT
> +                aio        /  HAVE_AIO_SUPPORT
> +                zstd       /  HAVE_ZSTD_SUPPORT
> +                libpfm4    /  HAVE_LIBPFM
> +                libtraceevent      /  HAVE_LIBTRACEEVENT
> +                bpf_skeletons      /  HAVE_BPF_SKEL

Is it case-sensitive?  Maybe better not.

Thanks,
Namhyung


> diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
> new file mode 100644
> index 000000000000..f121d657a7bc
> --- /dev/null
> +++ b/tools/perf/builtin-check.c
> @@ -0,0 +1,155 @@
> +// 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 check {
> +       const char *feature;
> +};
> +
> +static struct check check;
> +
> +static struct option check_options[] = {
> +       OPT_STRING(0, "feature", &check.feature, NULL, "check if feature(s) is built in"),
> +       OPT_BOOLEAN('q', "quiet", &quiet, "do not show any warnings or messages"),
> +       OPT_END(),
> +};
> +
> +static const char * const check_usage[] = {
> +       "perf check [<options>]",
> +       NULL
> +};
> +
> +struct feature_support supported_features[] = {
> +       FEATURE_SUPPORT("dwarf", HAVE_DWARF_SUPPORT),
> +       FEATURE_SUPPORT("dwarf_getlocations", HAVE_DWARF_GETLOCATIONS_SUPPORT),
> +       FEATURE_SUPPORT("libaudit", HAVE_LIBAUDIT_SUPPORT),
> +       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),
> +       FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL),
> +
> +       /* this should remain at end, to know the array end */
> +       FEATURE_SUPPORT(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:
> + *    0: NOT built-in or Feature not known
> + *    1: Built-in
> + */
> +static int has_support(const char *feature)
> +{
> +       for (int i = 0; supported_features[i].name; ++i) {
> +               if ((strcmp(feature, supported_features[i].name) == 0) ||
> +                   (strcmp(feature, supported_features[i].macro) == 0)) {
> +                       if (!quiet)
> +                               STATUS(supported_features[i]);
> +                       return supported_features[i].is_builtin;
> +               }
> +       }
> +
> +       if (!quiet)
> +               color_fprintf(stdout, PERF_COLOR_RED, "Feature not known: '%s'\n", feature);
> +
> +       return 0;
> +}
> +
> +int cmd_check(int argc, const char **argv)
> +{
> +       char *feature_list;
> +       char *feature_name;
> +       int feature_enabled;
> +       int buf_len;
> +
> +       argc = parse_options(argc, argv, check_options, check_usage,
> +                            PARSE_OPT_STOP_AT_NON_OPTION);
> +
> +       if (check.feature) {
> +               /* check.feature can be a single feature name, or a comma-separated list
> +                * of feature names.
> +                * eg. check.feature can be "libtraceevent" or
> +                * "libtraceevent,bpf" etc
> +                *
> +                * In case of a comma-separated list, feature_enabled will be 1, only if
> +                * all features passed in the string are supported
> +                * Note that check.feature will get modified due to strtok and str_trim
> +                */
> +               feature_enabled = 1;
> +               buf_len = strlen(check.feature) + 1;
> +               /* feature_list is a non-const copy of 'check.feature' */
> +               feature_list = malloc(buf_len * sizeof(char));
> +               if (!feature_list) {
> +                       color_fprintf(stdout, PERF_COLOR_RED,
> +                                       "ERROR: failed to allocate memory for feature list");
> +                       return -1;
> +               }
> +
> +               memcpy(feature_list, check.feature, buf_len);
> +               feature_name = strtok(feature_list, ",");
> +
> +               while (feature_name) {
> +                       feature_enabled &= has_support(feature_name);
> +                       feature_name = strtok(NULL, ",");
> +               }
> +
> +               free(feature_list);
> +
> +               return !feature_enabled;
> +       }
> +
> +       return 0;
> +}
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index f2ab5bae2150..9f895981bc9e 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h
> @@ -2,6 +2,23 @@
>  #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_) }
> +
> +extern struct feature_support supported_features[];
> +
>  void list_common_cmds_help(void);
>  const char *help_unknown_cmd(const char *cmd);
>
> @@ -9,6 +26,7 @@ int cmd_annotate(int argc, const char **argv);
>  int cmd_bench(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_check(int argc, const char **argv);
>  int cmd_config(int argc, const char **argv);
>  int cmd_c2c(int argc, const char **argv);
>  int cmd_diff(int argc, const char **argv);
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index d3fc8090413c..6514f4121c49 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -50,6 +50,7 @@ static struct cmd_struct commands[] = {
>         { "archive",    NULL,   0 },
>         { "buildid-cache", cmd_buildid_cache, 0 },
>         { "buildid-list", cmd_buildid_list, 0 },
> +       { "check",      cmd_check,      0 },
>         { "config",     cmd_config,     0 },
>         { "c2c",        cmd_c2c,        0 },
>         { "diff",       cmd_diff,       0 },
> --
> 2.42.0
>

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

* Re: Re: [PATCH v8 0/4] Introduce perf check subcommand
  2024-01-05 15:08     ` Arnaldo Carvalho de Melo
@ 2024-01-06  5:48       ` Aditya Gupta
  0 siblings, 0 replies; 11+ messages in thread
From: Aditya Gupta @ 2024-01-06  5:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, irogers, namhyung, linux-perf-users, maddy, atrajeev,
	kjain, disgoel

On Fri, Jan 05, 2024 at 12:08:57PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jan 05, 2024 at 01:30:19PM +0530, Aditya Gupta escreveu:
> > Hi Arnaldo,
> > Any comments on the series ?
> 
> I'm trying now to finish the pull req for v6.8, trying to avoid adding
> new features, Namhyung will take care for patches for v6.9, where he
> will most likely consider your latest v8 kit.

Sure, that's okay.

Thanks,
Aditya Gupta

> 
> Thanks,
> 
> - Arnaldo
>  
> > Thanks,
> > Aditya
> > 
> > > On Mon, Dec 04, 2023 at 03:09:50PM +0530, Aditya Gupta 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 check --feature", with which
> > > > scripts can test for presence of a feature or multiple features, such as:
> > > > 
> > > >     perf check --feature HAVE_LIBTRACEEVENT
> > > > 
> > > >     or
> > > > 
> > > >     perf check --feature libtraceevent
> > > > 
> > > >     or
> > > > 
> > > >     perf check --feature libtraceevent,bpf
> > > > 
> > > > The usage of "perf version --build-options | grep" has been replaced in two
> > > > tests, with "perf check --feature" 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 check --feature' 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-check-feature-v8
> > > > 
> > > > Changelog
> > > > =========
> > > > V8
> > > > + handle return value of 'malloc' in patch #1
> > > > + fix error due to strncpy depending on source string's length
> > > > 
> > > > V7
> > > > + modified patch #1 to fix compile issue, and add feature to allow
> > > >   multiple comma-separated features
> > > > 
> > > > V6
> > > > + rebased to perf-tools-next/perf-tools-next
> > > > + modified patch #1 to include FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL)
> > > > 
> > > > V5
> > > > + invert return value of 'has_support', but return value of perf check --feature
> > > > according to shell convention
> > > > 
> > > > V4
> > > > + invert return value of perf check --feature
> > > > 
> > > > V3
> > > > + simplified has_support code in builtin-check.c (patch #1)
> > > > + modified patch #3 and patch #4 according to change in return value in patch #1
> > > > 
> > > > V2
> > > > + improved the patch series with suggestions from Namhyung
> > > > + fix incorrect return value, added -q option, and moved array definition to
> > > >   perf-check.c
> > > > 
> > > > V1
> > > > + changed subcommand name to 'perf check --feature'
> > > > + added documentation for perf check
> > > > + support both macro (eg. HAVE_LIBTRACEEVENT), and name (eg. libtraceevent) as
> > > >   input to 'perf check --feature'
> > > > + change subject and descriptions of all patch mentioning perf check instead of
> > > >   perf build
> > > > 
> > > > V0: Previous patch series: https://lore.kernel.org/linux-perf-users/20230825061125.24312-1-adityag@linux.ibm.com/
> > > > 
> > > > Aditya Gupta (3):
> > > >   perf check: introduce check subcommand
> > > >   perf version: update --build-options to use 'supported_features' array
> > > >   perf tests task_analyzer: use perf check for libtraceevent support
> > > > 
> > > > Athira Rajeev (1):
> > > >   tools/perf/tests: Update probe_vfs_getname.sh script to use perf check
> > > >     --feature
> > > > 
> > > >  tools/perf/Build                              |   1 +
> > > >  tools/perf/Documentation/perf-check.txt       |  59 +++++++++
> > > >  tools/perf/builtin-check.c                    | 122 ++++++++++++++++++
> > > >  tools/perf/builtin-version.c                  |  39 ++----
> > > >  tools/perf/builtin.h                          |  18 +++
> > > >  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 +-
> > > >  10 files changed, 221 insertions(+), 37 deletions(-)
> > > >  create mode 100644 tools/perf/Documentation/perf-check.txt
> > > >  create mode 100644 tools/perf/builtin-check.c
> > > > 
> > > > -- 
> > > > 2.41.0
> > > > 
> 
> -- 
> 
> - Arnaldo

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

* Re: Re: [PATCH v8 1/4] perf check: introduce check subcommand
  2024-01-05 23:29   ` Namhyung Kim
@ 2024-01-06 20:52     ` Aditya Gupta
  0 siblings, 0 replies; 11+ messages in thread
From: Aditya Gupta @ 2024-01-06 20:52 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: acme, jolsa, irogers, linux-perf-users, maddy, atrajeev, kjain,
	disgoel

Hi Namhyung,

On Fri, Jan 05, 2024 at 03:29:45PM -0800, Namhyung Kim wrote:
> Hello,
> 
> On Mon, Dec 4, 2023 at 1:40 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
> >
> > 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 check --feature", with which
> > scripts can test for presence of a feature, such as:
> >
> >     perf check --feature HAVE_FEATURE
> 
> Can you please change it to a sub-command instead of an option?
> Like "perf check feature HAVE_FEATURE" (without dashes).
> 
> I think it's more appropriate since it's intended to return a result.
> IMHO options should be used to control (optional) behaviors.
> Later, we might add other checking behavior then using it
> together with this won't make sense.  Making it a subcommand
> would prevent that.
> 

Okay, makes sense. I will make it a subcommand in V9, will have to
understand which OPT_* flags to pass for having a 'feature' subcommand
inside a 'check' subcommand. Seems 'OPT_GROUP' might do the thing, will
try to undersand it.

> >
> > 'perf check --feature' command is expected to have exit status of 0 if
> > feature is built-in, and 1 if it's not built-in or if feature is not known.
> >
> > Multiple features can also be passed as a comma-separated list, in which
> > case the exit status will be 1 only if all of the passed features are
> > built-in. For example, with below command, it will have exit status of 1 only
> > if both libtraceevent and bpf are enabled, else 0 in all other cases
> >
> >     perf check --feature libtraceevent,bpf
> >
> > An 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
> >
> > Reviewed-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> > ---
> >  tools/perf/Build                        |   1 +
> >  tools/perf/Documentation/perf-check.txt |  65 ++++++++++
> >  tools/perf/builtin-check.c              | 155 ++++++++++++++++++++++++
> >  tools/perf/builtin.h                    |  18 +++
> >  tools/perf/perf.c                       |   1 +
> >  5 files changed, 240 insertions(+)
> >  create mode 100644 tools/perf/Documentation/perf-check.txt
> >  create mode 100644 tools/perf/builtin-check.c
> >
> > diff --git a/tools/perf/Build b/tools/perf/Build
> > index aa7623622834..a55a797c1b5f 100644
> > --- a/tools/perf/Build
> > +++ b/tools/perf/Build
> > @@ -1,5 +1,6 @@
> >  perf-y += builtin-bench.o
> >  perf-y += builtin-annotate.o
> > +perf-y += builtin-check.o
> >  perf-y += builtin-config.o
> >  perf-y += builtin-diff.o
> >  perf-y += builtin-evlist.o
> > diff --git a/tools/perf/Documentation/perf-check.txt b/tools/perf/Documentation/perf-check.txt
> > new file mode 100644
> > index 000000000000..dfb1dddd0ac3
> > --- /dev/null
> > +++ b/tools/perf/Documentation/perf-check.txt
> > @@ -0,0 +1,65 @@
> > +perf-check(1)
> > +===============
> > +
> > +NAME
> > +----
> > +perf-check - check features in perf
> > +
> > +SYNOPSIS
> > +--------
> > +'perf check' [<options>]
> > +
> > +DESCRIPTION
> > +-----------
> > +With no options given, the 'perf check' just prints the perf version
> > +on the standard output.
> > +
> > +If the option '--feature' is given, then status of feature is printed
> > +on the standard output (unless '-q' is also passed), ie. whether it is
> > +compiled-in/built-in or not.
> > +Also, 'perf check --feature' returns with exit status 0 if the feature
> > +is built-in, otherwise returns with exit status 1.
> > +
> > +OPTIONS
> > +-------
> > +-q::
> > +               Do not print any messages or warnings
> > +
> > +--feature::
> > +        Print whether feature(s) is compiled-in or not. A feature name/macro is
> > +        required to be passed after this flag
> > +
> > +        Multiple features can also be passed as a comma-separated list, in which
> > +        case the exit status will be 1 only if all of the passed features are
> 
> Do you mean 0 instead of 1?

Yes, thanks for noticing this mistake.

> 
> > +        compiled-in.
> > +
> > +        Example Usage:
> > +                perf check --feature libtraceevent
> > +                perf check --feature HAVE_LIBTRACEEVENT
> > +                perf check --feature libtraceevent,bpf
> > +
> > +        Supported feature names/macro:
> > +                dwarf      /  HAVE_DWARF_SUPPORT
> > +                dwarf_getlocations  /  HAVE_DWARF_GETLOCATIONS_SUPPORT
> > +                libaudit   /  HAVE_LIBAUDIT_SUPPORT
> > +                syscall_table       /  HAVE_SYSCALL_TABLE_SUPPORT
> > +                libbfd     /  HAVE_LIBBFD_SUPPORT
> > +                debuginfod /  HAVE_DEBUGINFOD_SUPPORT
> > +                libelf     /  HAVE_LIBELF_SUPPORT
> > +                libnuma    /  HAVE_LIBNUMA_SUPPORT
> > +                numa_num_possible_cpus  /  HAVE_LIBNUMA_SUPPORT
> > +                libperl    /  HAVE_LIBPERL_SUPPORT
> > +                libpython  /  HAVE_LIBPYTHON_SUPPORT
> > +                libslang   /  HAVE_SLANG_SUPPORT
> > +                libcrypto  /  HAVE_LIBCRYPTO_SUPPORT
> > +                libunwind  /  HAVE_LIBUNWIND_SUPPORT
> > +                libdw-dwarf-unwind  /  HAVE_DWARF_SUPPORT
> > +                zlib       /  HAVE_ZLIB_SUPPORT
> > +                lzma       /  HAVE_LZMA_SUPPORT
> > +                get_cpuid  /  HAVE_AUXTRACE_SUPPORT
> > +                bpf        /  HAVE_LIBBPF_SUPPORT
> > +                aio        /  HAVE_AIO_SUPPORT
> > +                zstd       /  HAVE_ZSTD_SUPPORT
> > +                libpfm4    /  HAVE_LIBPFM
> > +                libtraceevent      /  HAVE_LIBTRACEEVENT
> > +                bpf_skeletons      /  HAVE_BPF_SKEL
> 
> Is it case-sensitive?  Maybe better not.

Yes, with current code it is case-sensitive. Will make the check
case-insensitive in V9.

Thanks for the reviews,

- Aditya Gupta

> 
> Thanks,
> Namhyung
> 
> 
> > diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
> > new file mode 100644
> > index 000000000000..f121d657a7bc
> > --- /dev/null
> > +++ b/tools/perf/builtin-check.c
> > @@ -0,0 +1,155 @@
> > +// 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 check {
> > +       const char *feature;
> > +};
> > +
> > +static struct check check;
> > +
> > +static struct option check_options[] = {
> > +       OPT_STRING(0, "feature", &check.feature, NULL, "check if feature(s) is built in"),
> > +       OPT_BOOLEAN('q', "quiet", &quiet, "do not show any warnings or messages"),
> > +       OPT_END(),
> > +};
> > +
> > +static const char * const check_usage[] = {
> > +       "perf check [<options>]",
> > +       NULL
> > +};
> > +
> > +struct feature_support supported_features[] = {
> > +       FEATURE_SUPPORT("dwarf", HAVE_DWARF_SUPPORT),
> > +       FEATURE_SUPPORT("dwarf_getlocations", HAVE_DWARF_GETLOCATIONS_SUPPORT),
> > +       FEATURE_SUPPORT("libaudit", HAVE_LIBAUDIT_SUPPORT),
> > +       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),
> > +       FEATURE_SUPPORT("bpf_skeletons", HAVE_BPF_SKEL),
> > +
> > +       /* this should remain at end, to know the array end */
> > +       FEATURE_SUPPORT(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:
> > + *    0: NOT built-in or Feature not known
> > + *    1: Built-in
> > + */
> > +static int has_support(const char *feature)
> > +{
> > +       for (int i = 0; supported_features[i].name; ++i) {
> > +               if ((strcmp(feature, supported_features[i].name) == 0) ||
> > +                   (strcmp(feature, supported_features[i].macro) == 0)) {
> > +                       if (!quiet)
> > +                               STATUS(supported_features[i]);
> > +                       return supported_features[i].is_builtin;
> > +               }
> > +       }
> > +
> > +       if (!quiet)
> > +               color_fprintf(stdout, PERF_COLOR_RED, "Feature not known: '%s'\n", feature);
> > +
> > +       return 0;
> > +}
> > +
> > +int cmd_check(int argc, const char **argv)
> > +{
> > +       char *feature_list;
> > +       char *feature_name;
> > +       int feature_enabled;
> > +       int buf_len;
> > +
> > +       argc = parse_options(argc, argv, check_options, check_usage,
> > +                            PARSE_OPT_STOP_AT_NON_OPTION);
> > +
> > +       if (check.feature) {
> > +               /* check.feature can be a single feature name, or a comma-separated list
> > +                * of feature names.
> > +                * eg. check.feature can be "libtraceevent" or
> > +                * "libtraceevent,bpf" etc
> > +                *
> > +                * In case of a comma-separated list, feature_enabled will be 1, only if
> > +                * all features passed in the string are supported
> > +                * Note that check.feature will get modified due to strtok and str_trim
> > +                */
> > +               feature_enabled = 1;
> > +               buf_len = strlen(check.feature) + 1;
> > +               /* feature_list is a non-const copy of 'check.feature' */
> > +               feature_list = malloc(buf_len * sizeof(char));
> > +               if (!feature_list) {
> > +                       color_fprintf(stdout, PERF_COLOR_RED,
> > +                                       "ERROR: failed to allocate memory for feature list");
> > +                       return -1;
> > +               }
> > +
> > +               memcpy(feature_list, check.feature, buf_len);
> > +               feature_name = strtok(feature_list, ",");
> > +
> > +               while (feature_name) {
> > +                       feature_enabled &= has_support(feature_name);
> > +                       feature_name = strtok(NULL, ",");
> > +               }
> > +
> > +               free(feature_list);
> > +
> > +               return !feature_enabled;
> > +       }
> > +
> > +       return 0;
> > +}
> > diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> > index f2ab5bae2150..9f895981bc9e 100644
> > --- a/tools/perf/builtin.h
> > +++ b/tools/perf/builtin.h
> > @@ -2,6 +2,23 @@
> >  #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_) }
> > +
> > +extern struct feature_support supported_features[];
> > +
> >  void list_common_cmds_help(void);
> >  const char *help_unknown_cmd(const char *cmd);
> >
> > @@ -9,6 +26,7 @@ int cmd_annotate(int argc, const char **argv);
> >  int cmd_bench(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_check(int argc, const char **argv);
> >  int cmd_config(int argc, const char **argv);
> >  int cmd_c2c(int argc, const char **argv);
> >  int cmd_diff(int argc, const char **argv);
> > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > index d3fc8090413c..6514f4121c49 100644
> > --- a/tools/perf/perf.c
> > +++ b/tools/perf/perf.c
> > @@ -50,6 +50,7 @@ static struct cmd_struct commands[] = {
> >         { "archive",    NULL,   0 },
> >         { "buildid-cache", cmd_buildid_cache, 0 },
> >         { "buildid-list", cmd_buildid_list, 0 },
> > +       { "check",      cmd_check,      0 },
> >         { "config",     cmd_config,     0 },
> >         { "c2c",        cmd_c2c,        0 },
> >         { "diff",       cmd_diff,       0 },
> > --
> > 2.42.0
> >

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

end of thread, other threads:[~2024-01-06 20:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04  9:39 [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
2023-12-04  9:39 ` [PATCH v8 1/4] perf check: introduce " Aditya Gupta
2024-01-05 23:29   ` Namhyung Kim
2024-01-06 20:52     ` Aditya Gupta
2023-12-04  9:39 ` [PATCH v8 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
2023-12-04  9:39 ` [PATCH v8 3/4] perf tests task_analyzer: use perf check for libtraceevent support Aditya Gupta
2023-12-04  9:39 ` [PATCH v8 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf check --feature Aditya Gupta
2023-12-06  6:12 ` [PATCH v8 0/4] Introduce perf check subcommand Aditya Gupta
2024-01-05  8:00   ` Aditya Gupta
2024-01-05 15:08     ` Arnaldo Carvalho de Melo
2024-01-06  5:48       ` 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).