* [PATCH v1 1/6] perf ui: Move window resize signal functions
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 1:40 ` [PATCH v1 2/6] perf usage: Move usage strings Ian Rogers
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
Move under tools/perf/ui rather than in perf.c. This simplifies
linking a main function when perf.c isn't present.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/perf.c | 18 ------------------
tools/perf/perf.h | 2 --
tools/perf/ui/setup.c | 19 +++++++++++++++++++
tools/perf/ui/tui/setup.c | 1 -
tools/perf/ui/ui.h | 3 +++
5 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 82bbe0ca858b..4b1b31e78332 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -432,24 +432,6 @@ static int run_argv(int *argcp, const char ***argv)
return 0;
}
-static void pthread__block_sigwinch(void)
-{
- sigset_t set;
-
- sigemptyset(&set);
- sigaddset(&set, SIGWINCH);
- pthread_sigmask(SIG_BLOCK, &set, NULL);
-}
-
-void pthread__unblock_sigwinch(void)
-{
- sigset_t set;
-
- sigemptyset(&set);
- sigaddset(&set, SIGWINCH);
- pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-}
-
static int libperf_print(enum libperf_print_level level,
const char *fmt, va_list ap)
{
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 74014033df60..e21a7e15a34c 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -12,8 +12,6 @@ extern const char *input_name;
extern bool perf_host, perf_guest;
extern const char perf_version_string[];
-void pthread__unblock_sigwinch(void);
-
enum perf_affinity {
PERF_AFFINITY_SYS = 0,
PERF_AFFINITY_NODE,
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 25ded88801a3..ff800047e697 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <dlfcn.h>
+#include <signal.h>
#include <unistd.h>
#include <subcmd/pager.h>
@@ -120,3 +121,21 @@ void exit_browser(bool wait_for_ok)
}
mutex_destroy(&ui__lock);
}
+
+void pthread__block_sigwinch(void)
+{
+ sigset_t set;
+
+ sigemptyset(&set);
+ sigaddset(&set, SIGWINCH);
+ pthread_sigmask(SIG_BLOCK, &set, NULL);
+}
+
+void pthread__unblock_sigwinch(void)
+{
+ sigset_t set;
+
+ sigemptyset(&set);
+ sigaddset(&set, SIGWINCH);
+ pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+}
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index a3b8c397c24d..c1886aa184b3 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -9,7 +9,6 @@
#endif
#include "../../util/debug.h"
-#include "../../perf.h"
#include "../browser.h"
#include "../helpline.h"
#include "../ui.h"
diff --git a/tools/perf/ui/ui.h b/tools/perf/ui/ui.h
index 99f8d2fe9bc5..d8e911205582 100644
--- a/tools/perf/ui/ui.h
+++ b/tools/perf/ui/ui.h
@@ -31,4 +31,7 @@ struct option;
int stdio__config_color(const struct option *opt, const char *mode, int unset);
+void pthread__block_sigwinch(void);
+void pthread__unblock_sigwinch(void);
+
#endif /* _PERF_UI_H_ */
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v1 2/6] perf usage: Move usage strings
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
2023-03-28 1:40 ` [PATCH v1 1/6] perf ui: Move window resize signal functions Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 1:40 ` [PATCH v1 3/6] perf header: Move perf_version_string declaration Ian Rogers
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
This simplifies linking a main function when perf.c isn't present.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-help.c | 1 +
tools/perf/builtin.h | 3 ---
tools/perf/perf.c | 6 ------
tools/perf/util/usage.c | 6 ++++++
tools/perf/util/util.h | 3 +++
5 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 3976aebe3677..3e7f52054fac 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -14,6 +14,7 @@
#include <subcmd/run-command.h>
#include <subcmd/help.h>
#include "util/debug.h"
+#include "util/util.h"
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/zalloc.h>
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index d03afea86217..f2ab5bae2150 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -2,9 +2,6 @@
#ifndef BUILTIN_H
#define BUILTIN_H
-extern const char perf_usage_string[];
-extern const char perf_more_info_string[];
-
void list_common_cmds_help(void);
const char *help_unknown_cmd(const char *cmd);
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 4b1b31e78332..997bb9ea5ebc 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -39,12 +39,6 @@
#include <linux/string.h>
#include <linux/zalloc.h>
-const char perf_usage_string[] =
- "perf [--version] [--help] [OPTIONS] COMMAND [ARGS]";
-
-const char perf_more_info_string[] =
- "See 'perf help COMMAND' for more information on a specific command.";
-
static int use_pager = -1;
const char *input_name;
diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
index 196438ee4c9d..4c8ffbad2323 100644
--- a/tools/perf/util/usage.c
+++ b/tools/perf/util/usage.c
@@ -12,6 +12,12 @@
#include <stdlib.h>
#include <linux/compiler.h>
+const char perf_usage_string[] =
+ "perf [--version] [--help] [OPTIONS] COMMAND [ARGS]";
+
+const char perf_more_info_string[] =
+ "See 'perf help COMMAND' for more information on a specific command.";
+
static __noreturn void usage_builtin(const char *err)
{
fprintf(stderr, "\n Usage: %s\n", err);
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 1d3b300af5a1..5010abf9e01e 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -15,6 +15,9 @@
#include <internal/cpumap.h>
#endif
+extern const char perf_usage_string[];
+extern const char perf_more_info_string[];
+
/* General helper functions */
void usage(const char *err) __noreturn;
void die(const char *err, ...) __noreturn __printf(1, 2);
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v1 3/6] perf header: Move perf_version_string declaration
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
2023-03-28 1:40 ` [PATCH v1 1/6] perf ui: Move window resize signal functions Ian Rogers
2023-03-28 1:40 ` [PATCH v1 2/6] perf usage: Move usage strings Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 1:40 ` [PATCH v1 4/6] perf version: Use regular verbose flag Ian Rogers
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
Move to match the definition in header.c.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-version.c | 2 +-
tools/perf/perf.h | 1 -
| 2 ++
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index c5d03a11e565..4a43043ca8ef 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include "builtin.h"
-#include "perf.h"
#include "color.h"
+#include "util/header.h"
#include <tools/config.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e21a7e15a34c..20e9b93f8a39 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -10,7 +10,6 @@
extern const char *input_name;
extern bool perf_host, perf_guest;
-extern const char perf_version_string[];
enum perf_affinity {
PERF_AFFINITY_SYS = 0,
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index e3861ae62172..59eeb4a32ac5 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -115,6 +115,8 @@ struct perf_session;
struct perf_tool;
union perf_event;
+extern const char perf_version_string[];
+
int perf_session__read_header(struct perf_session *session, int repipe_fd);
int perf_session__write_header(struct perf_session *session,
struct evlist *evlist,
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v1 4/6] perf version: Use regular verbose flag
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
` (2 preceding siblings ...)
2023-03-28 1:40 ` [PATCH v1 3/6] perf header: Move perf_version_string declaration Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 1:40 ` [PATCH v1 5/6] perf util: Move input_name to util Ian Rogers
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
Remove additional version_verbose flag by using the existing verbose
variable.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-version.c | 5 ++---
tools/perf/perf.c | 2 +-
tools/perf/perf.h | 1 -
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-version.c b/tools/perf/builtin-version.c
index 4a43043ca8ef..1872c90a6667 100644
--- a/tools/perf/builtin-version.c
+++ b/tools/perf/builtin-version.c
@@ -1,6 +1,7 @@
// 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>
@@ -8,8 +9,6 @@
#include <string.h>
#include <subcmd/parse-options.h>
-int version_verbose;
-
struct version {
bool build_options;
};
@@ -92,7 +91,7 @@ int cmd_version(int argc, const char **argv)
printf("perf version %s\n", perf_version_string);
- if (version.build_options || version_verbose == 1)
+ if (version.build_options || verbose > 0)
library_status();
return 0;
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 997bb9ea5ebc..441c354b8304 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -212,7 +212,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
if (!strcmp(cmd, "-vv")) {
(*argv)[0] = "version";
- version_verbose = 1;
+ verbose = 1;
break;
}
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 20e9b93f8a39..989eb17ec474 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -18,5 +18,4 @@ enum perf_affinity {
PERF_AFFINITY_MAX
};
-extern int version_verbose;
#endif
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v1 5/6] perf util: Move input_name to util
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
` (3 preceding siblings ...)
2023-03-28 1:40 ` [PATCH v1 4/6] perf version: Use regular verbose flag Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 1:40 ` [PATCH v1 6/6] perf util: Move perf_guest/host declarations Ian Rogers
2023-03-28 13:24 ` [PATCH v1 0/6] Simplify linking against tools/perf code Adrian Hunter
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
input_name is the name of the input perf.data file, but having it in
perf.c makes it difficult to link without perf.c. Move it to util.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-annotate.c | 2 +-
tools/perf/builtin-buildid-list.c | 2 +-
tools/perf/builtin-c2c.c | 2 +-
tools/perf/builtin-data.c | 2 +-
tools/perf/builtin-evlist.c | 2 +-
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-kwork.c | 2 +-
tools/perf/builtin-mem.c | 2 +-
tools/perf/builtin-sched.c | 2 +-
tools/perf/builtin-stat.c | 2 +-
tools/perf/builtin-timechart.c | 2 +-
tools/perf/perf.c | 1 -
tools/perf/perf.h | 1 -
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/util/util.c | 2 ++
tools/perf/util/util.h | 2 ++
16 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4750fac7bf93..5a93a37318cc 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -15,7 +15,6 @@
#include <linux/zalloc.h>
#include "util/symbol.h"
-#include "perf.h"
#include "util/debug.h"
#include "util/evlist.h"
@@ -36,6 +35,7 @@
#include "util/block-range.h"
#include "util/map_symbol.h"
#include "util/branch.h"
+#include "util/util.h"
#include <dlfcn.h>
#include <errno.h>
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 00bfe89f0b5d..b6816c4dd889 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -8,7 +8,6 @@
* Copyright (C) 2009, Arnaldo Carvalho de Melo <acme@redhat.com>
*/
#include "builtin.h"
-#include "perf.h"
#include "util/build-id.h"
#include "util/debug.h"
#include "util/dso.h"
@@ -18,6 +17,7 @@
#include "util/session.h"
#include "util/symbol.h"
#include "util/data.h"
+#include "util/util.h"
#include <errno.h>
#include <inttypes.h>
#include <linux/err.h>
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index d3181fee4d3d..6c12f0865860 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -41,10 +41,10 @@
#include "symbol.h"
#include "ui/ui.h"
#include "ui/progress.h"
-#include "../perf.h"
#include "pmu.h"
#include "pmu-hybrid.h"
#include "string2.h"
+#include "util/util.h"
struct c2c_hists {
struct hists hists;
diff --git a/tools/perf/builtin-data.c b/tools/perf/builtin-data.c
index b2a9a3b7f68d..ce51cbf6dc97 100644
--- a/tools/perf/builtin-data.c
+++ b/tools/perf/builtin-data.c
@@ -3,10 +3,10 @@
#include <stdio.h>
#include <string.h>
#include "builtin.h"
-#include "perf.h"
#include "debug.h"
#include <subcmd/parse-options.h>
#include "data-convert.h"
+#include "util/util.h"
typedef int (*data_cmd_fn_t)(int argc, const char **argv);
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index b1076177c37f..7117656939e7 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -7,7 +7,6 @@
#include <linux/list.h>
-#include "perf.h"
#include "util/evlist.h"
#include "util/evsel.h"
#include "util/evsel_fprintf.h"
@@ -18,6 +17,7 @@
#include "util/debug.h"
#include <linux/err.h>
#include "util/tool.h"
+#include "util/util.h"
static int process_header_feature(struct perf_session *session __maybe_unused,
union perf_event *event __maybe_unused)
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index f3029742b800..8bc2d1775ed8 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
#include "builtin.h"
-#include "perf.h"
#include "util/dso.h"
#include "util/evlist.h"
@@ -24,6 +23,7 @@
#include "util/debug.h"
#include "util/string2.h"
+#include "util/util.h"
#include <linux/kernel.h>
#include <linux/numa.h>
diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index dc59d75180d1..a9395c52b23b 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -6,7 +6,6 @@
*/
#include "builtin.h"
-#include "perf.h"
#include "util/data.h"
#include "util/evlist.h"
@@ -20,6 +19,7 @@
#include "util/string2.h"
#include "util/callchain.h"
#include "util/evsel_fprintf.h"
+#include "util/util.h"
#include <subcmd/pager.h>
#include <subcmd/parse-options.h>
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index dedd612eae5e..93caf50df895 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -4,7 +4,6 @@
#include <sys/stat.h>
#include <unistd.h>
#include "builtin.h"
-#include "perf.h"
#include <subcmd/parse-options.h>
#include "util/auxtrace.h"
@@ -22,6 +21,7 @@
#include "util/pmu-hybrid.h"
#include "util/sample.h"
#include "util/string2.h"
+#include "util/util.h"
#include <linux/err.h>
#define MEM_OPERATION_LOAD 0x1
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 86e18575c9be..96a0dceadeff 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
#include "builtin.h"
-#include "perf.h"
#include "perf-sys.h"
#include "util/cpumap.h"
@@ -27,6 +26,7 @@
#include "util/debug.h"
#include "util/event.h"
+#include "util/util.h"
#include <linux/kernel.h>
#include <linux/log2.h>
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7ef565ae7265..31ac7a68a36c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -41,7 +41,6 @@
*/
#include "builtin.h"
-#include "perf.h"
#include "util/cgroup.h"
#include <subcmd/parse-options.h>
#include "util/parse-events.h"
@@ -71,6 +70,7 @@
#include "util/bpf_counter.h"
#include "util/iostat.h"
#include "util/pmu-hybrid.h"
+#include "util/util.h"
#include "asm/bug.h"
#include <linux/time64.h>
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 6c629e7d370a..bce1cf896f9c 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -24,7 +24,6 @@
#include "util/thread.h"
#include "util/callchain.h"
-#include "perf.h"
#include "util/header.h"
#include <subcmd/pager.h>
#include <subcmd/parse-options.h>
@@ -37,6 +36,7 @@
#include "util/debug.h"
#include "util/string2.h"
#include "util/tracepoint.h"
+#include "util/util.h"
#include <linux/err.h>
#include <traceevent/event-parse.h>
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 441c354b8304..38cae4721583 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -40,7 +40,6 @@
#include <linux/zalloc.h>
static int use_pager = -1;
-const char *input_name;
struct cmd_struct {
const char *cmd;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 989eb17ec474..49e15e2be49e 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -8,7 +8,6 @@
#define MAX_NR_CPUS 2048
#endif
-extern const char *input_name;
extern bool perf_host, perf_guest;
enum perf_affinity {
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b72ee6822222..23d1b592ebc0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -29,8 +29,8 @@
#include "../../util/top.h"
#include "../../util/thread.h"
#include "../../util/block-info.h"
+#include "../../util/util.h"
#include "../../arch/common.h"
-#include "../../perf.h"
#include "../browsers/hists.h"
#include "../helpline.h"
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index b356c9f7f0c3..0cdd459a319d 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -28,6 +28,8 @@
* XXX We need to find a better place for these things...
*/
+const char *input_name;
+
bool perf_singlethreaded = true;
void perf_set_singlethreaded(void)
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 5010abf9e01e..8bd515b67739 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -18,6 +18,8 @@
extern const char perf_usage_string[];
extern const char perf_more_info_string[];
+extern const char *input_name;
+
/* General helper functions */
void usage(const char *err) __noreturn;
void die(const char *err, ...) __noreturn __printf(1, 2);
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v1 6/6] perf util: Move perf_guest/host declarations
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
` (4 preceding siblings ...)
2023-03-28 1:40 ` [PATCH v1 5/6] perf util: Move input_name to util Ian Rogers
@ 2023-03-28 1:40 ` Ian Rogers
2023-03-28 13:24 ` [PATCH v1 0/6] Simplify linking against tools/perf code Adrian Hunter
6 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 1:40 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, John Garry, Will Deacon, James Clark,
Mike Leach, Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
The definitions are in util.c so move the declarations to match.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-diff.c | 2 +-
tools/perf/builtin-kvm.c | 1 +
tools/perf/perf.h | 4 ----
tools/perf/ui/hist.c | 2 +-
tools/perf/util/cs-etm.c | 1 +
tools/perf/util/event.c | 2 +-
tools/perf/util/evlist.c | 1 +
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/session.c | 2 +-
tools/perf/util/top.c | 2 +-
tools/perf/util/util.h | 3 +++
11 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 22b526766e14..dbb0562d6a4f 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -6,7 +6,6 @@
* DSOs and symbol information, sort them and produce a diff.
*/
#include "builtin.h"
-#include "perf.h"
#include "util/debug.h"
#include "util/event.h"
@@ -26,6 +25,7 @@
#include "util/spark.h"
#include "util/block-info.h"
#include "util/stream.h"
+#include "util/util.h"
#include <linux/err.h>
#include <linux/zalloc.h>
#include <subcmd/pager.h>
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index fb9dc0dc46f9..747d19336340 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -23,6 +23,7 @@
#include "util/data.h"
#include "util/ordered-events.h"
#include "util/kvm-stat.h"
+#include "util/util.h"
#include "ui/browsers/hists.h"
#include "ui/progress.h"
#include "ui/ui.h"
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 49e15e2be49e..c004dd4e65a3 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -2,14 +2,10 @@
#ifndef _PERF_PERF_H
#define _PERF_PERF_H
-#include <stdbool.h>
-
#ifndef MAX_NR_CPUS
#define MAX_NR_CPUS 2048
#endif
-extern bool perf_host, perf_guest;
-
enum perf_affinity {
PERF_AFFINITY_SYS = 0,
PERF_AFFINITY_NODE,
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 5075ecead5f3..f164bd26fc41 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -11,7 +11,7 @@
#include "../util/sort.h"
#include "../util/evsel.h"
#include "../util/evlist.h"
-#include "../perf.h"
+#include "../util/util.h"
/* hist period print (hpp) functions */
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index f65bac5ddbdb..9e9c85533f05 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -38,6 +38,7 @@
#include "tsc.h"
#include <tools/libc_compat.h>
#include "util/synthetic-events.h"
+#include "util/util.h"
struct cs_etm_auxtrace {
struct auxtrace auxtrace;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fa14598b916..31474b31899a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -33,7 +33,7 @@
#include "bpf-event.h"
#include "print_binary.h"
#include "tool.h"
-#include "../perf.h"
+#include "util.h"
static const char *perf_event__names[] = {
[0] = "TOTAL",
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index cc491a037836..df6af38ca22e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -32,6 +32,7 @@
#include "util/pmu.h"
#include "util/sample.h"
#include "util/bpf-filter.h"
+#include "util/util.h"
#include <signal.h>
#include <unistd.h>
#include <sched.h>
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index cc8e8766ca30..5ecde113e642 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -25,10 +25,10 @@
#include "util/parse-branch-options.h"
#include "util/evsel_config.h"
#include "util/event.h"
-#include "perf.h"
#include "util/parse-events-hybrid.h"
#include "util/pmu-hybrid.h"
#include "util/bpf-filter.h"
+#include "util/util.h"
#include "tracepoint.h"
#include "thread_map.h"
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7d8d057d1772..e2806791c76a 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -33,7 +33,7 @@
#include "stat.h"
#include "tsc.h"
#include "ui/progress.h"
-#include "../perf.h"
+#include "util.h"
#include "arch/common.h"
#include "units.h"
#include <internal/lib.h>
diff --git a/tools/perf/util/top.c b/tools/perf/util/top.c
index b8b32431d2f7..be7157de0451 100644
--- a/tools/perf/util/top.c
+++ b/tools/perf/util/top.c
@@ -11,7 +11,7 @@
#include "parse-events.h"
#include "symbol.h"
#include "top.h"
-#include "../perf.h"
+#include "util.h"
#include <inttypes.h>
#define SNPRINTF(buf, size, fmt, args...) \
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 8bd515b67739..7c8915d92dca 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -20,6 +20,9 @@ extern const char perf_more_info_string[];
extern const char *input_name;
+extern bool perf_host;
+extern bool perf_guest;
+
/* General helper functions */
void usage(const char *err) __noreturn;
void die(const char *err, ...) __noreturn __printf(1, 2);
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 0/6] Simplify linking against tools/perf code
2023-03-28 1:40 [PATCH v1 0/6] Simplify linking against tools/perf code Ian Rogers
` (5 preceding siblings ...)
2023-03-28 1:40 ` [PATCH v1 6/6] perf util: Move perf_guest/host declarations Ian Rogers
@ 2023-03-28 13:24 ` Adrian Hunter
2023-03-28 16:14 ` Ian Rogers
6 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2023-03-28 13:24 UTC (permalink / raw)
To: Ian Rogers, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Adrian Hunter, John Garry, Will Deacon, James Clark, Mike Leach,
Leo Yan, Mathieu Poirier, Suzuki K Poulose, Kan Liang,
Raul Silvera, Athira Rajeev, Ravi Bangoria, Florian Fischer,
Rob Herring, Xing Zhengjun, Sean Christopherson, Chengdong Li,
Denis Nikitin, Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
On 28/03/23 04:40, Ian Rogers wrote:
> When fuzzing something like parse-events, having the main function in
> perf.c alongside global variables like input_name means that
> input_name must be redeclared with the fuzzer function's
> main. However, as the fuzzer is using the tools/perf code as a library
> this causes backward linking reference that the linker may warn
> about. Reorganize perf.c and perf.h to avoid potential backward
> references, or so that the declaration/definition locations are more
> consistent.
>
Seems like it could be a pain to maintain.
Did you consider just adding:
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 82bbe0ca858b..a75dd47d68ee 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
return veprintf(level, verbose, fmt, ap);
}
+#ifndef CUSTOM_MAIN
int main(int argc, const char **argv)
{
int err;
@@ -576,3 +577,4 @@ int main(int argc, const char **argv)
out:
return 1;
}
+#endif
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 0/6] Simplify linking against tools/perf code
2023-03-28 13:24 ` [PATCH v1 0/6] Simplify linking against tools/perf code Adrian Hunter
@ 2023-03-28 16:14 ` Ian Rogers
2023-03-28 17:11 ` Adrian Hunter
0 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 16:14 UTC (permalink / raw)
To: Adrian Hunter
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
John Garry, Will Deacon, James Clark, Mike Leach, Leo Yan,
Mathieu Poirier, Suzuki K Poulose, Kan Liang, Raul Silvera,
Athira Rajeev, Ravi Bangoria, Florian Fischer, Rob Herring,
Xing Zhengjun, Sean Christopherson, Chengdong Li, Denis Nikitin,
Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
On Tue, Mar 28, 2023 at 6:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 28/03/23 04:40, Ian Rogers wrote:
> > When fuzzing something like parse-events, having the main function in
> > perf.c alongside global variables like input_name means that
> > input_name must be redeclared with the fuzzer function's
> > main. However, as the fuzzer is using the tools/perf code as a library
> > this causes backward linking reference that the linker may warn
> > about. Reorganize perf.c and perf.h to avoid potential backward
> > references, or so that the declaration/definition locations are more
> > consistent.
> >
>
> Seems like it could be a pain to maintain.
>
> Did you consider just adding:
>
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 82bbe0ca858b..a75dd47d68ee 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
> return veprintf(level, verbose, fmt, ap);
> }
>
> +#ifndef CUSTOM_MAIN
> int main(int argc, const char **argv)
> {
> int err;
> @@ -576,3 +577,4 @@ int main(int argc, const char **argv)
> out:
> return 1;
> }
> +#endif
>
It's possible. Would need to make the static functions not warn about
being declared and not used. I still think that just aligning
definitions and declarations yields the most expected code and will
lead to fewer problems in the long run.
Thanks,
Ian
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v1 0/6] Simplify linking against tools/perf code
2023-03-28 16:14 ` Ian Rogers
@ 2023-03-28 17:11 ` Adrian Hunter
2023-03-28 17:42 ` Ian Rogers
0 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2023-03-28 17:11 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
John Garry, Will Deacon, James Clark, Mike Leach, Leo Yan,
Mathieu Poirier, Suzuki K Poulose, Kan Liang, Raul Silvera,
Athira Rajeev, Ravi Bangoria, Florian Fischer, Rob Herring,
Xing Zhengjun, Sean Christopherson, Chengdong Li, Denis Nikitin,
Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
On 28/03/23 19:14, Ian Rogers wrote:
> On Tue, Mar 28, 2023 at 6:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 28/03/23 04:40, Ian Rogers wrote:
>>> When fuzzing something like parse-events, having the main function in
>>> perf.c alongside global variables like input_name means that
>>> input_name must be redeclared with the fuzzer function's
>>> main. However, as the fuzzer is using the tools/perf code as a library
>>> this causes backward linking reference that the linker may warn
>>> about. Reorganize perf.c and perf.h to avoid potential backward
>>> references, or so that the declaration/definition locations are more
>>> consistent.
>>>
>>
>> Seems like it could be a pain to maintain.
>>
>> Did you consider just adding:
>>
>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> index 82bbe0ca858b..a75dd47d68ee 100644
>> --- a/tools/perf/perf.c
>> +++ b/tools/perf/perf.c
>> @@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
>> return veprintf(level, verbose, fmt, ap);
>> }
>>
>> +#ifndef CUSTOM_MAIN
>> int main(int argc, const char **argv)
>> {
>> int err;
>> @@ -576,3 +577,4 @@ int main(int argc, const char **argv)
>> out:
>> return 1;
>> }
>> +#endif
>>
>
> It's possible. Would need to make the static functions not warn about
> being declared and not used. I still think that just aligning
> definitions and declarations yields the most expected code and will
> lead to fewer problems in the long run.
Making perf source dependent on an unknown derivative makes
things more complicated.
If you are not going to contribute it to perf, then a
suggestion is along the lines of the following:
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 82bbe0ca858b..6a7fe1534664 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -456,7 +456,18 @@ static int libperf_print(enum libperf_print_level level,
return veprintf(level, verbose, fmt, ap);
}
$ git diff
+#ifdef CUSTOM_MAIN
+int main(void)
+{
+ printf("This is not perf\n");
+ return 0;
+}
+
+int perf_main(int argc, const char **argv);
+int perf_main(int argc, const char **argv)
+#else
int main(int argc, const char **argv)
+#endif
{
int err;
const char *cmd;
$ make EXTRA_CFLAGS="-DCUSTOM_MAIN" NO_BPF_SKEL=1 -C tools/perf >/dev/null
Warning: Kernel ABI header at 'tools/include/uapi/linux/in.h' differs from latest version at 'include/uapi/linux/in.h'
Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
Warning: Kernel ABI header at 'tools/arch/arm64/include/uapi/asm/perf_regs.h' differs from latest version at 'arch/arm64/include/uapi/asm/perf_regs.h'
Warning: Kernel ABI header at 'tools/include/linux/coresight-pmu.h' differs from latest version at 'include/linux/coresight-pmu.h'
Makefile.config:587: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-dev
Makefile.config:805: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
Makefile.config:1046: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
Makefile.config:1075: No alternatives command found, you need to set JDIR= to point to the root of your Java directory
Makefile.config:1137: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
$ tools/perf/perf version
This is not perf
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v1 0/6] Simplify linking against tools/perf code
2023-03-28 17:11 ` Adrian Hunter
@ 2023-03-28 17:42 ` Ian Rogers
2023-03-30 5:39 ` Adrian Hunter
0 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2023-03-28 17:42 UTC (permalink / raw)
To: Adrian Hunter
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
John Garry, Will Deacon, James Clark, Mike Leach, Leo Yan,
Mathieu Poirier, Suzuki K Poulose, Kan Liang, Raul Silvera,
Athira Rajeev, Ravi Bangoria, Florian Fischer, Rob Herring,
Xing Zhengjun, Sean Christopherson, Chengdong Li, Denis Nikitin,
Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
On Tue, Mar 28, 2023 at 10:12 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 28/03/23 19:14, Ian Rogers wrote:
> > On Tue, Mar 28, 2023 at 6:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>
> >> On 28/03/23 04:40, Ian Rogers wrote:
> >>> When fuzzing something like parse-events, having the main function in
> >>> perf.c alongside global variables like input_name means that
> >>> input_name must be redeclared with the fuzzer function's
> >>> main. However, as the fuzzer is using the tools/perf code as a library
> >>> this causes backward linking reference that the linker may warn
> >>> about. Reorganize perf.c and perf.h to avoid potential backward
> >>> references, or so that the declaration/definition locations are more
> >>> consistent.
> >>>
> >>
> >> Seems like it could be a pain to maintain.
> >>
> >> Did you consider just adding:
> >>
> >> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> >> index 82bbe0ca858b..a75dd47d68ee 100644
> >> --- a/tools/perf/perf.c
> >> +++ b/tools/perf/perf.c
> >> @@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
> >> return veprintf(level, verbose, fmt, ap);
> >> }
> >>
> >> +#ifndef CUSTOM_MAIN
> >> int main(int argc, const char **argv)
> >> {
> >> int err;
> >> @@ -576,3 +577,4 @@ int main(int argc, const char **argv)
> >> out:
> >> return 1;
> >> }
> >> +#endif
> >>
> >
> > It's possible. Would need to make the static functions not warn about
> > being declared and not used. I still think that just aligning
> > definitions and declarations yields the most expected code and will
> > lead to fewer problems in the long run.
>
> Making perf source dependent on an unknown derivative makes
> things more complicated.
>
> If you are not going to contribute it to perf, then a
> suggestion is along the lines of the following:
There's not an issue with contribution, most of the fuzzers are very
simple. For example, here is the coresight fuzzer we run:
```
#include <fuzzer/FuzzedDataProvider.h>
extern "C" {
#include "tools/perf/util/cs-etm-decoder/cs-etm-decoder.h"
}
static void null_packet_dump(const char *) {}
extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
FuzzedDataProvider fdp(data, size);
int num_cpu = fdp.ConsumeIntegralInRange(1, 1024);
struct cs_etm_decoder_params d_params;
std::unique_ptr<struct cs_etm_trace_params[]>
t_params(new cs_etm_trace_params[num_cpu]);
for(int i=0; i < num_cpu; ++i) {
t_params[i].protocol =
fdp.ConsumeIntegralInRange(static_cast<int>(CS_ETM_PROTO_ETMV3)
,
static_cast<int>(CS_ETM_PROTO_PTM));
fdp.ConsumeData(&t_params[i].etmv4, sizeof(struct cs_etmv4_trace_params));
}
d_params.packet_printer = null_packet_dump;
d_params.operation = CS_ETM_OPERATION_DECODE;
d_params.data = NULL;
d_params.formatted = true;
d_params.fsyncs = false;
d_params.hsyncs = false;
d_params.frame_aligned = true;
std::unique_ptr<struct cs_etm_decoder, decltype(&cs_etm_decoder__free)>
decoder(cs_etm_decoder__new(num_cpu, &d_params, t_params.get()),
cs_etm_decoder__free);
if (decoder == nullptr)
return 0;
do {
size_t consumed;
uint8_t buf[1024];
size_t len = fdp.ConsumeData(buf, sizeof(buf));
int ret = cs_etm_decoder__process_data_block(
decoder.get(), 0, buf, len, &consumed);
if (ret)
return 0;
} while (fdp.remaining_bytes() > 0);
return 0;
}
```
Most of the code is boiler plate around a single function call. The
issues are build support, where to put the fuzzer generated test
corpus, the code is C++, etc.
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 82bbe0ca858b..6a7fe1534664 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -456,7 +456,18 @@ static int libperf_print(enum libperf_print_level level,
> return veprintf(level, verbose, fmt, ap);
> }
>
> $ git diff
> +#ifdef CUSTOM_MAIN
> +int main(void)
> +{
> + printf("This is not perf\n");
> + return 0;
> +}
> +
> +int perf_main(int argc, const char **argv);
> +int perf_main(int argc, const char **argv)
> +#else
> int main(int argc, const char **argv)
> +#endif
> {
> int err;
> const char *cmd;
> $ make EXTRA_CFLAGS="-DCUSTOM_MAIN" NO_BPF_SKEL=1 -C tools/perf >/dev/null
> Warning: Kernel ABI header at 'tools/include/uapi/linux/in.h' differs from latest version at 'include/uapi/linux/in.h'
> Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
> Warning: Kernel ABI header at 'tools/arch/arm64/include/uapi/asm/perf_regs.h' differs from latest version at 'arch/arm64/include/uapi/asm/perf_regs.h'
> Warning: Kernel ABI header at 'tools/include/linux/coresight-pmu.h' differs from latest version at 'include/linux/coresight-pmu.h'
> Makefile.config:587: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-dev
> Makefile.config:805: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
> Makefile.config:1046: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
> Makefile.config:1075: No alternatives command found, you need to set JDIR= to point to the root of your Java directory
> Makefile.config:1137: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
> $ tools/perf/perf version
> This is not perf
>
Sure, I'm aware of how the #ifdef would function. What I'm saying is
that even with a CUSTOM_MAIN ifdef, which wouldn't be necessary with
my patches as you can drop the object file, the refactoring in the
patches still makes sense.
perf ui: Move window resize signal functions
It isn't clear why these UI functions should be in perf.c given window
resizing is a UI issue.
perf usage: Move usage strings
The usage strings were in builtin.h and not perf.h, so the
declaration/definition didn't align. util.h declares usage(), although
the definition is in usage.c, the variables are moved to match this.
perf header: Move perf_version_string declaration
perf_version_string is defined in header.c and so declaring it in
header.h rather than perf.h is consistent.
perf version: Use regular verbose flag
Just remove an unnecessary global by repurposing the existing global
for the job.
perf util: Move input_name to util
There's no reference to input_name in perf.c and so util.c is as
sensible a location. Having this be a global is something of an
encapsulation fail, but cleaning that up wasn't the point of these
patches. The variable is used in util, so reaching up a directory to
get it feels somewhat unnatural.
perf util: Move perf_guest/host declarations
As with input_name.
Thanks,
Ian
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v1 0/6] Simplify linking against tools/perf code
2023-03-28 17:42 ` Ian Rogers
@ 2023-03-30 5:39 ` Adrian Hunter
0 siblings, 0 replies; 12+ messages in thread
From: Adrian Hunter @ 2023-03-30 5:39 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
John Garry, Will Deacon, James Clark, Mike Leach, Leo Yan,
Mathieu Poirier, Suzuki K Poulose, Kan Liang, Raul Silvera,
Athira Rajeev, Ravi Bangoria, Florian Fischer, Rob Herring,
Xing Zhengjun, Sean Christopherson, Chengdong Li, Denis Nikitin,
Martin Liška, linux-perf-users, linux-kernel,
linux-arm-kernel, coresight
On 28/03/23 20:42, Ian Rogers wrote:
> On Tue, Mar 28, 2023 at 10:12 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 28/03/23 19:14, Ian Rogers wrote:
>>> On Tue, Mar 28, 2023 at 6:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>
>>>> On 28/03/23 04:40, Ian Rogers wrote:
>>>>> When fuzzing something like parse-events, having the main function in
>>>>> perf.c alongside global variables like input_name means that
>>>>> input_name must be redeclared with the fuzzer function's
>>>>> main. However, as the fuzzer is using the tools/perf code as a library
>>>>> this causes backward linking reference that the linker may warn
>>>>> about. Reorganize perf.c and perf.h to avoid potential backward
>>>>> references, or so that the declaration/definition locations are more
>>>>> consistent.
>>>>>
>>>>
>>>> Seems like it could be a pain to maintain.
>>>>
>>>> Did you consider just adding:
>>>>
>>>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>>>> index 82bbe0ca858b..a75dd47d68ee 100644
>>>> --- a/tools/perf/perf.c
>>>> +++ b/tools/perf/perf.c
>>>> @@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
>>>> return veprintf(level, verbose, fmt, ap);
>>>> }
>>>>
>>>> +#ifndef CUSTOM_MAIN
>>>> int main(int argc, const char **argv)
>>>> {
>>>> int err;
>>>> @@ -576,3 +577,4 @@ int main(int argc, const char **argv)
>>>> out:
>>>> return 1;
>>>> }
>>>> +#endif
>>>>
>>>
>>> It's possible. Would need to make the static functions not warn about
>>> being declared and not used. I still think that just aligning
>>> definitions and declarations yields the most expected code and will
>>> lead to fewer problems in the long run.
>>
>> Making perf source dependent on an unknown derivative makes
>> things more complicated.
>>
>> If you are not going to contribute it to perf, then a
>> suggestion is along the lines of the following:
>
> There's not an issue with contribution, most of the fuzzers are very
> simple. For example, here is the coresight fuzzer we run:
>
> ```
> #include <fuzzer/FuzzedDataProvider.h>
>
> extern "C" {
> #include "tools/perf/util/cs-etm-decoder/cs-etm-decoder.h"
> }
>
> static void null_packet_dump(const char *) {}
>
> extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
> FuzzedDataProvider fdp(data, size);
> int num_cpu = fdp.ConsumeIntegralInRange(1, 1024);
> struct cs_etm_decoder_params d_params;
> std::unique_ptr<struct cs_etm_trace_params[]>
> t_params(new cs_etm_trace_params[num_cpu]);
>
> for(int i=0; i < num_cpu; ++i) {
> t_params[i].protocol =
> fdp.ConsumeIntegralInRange(static_cast<int>(CS_ETM_PROTO_ETMV3)
> ,
>
> static_cast<int>(CS_ETM_PROTO_PTM));
> fdp.ConsumeData(&t_params[i].etmv4, sizeof(struct cs_etmv4_trace_params));
> }
>
> d_params.packet_printer = null_packet_dump;
> d_params.operation = CS_ETM_OPERATION_DECODE;
> d_params.data = NULL;
> d_params.formatted = true;
> d_params.fsyncs = false;
> d_params.hsyncs = false;
> d_params.frame_aligned = true;
>
> std::unique_ptr<struct cs_etm_decoder, decltype(&cs_etm_decoder__free)>
> decoder(cs_etm_decoder__new(num_cpu, &d_params, t_params.get()),
> cs_etm_decoder__free);
>
> if (decoder == nullptr)
> return 0;
>
> do {
> size_t consumed;
> uint8_t buf[1024];
> size_t len = fdp.ConsumeData(buf, sizeof(buf));
>
> int ret = cs_etm_decoder__process_data_block(
> decoder.get(), 0, buf, len, &consumed);
> if (ret)
> return 0;
>
> } while (fdp.remaining_bytes() > 0);
>
> return 0;
> }
> ```
> Most of the code is boiler plate around a single function call. The
> issues are build support, where to put the fuzzer generated test
> corpus, the code is C++, etc.
>
>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> index 82bbe0ca858b..6a7fe1534664 100644
>> --- a/tools/perf/perf.c
>> +++ b/tools/perf/perf.c
>> @@ -456,7 +456,18 @@ static int libperf_print(enum libperf_print_level level,
>> return veprintf(level, verbose, fmt, ap);
>> }
>>
>> $ git diff
>> +#ifdef CUSTOM_MAIN
>> +int main(void)
>> +{
>> + printf("This is not perf\n");
>> + return 0;
>> +}
>> +
>> +int perf_main(int argc, const char **argv);
>> +int perf_main(int argc, const char **argv)
>> +#else
>> int main(int argc, const char **argv)
>> +#endif
>> {
>> int err;
>> const char *cmd;
>> $ make EXTRA_CFLAGS="-DCUSTOM_MAIN" NO_BPF_SKEL=1 -C tools/perf >/dev/null
>> Warning: Kernel ABI header at 'tools/include/uapi/linux/in.h' differs from latest version at 'include/uapi/linux/in.h'
>> Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
>> Warning: Kernel ABI header at 'tools/arch/arm64/include/uapi/asm/perf_regs.h' differs from latest version at 'arch/arm64/include/uapi/asm/perf_regs.h'
>> Warning: Kernel ABI header at 'tools/include/linux/coresight-pmu.h' differs from latest version at 'include/linux/coresight-pmu.h'
>> Makefile.config:587: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-dev
>> Makefile.config:805: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
>> Makefile.config:1046: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
>> Makefile.config:1075: No alternatives command found, you need to set JDIR= to point to the root of your Java directory
>> Makefile.config:1137: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
>> $ tools/perf/perf version
>> This is not perf
>>
>
> Sure, I'm aware of how the #ifdef would function. What I'm saying is
> that even with a CUSTOM_MAIN ifdef, which wouldn't be necessary with
> my patches as you can drop the object file, the refactoring in the
> patches still makes sense.
I am just going off what your commit messages say
>
> perf ui: Move window resize signal functions
>
> It isn't clear why these UI functions should be in perf.c given window
> resizing is a UI issue.
>
> perf usage: Move usage strings
>
> The usage strings were in builtin.h and not perf.h, so the
> declaration/definition didn't align. util.h declares usage(), although
> the definition is in usage.c, the variables are moved to match this.
>
> perf header: Move perf_version_string declaration
>
> perf_version_string is defined in header.c and so declaring it in
> header.h rather than perf.h is consistent.
>
> perf version: Use regular verbose flag
>
> Just remove an unnecessary global by repurposing the existing global
> for the job.
>
> perf util: Move input_name to util
>
> There's no reference to input_name in perf.c and so util.c is as
> sensible a location. Having this be a global is something of an
> encapsulation fail, but cleaning that up wasn't the point of these
> patches. The variable is used in util, so reaching up a directory to
> get it feels somewhat unnatural.
>
> perf util: Move perf_guest/host declarations
>
> As with input_name.
>
> Thanks,
> Ian
^ permalink raw reply [flat|nested] 12+ messages in thread