* [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
@ 2024-06-01 12:59 Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 01/10] tools lib: adopt str_has_suffix() from bpftool/gen.c Chaitanya S Prakash
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users
Cc: anshuman.khandual, james.clark, Chaitanya S Prakash,
Josh Poimboeuf, Peter Zijlstra, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
Chenyuan Mi, Masami Hiramatsu, Ravi Bangoria,
Ahelenia Ziemiańska, Colin Ian King, Changbin Du, Kan Liang,
Athira Rajeev, Tiezhu Yang, Alexey Dobriyan, Georg Müller,
Liam Howlett, bpf, coresight, linux-arm-kernel, linux-kernel
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Perf treated all files beginning with "/tmp/perf-" as a map file despite
them always ending in ".map", this caused the test "perf probe of
function from different CU" to fail when Perf was built with NO_DWARF=1.
As the file was parsed as a map file, the probe...--funcs command output
garbage values instead of listing the functions in the binary. After
fixing the issue an additional check to test the output of the
probe...--funcs command has been added.
Additionally, various functions within the codebase have been refactored
and restructured. The definition of str_has_suffix() has been adopted
from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
attempt to make the function more generic. The implementation has been
retained but the return values have been modified to resemble that of
str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
failure. In light of the new addition, "ends_with()", a locally defined
function used for checking if a string had a given suffix has been
deleted and str_has_suffix() has replaced its usage. A call to
strtailcmp() has also been replaced as str_has_suffix() seemed more
suited for that particular use case.
Finally str_has_prefix() is adopted from the kernel and is added to
tools/lib/string.c, following which strstarts() is deleted and its use
has been replaced with str_has_prefix().
This patch series has been tested on 6.10-rc1 mainline kernel, both on
arm64 and x86 platforms.
Changes in V3:
- Patch adding configs required by "perf probe of function from different
CU" was originally part of the series but has been merged in 6.10-rc1.
https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
- Restructure patches according to the maintainer trees.
- Add explanation for why '| grep "foo"' is used.
- Fix build errors for when perf is built with LLVM=1.
Changes in V2:
https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
- Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
- Delete ends_with() and replace its usage with str_has_suffix()
- Replace an instance of strtailcmp() with str_has_suffix()
- Delete strstarts() from tools/include/linux/string.h and replace its
usage with str_has_prefix()
Changes in V1:
https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Will Deacon <will@kernel.org>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Georg Müller <georgmueller@gmx.net>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: bpf@vger.kernel.org
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-perf-users@vger.kernel.org
Chaitanya S Prakash (10):
tools lib: adopt str_has_suffix() from bpftool/gen.c
perf util: Delete ends_with() and replace its use with
str_has_suffix()
perf util: Replace an instance of strtailcmp() by str_has_suffix()
tools lib: Adopt str_has_prefix() from kernel
libsubcmd: Replace strstarts() usage with str_has_prefix()
objtool: Replace strstarts() usage with str_has_prefix()
perf tools: Replace strstarts() usage with str_has_prefix()
tools lib: Remove strstarts() as all its usecases have been replaced
by str_has_prefix()
perf tools: Only treat files as map files when they have the extension
.map
perf test: Check output of the probe ... --funcs command
tools/include/linux/string.h | 12 ++---
tools/lib/string.c | 48 +++++++++++++++++++
tools/lib/subcmd/help.c | 2 +-
tools/lib/subcmd/parse-options.c | 18 +++----
tools/objtool/check.c | 2 +-
tools/perf/arch/arm/util/pmu.c | 4 +-
tools/perf/arch/x86/annotate/instructions.c | 14 +++---
tools/perf/arch/x86/util/env.c | 2 +-
tools/perf/builtin-c2c.c | 4 +-
tools/perf/builtin-config.c | 2 +-
tools/perf/builtin-daemon.c | 2 +-
tools/perf/builtin-ftrace.c | 2 +-
tools/perf/builtin-help.c | 6 +--
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-kvm.c | 14 +++---
tools/perf/builtin-kwork.c | 10 ++--
tools/perf/builtin-lock.c | 6 +--
tools/perf/builtin-mem.c | 4 +-
tools/perf/builtin-sched.c | 6 +--
tools/perf/builtin-script.c | 30 ++++--------
tools/perf/builtin-stat.c | 4 +-
tools/perf/builtin-timechart.c | 2 +-
tools/perf/builtin-trace.c | 6 +--
tools/perf/perf.c | 12 ++---
.../shell/test_uprobe_from_different_cu.sh | 2 +-
tools/perf/tests/symbols.c | 2 +-
tools/perf/ui/browser.c | 2 +-
tools/perf/ui/browsers/scripts.c | 2 +-
tools/perf/ui/stdio/hist.c | 2 +-
tools/perf/util/amd-sample-raw.c | 4 +-
tools/perf/util/annotate.c | 2 +-
tools/perf/util/callchain.c | 2 +-
tools/perf/util/config.c | 12 ++---
tools/perf/util/map.c | 8 ++--
tools/perf/util/pmus.c | 2 +-
tools/perf/util/probe-event.c | 2 +-
tools/perf/util/sample-raw.c | 2 +-
tools/perf/util/symbol-elf.c | 4 +-
tools/perf/util/symbol.c | 3 +-
39 files changed, 148 insertions(+), 117 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V3 01/10] tools lib: adopt str_has_suffix() from bpftool/gen.c
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 02/10] perf util: Delete ends_with() and replace its use with str_has_suffix() Chaitanya S Prakash
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Adopt the definition of str_has_suffix() from tools/bpf/bpftool/gen.c
and add it to tools/lib/string.c The function is modified to return
strlen(suffix) on success and 0 on failure.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/include/linux/string.h | 2 ++
tools/lib/string.c | 26 ++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index db5c99318c79..e43c8f78d63c 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -42,6 +42,8 @@ static inline bool strstarts(const char *str, const char *prefix)
return strncmp(str, prefix, strlen(prefix)) == 0;
}
+size_t str_has_suffix(const char *str, const char *suffix);
+
extern char * __must_check skip_spaces(const char *);
extern char *strim(char *);
diff --git a/tools/lib/string.c b/tools/lib/string.c
index 8b6892f959ab..5be69588dbe8 100644
--- a/tools/lib/string.c
+++ b/tools/lib/string.c
@@ -226,3 +226,29 @@ void *memchr_inv(const void *start, int c, size_t bytes)
return check_bytes8(start, value, bytes % 8);
}
+
+/**
+ * str_has_suffix - Test if a string has a given suffix
+ * @str: The string to be tested
+ * @suffix: The string to see if @str ends with
+ *
+ * Returns:
+ * * strlen(@suffix) if @str ends with @suffix
+ * * 0 if @str does not end with @suffix
+ */
+size_t str_has_suffix(const char *str, const char *suffix)
+{
+ size_t i;
+ size_t str_len = strlen(str);
+ size_t suffix_len = strlen(suffix);
+
+ if (str_len < suffix_len)
+ return 0;
+
+ for (i = 0; i < suffix_len ; i++) {
+ if (str[str_len - i - 1] != suffix[suffix_len - i - 1])
+ return 0;
+ }
+
+ return suffix_len;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 02/10] perf util: Delete ends_with() and replace its use with str_has_suffix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 01/10] tools lib: adopt str_has_suffix() from bpftool/gen.c Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 03/10] perf util: Replace an instance of strtailcmp() by str_has_suffix() Chaitanya S Prakash
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
As str_has_suffix() has been introduced in tools/lib/string.c having
another locally defined function ends_with() to check if a string has a
given suffix is redundant. Delete ends_with() and replace its use with
str_has_suffix(), and update the error checks accordingly.
The return type of ends_with() is const char*, whereas that of
str_has_suffix() is size_t. get_script_root() has been modified
accordingly to accommodate this change.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/perf/builtin-script.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index c16224b1fef3..ef622d71195f 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3301,20 +3301,6 @@ static struct script_desc *script_desc__findnew(const char *name)
return s;
}
-static const char *ends_with(const char *str, const char *suffix)
-{
- size_t suffix_len = strlen(suffix);
- const char *p = str;
-
- if (strlen(str) > suffix_len) {
- p = str + strlen(str) - suffix_len;
- if (!strncmp(p, suffix, suffix_len))
- return p;
- }
-
- return NULL;
-}
-
static int read_script_info(struct script_desc *desc, const char *filename)
{
char line[BUFSIZ], *p;
@@ -3358,19 +3344,21 @@ static int read_script_info(struct script_desc *desc, const char *filename)
static char *get_script_root(struct dirent *script_dirent, const char *suffix)
{
- char *script_root, *str;
+ char *script_root;
+ size_t script_len, suffix_len;
script_root = strdup(script_dirent->d_name);
if (!script_root)
return NULL;
- str = (char *)ends_with(script_root, suffix);
- if (!str) {
+ suffix_len = str_has_suffix(script_root, suffix);
+ if (!suffix_len) {
free(script_root);
return NULL;
}
- *str = '\0';
+ script_len = strlen(script_root);
+ script_root[script_len - suffix_len] = '\0';
return script_root;
}
@@ -3646,7 +3634,7 @@ static char *get_script_path(const char *script_root, const char *suffix)
static bool is_top_script(const char *script_path)
{
- return ends_with(script_path, "top") != NULL;
+ return str_has_suffix(script_path, "top");
}
static int has_required_arg(char *script_path)
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 03/10] perf util: Replace an instance of strtailcmp() by str_has_suffix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 01/10] tools lib: adopt str_has_suffix() from bpftool/gen.c Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 02/10] perf util: Delete ends_with() and replace its use with str_has_suffix() Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 04/10] tools lib: Adopt str_has_prefix() from kernel Chaitanya S Prakash
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
One instance of strtailcmp() that is identical to the use case of
str_has_suffix() has been replaced. As the other calls to strtailcmp()
do not seem equivalent to the str_has_suffix() use case, they are
retained as is.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/perf/util/probe-event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a17c9b8a7a79..72305ffa5b6c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -564,7 +564,7 @@ static struct debuginfo *open_debuginfo(const char *module, struct nsinfo *nsi,
ret = debuginfo__new(path);
if (!ret && !silent) {
pr_warning("The %s file has no debug information.\n", path);
- if (!module || !strtailcmp(path, ".ko"))
+ if (!module || str_has_suffix(path, ".ko"))
pr_warning("Rebuild with CONFIG_DEBUG_INFO=y, ");
else
pr_warning("Rebuild with -g, ");
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 04/10] tools lib: Adopt str_has_prefix() from kernel
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (2 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 03/10] perf util: Replace an instance of strtailcmp() by str_has_suffix() Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 05/10] libsubcmd: Replace strstarts() usage with str_has_prefix() Chaitanya S Prakash
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Copy str_has_prefix() from include/linux/string.h to
tools/include/linux/string.h with the intention to replace strstarts().
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/include/linux/string.h | 2 ++
tools/lib/string.c | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index e43c8f78d63c..e02292fa1fab 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -44,6 +44,8 @@ static inline bool strstarts(const char *str, const char *prefix)
size_t str_has_suffix(const char *str, const char *suffix);
+size_t str_has_prefix(const char *str, const char *prefix);
+
extern char * __must_check skip_spaces(const char *);
extern char *strim(char *);
diff --git a/tools/lib/string.c b/tools/lib/string.c
index 5be69588dbe8..243d79589b06 100644
--- a/tools/lib/string.c
+++ b/tools/lib/string.c
@@ -252,3 +252,25 @@ size_t str_has_suffix(const char *str, const char *suffix)
return suffix_len;
}
+
+/**
+ * str_has_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * A common way to test a prefix of a string is to do:
+ * strncmp(str, prefix, sizeof(prefix) - 1)
+ *
+ * But this can lead to bugs due to typos, or if prefix is a pointer
+ * and not a constant. Instead use str_has_prefix().
+ *
+ * Returns:
+ * * strlen(@prefix) if @str starts with @prefix
+ * * 0 if @str does not start with @prefix
+ */
+__always_inline size_t str_has_prefix(const char *str, const char *prefix)
+{
+ size_t len = strlen(prefix);
+
+ return strncmp(str, prefix, len) == 0 ? len : 0;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 05/10] libsubcmd: Replace strstarts() usage with str_has_prefix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (3 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 04/10] tools lib: Adopt str_has_prefix() from kernel Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 06/10] objtool: " Chaitanya S Prakash
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Calls to strstarts() are replaced with str_has_prefix() to unify the
string comparison functions between tools/ and the kernel.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/lib/subcmd/help.c | 2 +-
tools/lib/subcmd/parse-options.c | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
index 8561b0f01a24..589ecdab9318 100644
--- a/tools/lib/subcmd/help.c
+++ b/tools/lib/subcmd/help.c
@@ -196,7 +196,7 @@ static void list_commands_in_dir(struct cmdnames *cmds,
while ((de = readdir(dir)) != NULL) {
int entlen;
- if (!strstarts(de->d_name, prefix))
+ if (!str_has_prefix(de->d_name, prefix))
continue;
astrcat(&buf, de->d_name);
diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index 4b60ec03b0bb..b3206d46e2b7 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -390,7 +390,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
return 0;
}
if (!rest) {
- if (strstarts(options->long_name, "no-")) {
+ if (str_has_prefix(options->long_name, "no-")) {
/*
* The long name itself starts with "no-", so
* accept the option without "no-" so that users
@@ -403,7 +403,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
goto match;
}
/* Abbreviated case */
- if (strstarts(options->long_name + 3, arg)) {
+ if (str_has_prefix(options->long_name + 3, arg)) {
flags |= OPT_UNSET;
goto is_abbreviated;
}
@@ -428,7 +428,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
continue;
}
/* negated and abbreviated very much? */
- if (strstarts("no-", arg)) {
+ if (str_has_prefix("no-", arg)) {
flags |= OPT_UNSET;
goto is_abbreviated;
}
@@ -438,7 +438,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
flags |= OPT_UNSET;
rest = skip_prefix(arg + 3, options->long_name);
/* abbreviated and negated? */
- if (!rest && strstarts(options->long_name, arg + 3))
+ if (!rest && str_has_prefix(options->long_name, arg + 3))
goto is_abbreviated;
if (!rest)
continue;
@@ -478,7 +478,7 @@ static void check_typos(const char *arg, const struct option *options)
if (strlen(arg) < 3)
return;
- if (strstarts(arg, "no-")) {
+ if (str_has_prefix(arg, "no-")) {
fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}
@@ -486,7 +486,7 @@ static void check_typos(const char *arg, const struct option *options)
for (; options->type != OPTION_END; options++) {
if (!options->long_name)
continue;
- if (strstarts(options->long_name, arg)) {
+ if (str_has_prefix(options->long_name, arg)) {
fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}
@@ -981,10 +981,10 @@ int parse_options_usage(const char * const *usagestr,
if (opts->long_name == NULL)
continue;
- if (strstarts(opts->long_name, optstr))
+ if (str_has_prefix(opts->long_name, optstr))
print_option_help(opts, 0);
- if (strstarts("no-", optstr) &&
- strstarts(opts->long_name, optstr + 3))
+ if (str_has_prefix("no-", optstr) &&
+ str_has_prefix(opts->long_name, optstr + 3))
print_option_help(opts, 0);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 06/10] objtool: Replace strstarts() usage with str_has_prefix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (4 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 05/10] libsubcmd: Replace strstarts() usage with str_has_prefix() Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 07/10] perf tools: " Chaitanya S Prakash
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Calls to strstarts() are replaced with str_has_prefix() in an attempt to
standardize string comparison functions between tools/ and the kernel.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/objtool/check.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0a33d9195b7a..1c5ffd44ab79 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2535,7 +2535,7 @@ static int classify_symbols(struct objtool_file *file)
struct symbol *func;
for_each_sym(file, func) {
- if (func->type == STT_NOTYPE && strstarts(func->name, ".L"))
+ if (func->type == STT_NOTYPE && str_has_prefix(func->name, ".L"))
func->local_label = true;
if (func->bind != STB_GLOBAL)
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 07/10] perf tools: Replace strstarts() usage with str_has_prefix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (5 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 06/10] objtool: " Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 08/10] tools lib: Remove strstarts() as all its usecases have been replaced by str_has_prefix() Chaitanya S Prakash
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
To ensure consistency in string comparison functions between tools/ and
the kernel, calls to strstarts() are replaced with str_has_prefix().
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/perf/arch/arm/util/pmu.c | 4 ++--
tools/perf/arch/x86/annotate/instructions.c | 14 +++++++-------
tools/perf/arch/x86/util/env.c | 2 +-
tools/perf/builtin-c2c.c | 4 ++--
tools/perf/builtin-config.c | 2 +-
tools/perf/builtin-daemon.c | 2 +-
tools/perf/builtin-ftrace.c | 2 +-
tools/perf/builtin-help.c | 6 +++---
tools/perf/builtin-kmem.c | 2 +-
tools/perf/builtin-kvm.c | 14 +++++++-------
tools/perf/builtin-kwork.c | 10 +++++-----
tools/perf/builtin-lock.c | 6 +++---
tools/perf/builtin-mem.c | 4 ++--
tools/perf/builtin-sched.c | 6 +++---
tools/perf/builtin-script.c | 4 ++--
tools/perf/builtin-stat.c | 4 ++--
tools/perf/builtin-timechart.c | 2 +-
tools/perf/builtin-trace.c | 6 +++---
tools/perf/perf.c | 12 ++++++------
tools/perf/tests/symbols.c | 2 +-
tools/perf/ui/browser.c | 2 +-
tools/perf/ui/browsers/scripts.c | 2 +-
tools/perf/ui/stdio/hist.c | 2 +-
tools/perf/util/amd-sample-raw.c | 4 ++--
tools/perf/util/annotate.c | 2 +-
tools/perf/util/callchain.c | 2 +-
tools/perf/util/config.c | 12 ++++++------
tools/perf/util/map.c | 8 ++++----
tools/perf/util/pmus.c | 2 +-
tools/perf/util/sample-raw.c | 2 +-
tools/perf/util/symbol-elf.c | 4 ++--
31 files changed, 75 insertions(+), 75 deletions(-)
diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
index 8b7cb68ba1a8..ed280732c6c6 100644
--- a/tools/perf/arch/arm/util/pmu.c
+++ b/tools/perf/arch/arm/util/pmu.c
@@ -23,13 +23,13 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused)
pmu->selectable = true;
pmu->perf_event_attr_init_default = cs_etm_get_default_config;
#if defined(__aarch64__)
- } else if (strstarts(pmu->name, ARM_SPE_PMU_NAME)) {
+ } else if (str_has_prefix(pmu->name, ARM_SPE_PMU_NAME)) {
pmu->selectable = true;
pmu->is_uncore = false;
pmu->perf_event_attr_init_default = arm_spe_pmu_default_config;
if (!strcmp(pmu->name, "arm_spe_0"))
pmu->mem_events = perf_mem_events_arm;
- } else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) {
+ } else if (str_has_prefix(pmu->name, HISI_PTT_PMU_NAME)) {
pmu->selectable = true;
#endif
}
diff --git a/tools/perf/arch/x86/annotate/instructions.c b/tools/perf/arch/x86/annotate/instructions.c
index 5cdf457f5cbe..05924ce76cc1 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -126,16 +126,16 @@ static bool amd__ins_is_fused(struct arch *arch, const char *ins1,
return false;
/* Family >= 15h supports cmp/test + branch fusion */
- if (arch->family >= 0x15 && (strstarts(ins1, "test") ||
- (strstarts(ins1, "cmp") && !strstr(ins1, "xchg")))) {
+ if (arch->family >= 0x15 && (str_has_prefix(ins1, "test") ||
+ (str_has_prefix(ins1, "cmp") && !strstr(ins1, "xchg")))) {
return true;
}
/* Family >= 19h supports some ALU + branch fusion */
- if (arch->family >= 0x19 && (strstarts(ins1, "add") ||
- strstarts(ins1, "sub") || strstarts(ins1, "and") ||
- strstarts(ins1, "inc") || strstarts(ins1, "dec") ||
- strstarts(ins1, "or") || strstarts(ins1, "xor"))) {
+ if (arch->family >= 0x19 && (str_has_prefix(ins1, "add") ||
+ str_has_prefix(ins1, "sub") || str_has_prefix(ins1, "and") ||
+ str_has_prefix(ins1, "inc") || str_has_prefix(ins1, "dec") ||
+ str_has_prefix(ins1, "or") || str_has_prefix(ins1, "xor"))) {
return true;
}
@@ -182,7 +182,7 @@ static int x86__cpuid_parse(struct arch *arch, char *cpuid)
if (ret == 3) {
arch->family = family;
arch->model = model;
- arch->ins_is_fused = strstarts(cpuid, "AuthenticAMD") ?
+ arch->ins_is_fused = str_has_prefix(cpuid, "AuthenticAMD") ?
amd__ins_is_fused :
intel__ins_is_fused;
return 0;
diff --git a/tools/perf/arch/x86/util/env.c b/tools/perf/arch/x86/util/env.c
index 3e537ffb1353..394de17d2530 100644
--- a/tools/perf/arch/x86/util/env.c
+++ b/tools/perf/arch/x86/util/env.c
@@ -12,7 +12,7 @@ bool x86__is_amd_cpu(void)
goto ret;
perf_env__cpuid(&env);
- is_amd = env.cpuid && strstarts(env.cpuid, "AuthenticAMD") ? 1 : -1;
+ is_amd = env.cpuid && str_has_prefix(env.cpuid, "AuthenticAMD") ? 1 : -1;
perf_env__exit(&env);
ret:
return is_amd >= 1 ? true : false;
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index c157bd31f2e5..4bce58ffeb58 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -3348,9 +3348,9 @@ int cmd_c2c(int argc, const char **argv)
if (!argc)
usage_with_options(c2c_usage, c2c_options);
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
return perf_c2c__record(argc, argv);
- } else if (strlen(argv[0]) > 2 && strstarts("report", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("report", argv[0])) {
return perf_c2c__report(argc, argv);
} else {
usage_with_options(c2c_usage, c2c_options);
diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 2e8363778935..1a19d40fb492 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -79,7 +79,7 @@ static int show_spec_config(struct perf_config_set *set, const char *var)
return -1;
perf_config_items__for_each_entry(&set->sections, section) {
- if (!strstarts(var, section->name))
+ if (!str_has_prefix(var, section->name))
continue;
perf_config_items__for_each_entry(§ion->items, item) {
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index de76bbc50bfb..175ba1921cad 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -216,7 +216,7 @@ static int server_config(const char *var, const char *value, void *cb)
{
struct daemon *daemon = cb;
- if (strstarts(var, "session-")) {
+ if (str_has_prefix(var, "session-")) {
return session_config(daemon, var, value);
} else if (!strcmp(var, "daemon.base") && !daemon->base_user) {
if (daemon->base && strcmp(daemon->base, value)) {
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index eb30c8eca488..485696c2bff7 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -954,7 +954,7 @@ static int perf_ftrace_config(const char *var, const char *value, void *cb)
{
struct perf_ftrace *ftrace = cb;
- if (!strstarts(var, "ftrace."))
+ if (!str_has_prefix(var, "ftrace."))
return 0;
if (strcmp(var, "ftrace.tracer"))
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index b2a368ae295a..f5e3f1cb6eaa 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -98,7 +98,7 @@ static int check_emacsclient_version(void)
*/
finish_command(&ec_process);
- if (!strstarts(buffer.buf, "emacsclient")) {
+ if (!str_has_prefix(buffer.buf, "emacsclient")) {
fprintf(stderr, "Failed to parse emacsclient version.\n");
goto out;
}
@@ -291,7 +291,7 @@ static int perf_help_config(const char *var, const char *value, void *cb)
add_man_viewer(value);
return 0;
}
- if (strstarts(var, "man."))
+ if (str_has_prefix(var, "man."))
return add_man_viewer_info(var, value);
return 0;
@@ -321,7 +321,7 @@ static const char *cmd_to_page(const char *perf_cmd)
if (!perf_cmd)
return "perf";
- else if (strstarts(perf_cmd, "perf"))
+ else if (str_has_prefix(perf_cmd, "perf"))
return perf_cmd;
return asprintf(&s, "perf-%s", perf_cmd) < 0 ? NULL : s;
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 6fd95be5032b..110e4900bc5f 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1991,7 +1991,7 @@ int cmd_kmem(int argc, const char **argv)
kmem_page = 1;
}
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
symbol__init(NULL);
return __cmd_record(argc, argv);
}
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 71165036e4ca..47a9cdf417b8 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -2037,10 +2037,10 @@ static int kvm_cmd_stat(const char *file_name, int argc, const char **argv)
goto perf_stat;
}
- if (strlen(argv[1]) > 2 && strstarts("record", argv[1]))
+ if (strlen(argv[1]) > 2 && str_has_prefix("record", argv[1]))
return kvm_events_record(&kvm, argc - 1, argv + 1);
- if (strlen(argv[1]) > 2 && strstarts("report", argv[1]))
+ if (strlen(argv[1]) > 2 && str_has_prefix("report", argv[1]))
return kvm_events_report(&kvm, argc - 1 , argv + 1);
#if defined(HAVE_TIMERFD_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
@@ -2170,18 +2170,18 @@ int cmd_kvm(int argc, const char **argv)
}
}
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0]))
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0]))
return __cmd_record(file_name, argc, argv);
- else if (strlen(argv[0]) > 2 && strstarts("report", argv[0]))
+ else if (strlen(argv[0]) > 2 && str_has_prefix("report", argv[0]))
return __cmd_report(file_name, argc, argv);
- else if (strlen(argv[0]) > 2 && strstarts("diff", argv[0]))
+ else if (strlen(argv[0]) > 2 && str_has_prefix("diff", argv[0]))
return cmd_diff(argc, argv);
else if (!strcmp(argv[0], "top"))
return cmd_top(argc, argv);
- else if (strlen(argv[0]) > 2 && strstarts("buildid-list", argv[0]))
+ else if (strlen(argv[0]) > 2 && str_has_prefix("buildid-list", argv[0]))
return __cmd_buildid_list(file_name, argc, argv);
#if defined(HAVE_KVM_STAT_SUPPORT) && defined(HAVE_LIBTRACEEVENT)
- else if (strlen(argv[0]) > 2 && strstarts("stat", argv[0]))
+ else if (strlen(argv[0]) > 2 && str_has_prefix("stat", argv[0]))
return kvm_cmd_stat(file_name, argc, argv);
#endif
else
diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index 56e3f3a5e03a..820172317d51 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -2470,10 +2470,10 @@ int cmd_kwork(int argc, const char **argv)
sort_dimension__add(&kwork, "id", &kwork.cmp_id);
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
setup_event_list(&kwork, kwork_options, kwork_usage);
return perf_kwork__record(&kwork, argc, argv);
- } else if (strlen(argv[0]) > 2 && strstarts("report", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("report", argv[0])) {
kwork.sort_order = default_report_sort_order;
if (argc > 1) {
argc = parse_options(argc, argv, report_options, report_usage, 0);
@@ -2484,7 +2484,7 @@ int cmd_kwork(int argc, const char **argv)
setup_sorting(&kwork, report_options, report_usage);
setup_event_list(&kwork, kwork_options, kwork_usage);
return perf_kwork__report(&kwork);
- } else if (strlen(argv[0]) > 2 && strstarts("latency", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("latency", argv[0])) {
kwork.sort_order = default_latency_sort_order;
if (argc > 1) {
argc = parse_options(argc, argv, latency_options, latency_usage, 0);
@@ -2495,7 +2495,7 @@ int cmd_kwork(int argc, const char **argv)
setup_sorting(&kwork, latency_options, latency_usage);
setup_event_list(&kwork, kwork_options, kwork_usage);
return perf_kwork__report(&kwork);
- } else if (strlen(argv[0]) > 2 && strstarts("timehist", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("timehist", argv[0])) {
if (argc > 1) {
argc = parse_options(argc, argv, timehist_options, timehist_usage, 0);
if (argc)
@@ -2504,7 +2504,7 @@ int cmd_kwork(int argc, const char **argv)
kwork.report = KWORK_REPORT_TIMEHIST;
setup_event_list(&kwork, kwork_options, kwork_usage);
return perf_kwork__timehist(&kwork);
- } else if (strlen(argv[0]) > 2 && strstarts("top", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("top", argv[0])) {
kwork.sort_order = default_top_sort_order;
if (argc > 1) {
argc = parse_options(argc, argv, top_options, top_usage, 0);
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 7007d26fe654..9e9b64875c6c 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2663,9 +2663,9 @@ int cmd_lock(int argc, const char **argv)
if (!argc)
usage_with_options(lock_usage, lock_options);
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
return __cmd_record(argc, argv);
- } else if (strlen(argv[0]) > 2 && strstarts("report", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("report", argv[0])) {
trace_handler = &report_lock_ops;
if (argc) {
argc = parse_options(argc, argv,
@@ -2687,7 +2687,7 @@ int cmd_lock(int argc, const char **argv)
/* recycling report_lock_ops */
trace_handler = &report_lock_ops;
rc = __cmd_report(true);
- } else if (strlen(argv[0]) > 2 && strstarts("contention", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("contention", argv[0])) {
trace_handler = &contention_lock_ops;
sort_key = "wait_total";
output_fields = "contended,wait_total,wait_max,avg_wait";
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 863fcd735dae..5abd25472734 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -510,9 +510,9 @@ int cmd_mem(int argc, const char **argv)
mem.input_name = "perf.data";
}
- if (strlen(argv[0]) > 2 && strstarts("record", argv[0]))
+ if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0]))
return __cmd_record(argc, argv, &mem);
- else if (strlen(argv[0]) > 2 && strstarts("report", argv[0]))
+ else if (strlen(argv[0]) > 2 && str_has_prefix("report", argv[0]))
return report_events(argc, argv, &mem);
else
usage_with_options(mem_usage, mem_options);
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 5977c49ae2c7..537aea6c0d55 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3642,9 +3642,9 @@ int cmd_sched(int argc, const char **argv)
*/
if (!strcmp(argv[0], "script")) {
return cmd_script(argc, argv);
- } else if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
return __cmd_record(argc, argv);
- } else if (strlen(argv[0]) > 2 && strstarts("latency", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("latency", argv[0])) {
sched.tp_handler = &lat_ops;
if (argc > 1) {
argc = parse_options(argc, argv, latency_options, latency_usage, 0);
@@ -3662,7 +3662,7 @@ int cmd_sched(int argc, const char **argv)
sched.tp_handler = &map_ops;
setup_sorting(&sched, latency_options, latency_usage);
return perf_sched__map(&sched);
- } else if (strlen(argv[0]) > 2 && strstarts("replay", argv[0])) {
+ } else if (strlen(argv[0]) > 2 && str_has_prefix("replay", argv[0])) {
sched.tp_handler = &replay_ops;
if (argc) {
argc = parse_options(argc, argv, replay_options, replay_usage, 0);
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index ef622d71195f..7f5a4a20deac 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -4099,13 +4099,13 @@ int cmd_script(int argc, const char **argv)
if (symbol__validate_sym_arguments())
return -1;
- if (argc > 1 && strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (argc > 1 && strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
if (!rec_script_path)
return cmd_record(argc, argv);
}
- if (argc > 1 && strlen(argv[0]) > 2 && strstarts("report", argv[0])) {
+ if (argc > 1 && strlen(argv[0]) > 2 && str_has_prefix("report", argv[0])) {
rep_script_path = get_script_path(argv[1], REPORT_SUFFIX);
if (!rep_script_path) {
fprintf(stderr,
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 35f79b48e8dc..822b93c498de 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2529,11 +2529,11 @@ int cmd_stat(int argc, const char **argv)
} else
stat_config.csv_sep = DEFAULT_SEPARATOR;
- if (argc && strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (argc && strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
argc = __cmd_record(argc, argv);
if (argc < 0)
return -1;
- } else if (argc && strlen(argv[0]) > 2 && strstarts("report", argv[0]))
+ } else if (argc && strlen(argv[0]) > 2 && str_has_prefix("report", argv[0]))
return __cmd_report(argc, argv);
interval = stat_config.interval;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 19d4542ea18a..eed17971a2d6 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -2014,7 +2014,7 @@ int cmd_timechart(int argc, const char **argv)
goto out;
}
- if (argc && strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
+ if (argc && strlen(argv[0]) > 2 && str_has_prefix("record", argv[0])) {
argc = parse_options(argc, argv, timechart_record_options,
timechart_record_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 51eca671c797..f07951d665a7 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3135,7 +3135,7 @@ static bool evlist__add_vfs_getname(struct evlist *evlist)
return false;
evlist__for_each_entry_safe(evlist, evsel, tmp) {
- if (!strstarts(evsel__name(evsel), "probe:vfs_getname"))
+ if (!str_has_prefix(evsel__name(evsel), "probe:vfs_getname"))
continue;
if (evsel__field(evsel, "pathname")) {
@@ -3601,7 +3601,7 @@ static int trace__set_filter_loop_pids(struct trace *trace)
break;
if (!strcmp(thread__comm_str(parent), "sshd") ||
- strstarts(thread__comm_str(parent), "gnome-terminal")) {
+ str_has_prefix(thread__comm_str(parent), "gnome-terminal")) {
pids[nr++] = thread__tid(parent);
break;
}
@@ -5009,7 +5009,7 @@ int cmd_trace(int argc, const char **argv)
evsel->handler = trace__sys_enter;
}
- if (strstarts(evsel__name(evsel), "syscalls:sys_exit_")) {
+ if (str_has_prefix(evsel__name(evsel), "syscalls:sys_exit_")) {
struct syscall_tp *sc;
init_augmented_syscall_tp:
if (evsel__init_augmented_syscall_tp(evsel, evsel))
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index bd3f80b5bb46..11e148970c43 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -104,7 +104,7 @@ struct pager_config {
static bool same_cmd_with_prefix(const char *var, struct pager_config *c,
const char *header)
{
- return (strstarts(var, header) && !strcmp(var + strlen(header), c->cmd));
+ return (str_has_prefix(var, header) && !strcmp(var + strlen(header), c->cmd));
}
static int pager_command_config(const char *var, const char *value, void *data)
@@ -233,7 +233,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
/*
* Check remaining flags.
*/
- if (strstarts(cmd, CMD_EXEC_PATH)) {
+ if (str_has_prefix(cmd, CMD_EXEC_PATH)) {
cmd += strlen(CMD_EXEC_PATH);
if (*cmd == '=')
set_argv_exec_path(cmd + 1);
@@ -270,7 +270,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
*envchanged = 1;
(*argv)++;
(*argc)--;
- } else if (strstarts(cmd, CMD_DEBUGFS_DIR)) {
+ } else if (str_has_prefix(cmd, CMD_DEBUGFS_DIR)) {
tracing_path_set(cmd + strlen(CMD_DEBUGFS_DIR));
fprintf(stderr, "dir: %s\n", tracing_path_mount());
if (envchanged)
@@ -497,7 +497,7 @@ int main(int argc, const char **argv)
* that contains a dash in its name. To handle this scenario, we just
* fall through and ignore the "xxxx" part of the command string.
*/
- if (strstarts(cmd, "perf-")) {
+ if (str_has_prefix(cmd, "perf-")) {
cmd += 5;
argv[0] = cmd;
handle_internal_command(argc, argv);
@@ -508,7 +508,7 @@ int main(int argc, const char **argv)
cmd -= 5;
argv[0] = cmd;
}
- if (strstarts(cmd, "trace")) {
+ if (str_has_prefix(cmd, "trace")) {
#ifndef HAVE_LIBTRACEEVENT
fprintf(stderr,
"trace command not available: missing libtraceevent devel package at build time.\n");
@@ -530,7 +530,7 @@ int main(int argc, const char **argv)
commit_pager_choice();
if (argc > 0) {
- if (strstarts(argv[0], "--"))
+ if (str_has_prefix(argv[0], "--"))
argv[0] += 2;
} else {
/* The user didn't specify a command; give them help */
diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c
index ee20a366f32f..67b5b84ccec4 100644
--- a/tools/perf/tests/symbols.c
+++ b/tools/perf/tests/symbols.c
@@ -145,7 +145,7 @@ static int subdivided_dso_cb(struct dso *dso, struct machine *machine __maybe_un
{
struct dso *text_dso = d;
- if (dso != text_dso && strstarts(dso__short_name(dso), dso__short_name(text_dso)))
+ if (dso != text_dso && str_has_prefix(dso__short_name(dso), dso__short_name(text_dso)))
if (test_dso(dso) != TEST_OK)
return -1;
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 19503e838738..aa5e2713b011 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -579,7 +579,7 @@ static int ui_browser__color_config(const char *var, const char *value,
int i;
/* same dir for all commands */
- if (!strstarts(var, "colors.") != 0)
+ if ((!str_has_prefix(var, "colors.")) != 0)
return 0;
for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index e437d7889de6..eef15f87f1dd 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -63,7 +63,7 @@ static int scripts_config(const char *var, const char *value, void *data)
{
struct script_config *c = data;
- if (!strstarts(var, "scripts."))
+ if (!str_has_prefix(var, "scripts."))
return -1;
if (c->index >= SCRIPT_MAX_NO)
return -1;
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index b849caace398..a3431a00103b 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -237,7 +237,7 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
* displayed twice.
*/
if (!i++ && field_order == NULL &&
- sort_order && strstarts(sort_order, "sym"))
+ sort_order && str_has_prefix(sort_order, "sym"))
continue;
if (!printed) {
diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
index 9d0ce88e90e4..f99167bf6a78 100644
--- a/tools/perf/util/amd-sample-raw.c
+++ b/tools/perf/util/amd-sample-raw.c
@@ -320,9 +320,9 @@ bool evlist__has_amd_ibs(struct evlist *evlist)
while (nr_pmu_mappings--) {
ret = sscanf(pmu_mapping, "%u:%9s", &type, name);
if (ret == 2) {
- if (strstarts(name, "ibs_op"))
+ if (str_has_prefix(name, "ibs_op"))
ibs_op_type = type;
- else if (strstarts(name, "ibs_fetch"))
+ else if (str_has_prefix(name, "ibs_fetch"))
ibs_fetch_type = type;
}
pmu_mapping += strlen(pmu_mapping) + 1 /* '\0' */;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 1451caf25e77..b6983301f75f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1874,7 +1874,7 @@ static int annotation__config(const char *var, const char *value, void *data)
{
struct annotation_options *opt = data;
- if (!strstarts(var, "annotate."))
+ if (!str_has_prefix(var, "annotate."))
return 0;
if (!strcmp(var, "annotate.offset_level")) {
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 1730b852a947..fb6ca1d6996c 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -321,7 +321,7 @@ int perf_callchain_config(const char *var, const char *value)
{
char *endptr;
- if (!strstarts(var, "call-graph."))
+ if (!str_has_prefix(var, "call-graph."))
return 0;
var += sizeof("call-graph.") - 1;
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 7a650de0db83..0e9437a4ac25 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -473,22 +473,22 @@ static int perf_stat_config(const char *var, const char *value)
int perf_default_config(const char *var, const char *value,
void *dummy __maybe_unused)
{
- if (strstarts(var, "core."))
+ if (str_has_prefix(var, "core."))
return perf_default_core_config(var, value);
- if (strstarts(var, "hist."))
+ if (str_has_prefix(var, "hist."))
return perf_hist_config(var, value);
- if (strstarts(var, "ui."))
+ if (str_has_prefix(var, "ui."))
return perf_ui_config(var, value);
- if (strstarts(var, "call-graph."))
+ if (str_has_prefix(var, "call-graph."))
return perf_callchain_config(var, value);
- if (strstarts(var, "buildid."))
+ if (str_has_prefix(var, "buildid."))
return perf_buildid_config(var, value);
- if (strstarts(var, "stat."))
+ if (str_has_prefix(var, "stat."))
return perf_stat_config(var, value);
/* Add other config variables here. */
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index e1d14936a60d..d4e18e81c804 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -18,8 +18,8 @@
static inline int is_android_lib(const char *filename)
{
- return strstarts(filename, "/data/app-lib/") ||
- strstarts(filename, "/system/lib/");
+ return str_has_prefix(filename, "/data/app-lib/") ||
+ str_has_prefix(filename, "/system/lib/");
}
static inline bool replace_android_lib(const char *filename, char *newfilename)
@@ -39,7 +39,7 @@ static inline bool replace_android_lib(const char *filename, char *newfilename)
app_abi_length = strlen(app_abi);
- if (strstarts(filename, "/data/app-lib/")) {
+ if (str_has_prefix(filename, "/data/app-lib/")) {
char *apk_path;
if (!app_abi_length)
@@ -63,7 +63,7 @@ static inline bool replace_android_lib(const char *filename, char *newfilename)
return true;
}
- if (strstarts(filename, "/system/lib/")) {
+ if (str_has_prefix(filename, "/system/lib/")) {
char *ndk, *app;
const char *arch;
int ndk_length, app_length;
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index b9b4c5eb5002..34dfb01d3b9a 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -47,7 +47,7 @@ int pmu_name_len_no_suffix(const char *str, unsigned long *num)
orig_len = len = strlen(str);
/* Non-uncore PMUs have their full length, for example, i915. */
- if (!strstarts(str, "uncore_"))
+ if (!str_has_prefix(str, "uncore_"))
return len;
/*
diff --git a/tools/perf/util/sample-raw.c b/tools/perf/util/sample-raw.c
index f3f6bd9d290e..e5fcca8b755f 100644
--- a/tools/perf/util/sample-raw.c
+++ b/tools/perf/util/sample-raw.c
@@ -19,7 +19,7 @@ void evlist__init_trace_event_sample_raw(struct evlist *evlist)
if (arch_pf && !strcmp("s390", arch_pf))
evlist->trace_event_sample_raw = evlist__s390_sample_raw;
else if (arch_pf && !strcmp("x86", arch_pf) &&
- cpuid && strstarts(cpuid, "AuthenticAMD") &&
+ cpuid && str_has_prefix(cpuid, "AuthenticAMD") &&
evlist__has_amd_ibs(evlist)) {
evlist->trace_event_sample_raw = evlist__amd_sample_raw;
}
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index e398abfd13a0..2cd8226a07f1 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1368,8 +1368,8 @@ static u64 max_text_section(Elf *elf, GElf_Ehdr *ehdr)
/* .init and .exit sections are not placed with .text */
sec_name = elf_strptr(elf, ehdr->e_shstrndx, shdr.sh_name);
if (!sec_name ||
- strstarts(sec_name, ".init") ||
- strstarts(sec_name, ".exit"))
+ str_has_prefix(sec_name, ".init") ||
+ str_has_prefix(sec_name, ".exit"))
break;
/* Must be next to previous, assumes .text is first */
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 08/10] tools lib: Remove strstarts() as all its usecases have been replaced by str_has_prefix()
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (6 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 07/10] perf tools: " Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map Chaitanya S Prakash
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
As both strstarts() and str_has_prefix() are relatively similar in terms
of functionality, it is redundant to keep both. Delete strstarts() from
tools/include/linux/string.h as all its use cases have been replaced by
str_has_prefix()
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/include/linux/string.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index e02292fa1fab..696e064e139b 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -32,16 +32,6 @@ char *str_error_r(int errnum, char *buf, size_t buflen);
char *strreplace(char *s, char old, char new);
-/**
- * strstarts - does @str start with @prefix?
- * @str: string to examine
- * @prefix: prefix to look for.
- */
-static inline bool strstarts(const char *str, const char *prefix)
-{
- return strncmp(str, prefix, strlen(prefix)) == 0;
-}
-
size_t str_has_suffix(const char *str, const char *suffix);
size_t str_has_prefix(const char *str, const char *prefix);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (7 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 08/10] tools lib: Remove strstarts() as all its usecases have been replaced by str_has_prefix() Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-17 13:52 ` James Clark
2024-06-01 12:59 ` [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command Chaitanya S Prakash
2024-07-10 7:39 ` [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
10 siblings, 1 reply; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Strengthen the check to locate symbol map files in the tool's /tmp
directory. As the existing check allows non map files named in
"/tmp/perf-" pattern, it leads to possible dwarf library corruption
when perf is built with NO_DWARF=1. The try_to_find_probe_trace_events()
function behaves differently when built in that manner. Extending the
current check condition using the str_has_suffix() function to not only
validate the pattern but also ensure the file is of ".map" type allows
test perf probe of function from different CU to pass when built with
NO_DWARF=1.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/perf/util/symbol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9e5940b5bc59..fbcc189edd8d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1799,7 +1799,8 @@ int dso__load(struct dso *dso, struct map *map)
const char *map_path = dso__long_name(dso);
mutex_lock(dso__lock(dso));
- perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0;
+ perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0 &&
+ str_has_prefix(dso__name(dso), ".map") != 0;
if (perfmap) {
if (dso__nsinfo(dso) &&
(dso__find_perf_map(newmapname, sizeof(newmapname),
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (8 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map Chaitanya S Prakash
@ 2024-06-01 12:59 ` Chaitanya S Prakash
2024-06-24 23:30 ` Namhyung Kim
2024-06-26 3:57 ` Namhyung Kim
2024-07-10 7:39 ` [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
10 siblings, 2 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-06-01 12:59 UTC (permalink / raw)
To: linux-perf-users; +Cc: anshuman.khandual, james.clark, Chaitanya S Prakash
From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Test "perf probe of function from different CU" only checks if the perf
command has failed and doesn't test the --funcs output. In the issue
reported in the previous commit, the garbage output of the --funcs
command was being ignored by the test when it could have been caught.
The script first makes use of --funcs option with the perf probe command
to check if the function "foo" exists in the testfile before adding a
probe to it in the next command. The output of probe...--funcs command
is redirected to stdout, therefore, add '| grep "foo"' to validate the
result.
Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
index 319f36ebb9a4..82bc774a078a 100755
--- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
+++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
@@ -77,7 +77,7 @@ gcc -g -Og -flto -c ${temp_dir}/testfile-foo.c -o ${temp_dir}/testfile-foo.o
gcc -g -Og -c ${temp_dir}/testfile-main.c -o ${temp_dir}/testfile-main.o
gcc -g -Og -o ${temp_dir}/testfile ${temp_dir}/testfile-foo.o ${temp_dir}/testfile-main.o
-perf probe -x ${temp_dir}/testfile --funcs foo
+perf probe -x ${temp_dir}/testfile --funcs foo | grep "foo"
perf probe -x ${temp_dir}/testfile foo
cleanup
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map
2024-06-01 12:59 ` [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map Chaitanya S Prakash
@ 2024-06-17 13:52 ` James Clark
2024-06-24 23:29 ` Namhyung Kim
0 siblings, 1 reply; 22+ messages in thread
From: James Clark @ 2024-06-17 13:52 UTC (permalink / raw)
To: Chaitanya S Prakash, linux-perf-users; +Cc: anshuman.khandual
On 01/06/2024 13:59, Chaitanya S Prakash wrote:
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>
> Strengthen the check to locate symbol map files in the tool's /tmp
> directory. As the existing check allows non map files named in
> "/tmp/perf-" pattern, it leads to possible dwarf library corruption
> when perf is built with NO_DWARF=1. The try_to_find_probe_trace_events()
> function behaves differently when built in that manner. Extending the
> current check condition using the str_has_suffix() function to not only
> validate the pattern but also ensure the file is of ".map" type allows
> test perf probe of function from different CU to pass when built with
> NO_DWARF=1.
>
> Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> ---
> tools/perf/util/symbol.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 9e5940b5bc59..fbcc189edd8d 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1799,7 +1799,8 @@ int dso__load(struct dso *dso, struct map *map)
> const char *map_path = dso__long_name(dso);
>
> mutex_lock(dso__lock(dso));
> - perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0;
> + perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0 &&
> + str_has_prefix(dso__name(dso), ".map") != 0;
Does this work? Shouldn't it be str_has_suffix()?
For reference, there's another fix for the same issue here:
https://lore.kernel.org/linux-perf-users/20240617130332.13427-1-atrajeev@linux.vnet.ibm.com/T/#m9b1e62f0cdbcd4caf019f2ad97d69b29df568522
> if (perfmap) {
> if (dso__nsinfo(dso) &&
> (dso__find_perf_map(newmapname, sizeof(newmapname),
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map
2024-06-17 13:52 ` James Clark
@ 2024-06-24 23:29 ` Namhyung Kim
0 siblings, 0 replies; 22+ messages in thread
From: Namhyung Kim @ 2024-06-24 23:29 UTC (permalink / raw)
To: James Clark; +Cc: Chaitanya S Prakash, linux-perf-users, anshuman.khandual
Hello,
On Mon, Jun 17, 2024 at 02:52:52PM +0100, James Clark wrote:
>
>
> On 01/06/2024 13:59, Chaitanya S Prakash wrote:
> > From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> >
> > Strengthen the check to locate symbol map files in the tool's /tmp
> > directory. As the existing check allows non map files named in
> > "/tmp/perf-" pattern, it leads to possible dwarf library corruption
> > when perf is built with NO_DWARF=1. The try_to_find_probe_trace_events()
> > function behaves differently when built in that manner. Extending the
> > current check condition using the str_has_suffix() function to not only
> > validate the pattern but also ensure the file is of ".map" type allows
> > test perf probe of function from different CU to pass when built with
> > NO_DWARF=1.
> >
> > Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> > ---
> > tools/perf/util/symbol.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 9e5940b5bc59..fbcc189edd8d 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1799,7 +1799,8 @@ int dso__load(struct dso *dso, struct map *map)
> > const char *map_path = dso__long_name(dso);
> >
> > mutex_lock(dso__lock(dso));
> > - perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0;
> > + perfmap = strncmp(dso__name(dso), "/tmp/perf-", 10) == 0 &&
> > + str_has_prefix(dso__name(dso), ".map") != 0;
>
> Does this work? Shouldn't it be str_has_suffix()?
Yep, I think it should be
perfmap = str_has_prefix(dso__name(dso), "/tmp/perf-") &&
str_has_suffix(dso__name(dso), ".map"));
>
> For reference, there's another fix for the same issue here:
> https://lore.kernel.org/linux-perf-users/20240617130332.13427-1-atrajeev@linux.vnet.ibm.com/T/#m9b1e62f0cdbcd4caf019f2ad97d69b29df568522
Right, it seems this patchset is posted earlier than the above. But I
prefer Athira's fix as it touches less codes basically and contains more
fixes.
Thanks,
Namhyung
>
> > if (perfmap) {
> > if (dso__nsinfo(dso) &&
> > (dso__find_perf_map(newmapname, sizeof(newmapname),
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command
2024-06-01 12:59 ` [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command Chaitanya S Prakash
@ 2024-06-24 23:30 ` Namhyung Kim
2024-06-25 12:40 ` James Clark
2024-06-26 3:57 ` Namhyung Kim
1 sibling, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2024-06-24 23:30 UTC (permalink / raw)
To: Chaitanya S Prakash; +Cc: linux-perf-users, anshuman.khandual, james.clark
On Sat, Jun 01, 2024 at 06:29:46PM +0530, Chaitanya S Prakash wrote:
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>
> Test "perf probe of function from different CU" only checks if the perf
> command has failed and doesn't test the --funcs output. In the issue
> reported in the previous commit, the garbage output of the --funcs
> command was being ignored by the test when it could have been caught.
>
> The script first makes use of --funcs option with the perf probe command
> to check if the function "foo" exists in the testfile before adding a
> probe to it in the next command. The output of probe...--funcs command
> is redirected to stdout, therefore, add '| grep "foo"' to validate the
> result.
Ok, I think this can be applied separately.
Thanks,
Namhyung
>
> Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> ---
> tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> index 319f36ebb9a4..82bc774a078a 100755
> --- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> +++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
> @@ -77,7 +77,7 @@ gcc -g -Og -flto -c ${temp_dir}/testfile-foo.c -o ${temp_dir}/testfile-foo.o
> gcc -g -Og -c ${temp_dir}/testfile-main.c -o ${temp_dir}/testfile-main.o
> gcc -g -Og -o ${temp_dir}/testfile ${temp_dir}/testfile-foo.o ${temp_dir}/testfile-main.o
>
> -perf probe -x ${temp_dir}/testfile --funcs foo
> +perf probe -x ${temp_dir}/testfile --funcs foo | grep "foo"
> perf probe -x ${temp_dir}/testfile foo
>
> cleanup
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command
2024-06-24 23:30 ` Namhyung Kim
@ 2024-06-25 12:40 ` James Clark
2024-06-25 19:08 ` Namhyung Kim
0 siblings, 1 reply; 22+ messages in thread
From: James Clark @ 2024-06-25 12:40 UTC (permalink / raw)
To: Namhyung Kim, Chaitanya S Prakash; +Cc: linux-perf-users, anshuman.khandual
On 25/06/2024 00:30, Namhyung Kim wrote:
> On Sat, Jun 01, 2024 at 06:29:46PM +0530, Chaitanya S Prakash wrote:
>> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>>
>> Test "perf probe of function from different CU" only checks if the perf
>> command has failed and doesn't test the --funcs output. In the issue
>> reported in the previous commit, the garbage output of the --funcs
>> command was being ignored by the test when it could have been caught.
>>
>> The script first makes use of --funcs option with the perf probe command
>> to check if the function "foo" exists in the testfile before adding a
>> probe to it in the next command. The output of probe...--funcs command
>> is redirected to stdout, therefore, add '| grep "foo"' to validate the
>> result.
>
> Ok, I think this can be applied separately.
>
Yeah I think it can. There was a question from Arnaldo about this one on
a previous version, but Chaitanya clarified about the stderr vs stdout
thing:
https://lore.kernel.org/linux-perf-users/7d6330bd-5b0a-49d2-83bf-ac1ca11996d0@arm.com/#t
> Thanks,
> Namhyung
>
>>
>> Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>> ---
>> tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
>> index 319f36ebb9a4..82bc774a078a 100755
>> --- a/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
>> +++ b/tools/perf/tests/shell/test_uprobe_from_different_cu.sh
>> @@ -77,7 +77,7 @@ gcc -g -Og -flto -c ${temp_dir}/testfile-foo.c -o ${temp_dir}/testfile-foo.o
>> gcc -g -Og -c ${temp_dir}/testfile-main.c -o ${temp_dir}/testfile-main.o
>> gcc -g -Og -o ${temp_dir}/testfile ${temp_dir}/testfile-foo.o ${temp_dir}/testfile-main.o
>>
>> -perf probe -x ${temp_dir}/testfile --funcs foo
>> +perf probe -x ${temp_dir}/testfile --funcs foo | grep "foo"
>> perf probe -x ${temp_dir}/testfile foo
>>
>> cleanup
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command
2024-06-25 12:40 ` James Clark
@ 2024-06-25 19:08 ` Namhyung Kim
0 siblings, 0 replies; 22+ messages in thread
From: Namhyung Kim @ 2024-06-25 19:08 UTC (permalink / raw)
To: James Clark; +Cc: Chaitanya S Prakash, linux-perf-users, anshuman.khandual
On Tue, Jun 25, 2024 at 01:40:57PM +0100, James Clark wrote:
>
>
> On 25/06/2024 00:30, Namhyung Kim wrote:
> > On Sat, Jun 01, 2024 at 06:29:46PM +0530, Chaitanya S Prakash wrote:
> >> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> >>
> >> Test "perf probe of function from different CU" only checks if the perf
> >> command has failed and doesn't test the --funcs output. In the issue
> >> reported in the previous commit, the garbage output of the --funcs
> >> command was being ignored by the test when it could have been caught.
> >>
> >> The script first makes use of --funcs option with the perf probe command
> >> to check if the function "foo" exists in the testfile before adding a
> >> probe to it in the next command. The output of probe...--funcs command
> >> is redirected to stdout, therefore, add '| grep "foo"' to validate the
> >> result.
> >
> > Ok, I think this can be applied separately.
> >
>
> Yeah I think it can. There was a question from Arnaldo about this one on
> a previous version, but Chaitanya clarified about the stderr vs stdout
> thing:
>
> https://lore.kernel.org/linux-perf-users/7d6330bd-5b0a-49d2-83bf-ac1ca11996d0@arm.com/#t
Thanks for the pointer. I've also checked show_available_funcs() -
which is called for -F/--funcs option - uses the plain printf().
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/probe-event.c?h=perf-tools-next#n3801
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command
2024-06-01 12:59 ` [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command Chaitanya S Prakash
2024-06-24 23:30 ` Namhyung Kim
@ 2024-06-26 3:57 ` Namhyung Kim
1 sibling, 0 replies; 22+ messages in thread
From: Namhyung Kim @ 2024-06-26 3:57 UTC (permalink / raw)
To: Chaitanya S Prakash; +Cc: linux-perf-users, anshuman.khandual, james.clark
On Sat, Jun 1, 2024 at 6:00 AM Chaitanya S Prakash
<ChaitanyaS.Prakash@arm.com> wrote:
>
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>
> Test "perf probe of function from different CU" only checks if the perf
> command has failed and doesn't test the --funcs output. In the issue
> reported in the previous commit, the garbage output of the --funcs
> command was being ignored by the test when it could have been caught.
>
> The script first makes use of --funcs option with the perf probe command
> to check if the function "foo" exists in the testfile before adding a
> probe to it in the next command. The output of probe...--funcs command
> is redirected to stdout, therefore, add '| grep "foo"' to validate the
> result.
Applied to perf-tools-next, thanks!
Best Regards,
Namhyung
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
` (9 preceding siblings ...)
2024-06-01 12:59 ` [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command Chaitanya S Prakash
@ 2024-07-10 7:39 ` Chaitanya S Prakash
2024-07-12 19:55 ` Namhyung Kim
2024-07-15 9:34 ` James Clark
10 siblings, 2 replies; 22+ messages in thread
From: Chaitanya S Prakash @ 2024-07-10 7:39 UTC (permalink / raw)
To: linux-perf-users, Peter Zijlstra, Arnaldo Carvalho de Melo,
Ingo Molnar
Cc: anshuman.khandual, james.clark, Josh Poimboeuf, Suzuki K Poulose,
Mike Leach, John Garry, Will Deacon, Leo Yan, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Chenyuan Mi, Masami Hiramatsu, Ravi Bangoria,
Ahelenia Ziemiańska, Colin Ian King, Changbin Du, Kan Liang,
Athira Rajeev, Tiezhu Yang, Alexey Dobriyan, Georg Müller,
Liam Howlett, bpf, coresight, linux-arm-kernel, linux-kernel
Gentle ping, are there any updates on the string clean up patch set?
On 6/1/24 18:29, Chaitanya S Prakash wrote:
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>
> Perf treated all files beginning with "/tmp/perf-" as a map file despite
> them always ending in ".map", this caused the test "perf probe of
> function from different CU" to fail when Perf was built with NO_DWARF=1.
> As the file was parsed as a map file, the probe...--funcs command output
> garbage values instead of listing the functions in the binary. After
> fixing the issue an additional check to test the output of the
> probe...--funcs command has been added.
>
> Additionally, various functions within the codebase have been refactored
> and restructured. The definition of str_has_suffix() has been adopted
> from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
> attempt to make the function more generic. The implementation has been
> retained but the return values have been modified to resemble that of
> str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
> failure. In light of the new addition, "ends_with()", a locally defined
> function used for checking if a string had a given suffix has been
> deleted and str_has_suffix() has replaced its usage. A call to
> strtailcmp() has also been replaced as str_has_suffix() seemed more
> suited for that particular use case.
>
> Finally str_has_prefix() is adopted from the kernel and is added to
> tools/lib/string.c, following which strstarts() is deleted and its use
> has been replaced with str_has_prefix().
>
> This patch series has been tested on 6.10-rc1 mainline kernel, both on
> arm64 and x86 platforms.
>
> Changes in V3:
>
> - Patch adding configs required by "perf probe of function from different
> CU" was originally part of the series but has been merged in 6.10-rc1.
> https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
>
> - Restructure patches according to the maintainer trees.
> - Add explanation for why '| grep "foo"' is used.
> - Fix build errors for when perf is built with LLVM=1.
>
> Changes in V2:
> https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
>
> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> - Delete ends_with() and replace its usage with str_has_suffix()
> - Replace an instance of strtailcmp() with str_has_suffix()
> - Delete strstarts() from tools/include/linux/string.h and replace its
> usage with str_has_prefix()
>
> Changes in V1:
> https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
>
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
> Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> Cc: Colin Ian King <colin.i.king@gmail.com>
> Cc: Changbin Du <changbin.du@huawei.com>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Georg Müller <georgmueller@gmx.net>
> Cc: Liam Howlett <liam.howlett@oracle.com>
> Cc: bpf@vger.kernel.org
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-perf-users@vger.kernel.org
>
> Chaitanya S Prakash (10):
> tools lib: adopt str_has_suffix() from bpftool/gen.c
> perf util: Delete ends_with() and replace its use with
> str_has_suffix()
> perf util: Replace an instance of strtailcmp() by str_has_suffix()
> tools lib: Adopt str_has_prefix() from kernel
> libsubcmd: Replace strstarts() usage with str_has_prefix()
> objtool: Replace strstarts() usage with str_has_prefix()
> perf tools: Replace strstarts() usage with str_has_prefix()
> tools lib: Remove strstarts() as all its usecases have been replaced
> by str_has_prefix()
> perf tools: Only treat files as map files when they have the extension
> .map
> perf test: Check output of the probe ... --funcs command
>
> tools/include/linux/string.h | 12 ++---
> tools/lib/string.c | 48 +++++++++++++++++++
> tools/lib/subcmd/help.c | 2 +-
> tools/lib/subcmd/parse-options.c | 18 +++----
> tools/objtool/check.c | 2 +-
> tools/perf/arch/arm/util/pmu.c | 4 +-
> tools/perf/arch/x86/annotate/instructions.c | 14 +++---
> tools/perf/arch/x86/util/env.c | 2 +-
> tools/perf/builtin-c2c.c | 4 +-
> tools/perf/builtin-config.c | 2 +-
> tools/perf/builtin-daemon.c | 2 +-
> tools/perf/builtin-ftrace.c | 2 +-
> tools/perf/builtin-help.c | 6 +--
> tools/perf/builtin-kmem.c | 2 +-
> tools/perf/builtin-kvm.c | 14 +++---
> tools/perf/builtin-kwork.c | 10 ++--
> tools/perf/builtin-lock.c | 6 +--
> tools/perf/builtin-mem.c | 4 +-
> tools/perf/builtin-sched.c | 6 +--
> tools/perf/builtin-script.c | 30 ++++--------
> tools/perf/builtin-stat.c | 4 +-
> tools/perf/builtin-timechart.c | 2 +-
> tools/perf/builtin-trace.c | 6 +--
> tools/perf/perf.c | 12 ++---
> .../shell/test_uprobe_from_different_cu.sh | 2 +-
> tools/perf/tests/symbols.c | 2 +-
> tools/perf/ui/browser.c | 2 +-
> tools/perf/ui/browsers/scripts.c | 2 +-
> tools/perf/ui/stdio/hist.c | 2 +-
> tools/perf/util/amd-sample-raw.c | 4 +-
> tools/perf/util/annotate.c | 2 +-
> tools/perf/util/callchain.c | 2 +-
> tools/perf/util/config.c | 12 ++---
> tools/perf/util/map.c | 8 ++--
> tools/perf/util/pmus.c | 2 +-
> tools/perf/util/probe-event.c | 2 +-
> tools/perf/util/sample-raw.c | 2 +-
> tools/perf/util/symbol-elf.c | 4 +-
> tools/perf/util/symbol.c | 3 +-
> 39 files changed, 148 insertions(+), 117 deletions(-)
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
2024-07-10 7:39 ` [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
@ 2024-07-12 19:55 ` Namhyung Kim
2024-07-15 9:34 ` James Clark
1 sibling, 0 replies; 22+ messages in thread
From: Namhyung Kim @ 2024-07-12 19:55 UTC (permalink / raw)
To: Chaitanya S Prakash
Cc: linux-perf-users, Peter Zijlstra, Arnaldo Carvalho de Melo,
Ingo Molnar, anshuman.khandual, james.clark, Josh Poimboeuf,
Suzuki K Poulose, Mike Leach, John Garry, Will Deacon, Leo Yan,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Chenyuan Mi, Masami Hiramatsu, Ravi Bangoria,
Ahelenia Ziemiańska, Colin Ian King, Changbin Du, Kan Liang,
Athira Rajeev, Tiezhu Yang, Alexey Dobriyan, Georg Müller,
Liam Howlett, bpf, coresight, linux-arm-kernel, linux-kernel
Hello,
On Wed, Jul 10, 2024 at 01:09:26PM +0530, Chaitanya S Prakash wrote:
> Gentle ping, are there any updates on the string clean up patch set?
Sorry, I don't see any specific needs to have them.
Thanks,
Namhyung
>
> On 6/1/24 18:29, Chaitanya S Prakash wrote:
> > From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> >
> > Perf treated all files beginning with "/tmp/perf-" as a map file despite
> > them always ending in ".map", this caused the test "perf probe of
> > function from different CU" to fail when Perf was built with NO_DWARF=1.
> > As the file was parsed as a map file, the probe...--funcs command output
> > garbage values instead of listing the functions in the binary. After
> > fixing the issue an additional check to test the output of the
> > probe...--funcs command has been added.
> >
> > Additionally, various functions within the codebase have been refactored
> > and restructured. The definition of str_has_suffix() has been adopted
> > from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
> > attempt to make the function more generic. The implementation has been
> > retained but the return values have been modified to resemble that of
> > str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
> > failure. In light of the new addition, "ends_with()", a locally defined
> > function used for checking if a string had a given suffix has been
> > deleted and str_has_suffix() has replaced its usage. A call to
> > strtailcmp() has also been replaced as str_has_suffix() seemed more
> > suited for that particular use case.
> >
> > Finally str_has_prefix() is adopted from the kernel and is added to
> > tools/lib/string.c, following which strstarts() is deleted and its use
> > has been replaced with str_has_prefix().
> >
> > This patch series has been tested on 6.10-rc1 mainline kernel, both on
> > arm64 and x86 platforms.
> >
> > Changes in V3:
> >
> > - Patch adding configs required by "perf probe of function from different
> > CU" was originally part of the series but has been merged in 6.10-rc1.
> > https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
> >
> > - Restructure patches according to the maintainer trees.
> > - Add explanation for why '| grep "foo"' is used.
> > - Fix build errors for when perf is built with LLVM=1.
> >
> > Changes in V2:
> > https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
> >
> > - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> > - Delete ends_with() and replace its usage with str_has_suffix()
> > - Replace an instance of strtailcmp() with str_has_suffix()
> > - Delete strstarts() from tools/include/linux/string.h and replace its
> > usage with str_has_prefix()
> >
> > Changes in V1:
> > https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
> >
> > Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Mike Leach <mike.leach@linaro.org>
> > Cc: James Clark <james.clark@arm.com>
> > Cc: John Garry <john.g.garry@oracle.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Leo Yan <leo.yan@linaro.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Ian Rogers <irogers@google.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Ravi Bangoria <ravi.bangoria@amd.com>
> > Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> > Cc: Colin Ian King <colin.i.king@gmail.com>
> > Cc: Changbin Du <changbin.du@huawei.com>
> > Cc: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > Cc: Georg Müller <georgmueller@gmx.net>
> > Cc: Liam Howlett <liam.howlett@oracle.com>
> > Cc: bpf@vger.kernel.org
> > Cc: coresight@lists.linaro.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-perf-users@vger.kernel.org
> >
> > Chaitanya S Prakash (10):
> > tools lib: adopt str_has_suffix() from bpftool/gen.c
> > perf util: Delete ends_with() and replace its use with
> > str_has_suffix()
> > perf util: Replace an instance of strtailcmp() by str_has_suffix()
> > tools lib: Adopt str_has_prefix() from kernel
> > libsubcmd: Replace strstarts() usage with str_has_prefix()
> > objtool: Replace strstarts() usage with str_has_prefix()
> > perf tools: Replace strstarts() usage with str_has_prefix()
> > tools lib: Remove strstarts() as all its usecases have been replaced
> > by str_has_prefix()
> > perf tools: Only treat files as map files when they have the extension
> > .map
> > perf test: Check output of the probe ... --funcs command
> >
> > tools/include/linux/string.h | 12 ++---
> > tools/lib/string.c | 48 +++++++++++++++++++
> > tools/lib/subcmd/help.c | 2 +-
> > tools/lib/subcmd/parse-options.c | 18 +++----
> > tools/objtool/check.c | 2 +-
> > tools/perf/arch/arm/util/pmu.c | 4 +-
> > tools/perf/arch/x86/annotate/instructions.c | 14 +++---
> > tools/perf/arch/x86/util/env.c | 2 +-
> > tools/perf/builtin-c2c.c | 4 +-
> > tools/perf/builtin-config.c | 2 +-
> > tools/perf/builtin-daemon.c | 2 +-
> > tools/perf/builtin-ftrace.c | 2 +-
> > tools/perf/builtin-help.c | 6 +--
> > tools/perf/builtin-kmem.c | 2 +-
> > tools/perf/builtin-kvm.c | 14 +++---
> > tools/perf/builtin-kwork.c | 10 ++--
> > tools/perf/builtin-lock.c | 6 +--
> > tools/perf/builtin-mem.c | 4 +-
> > tools/perf/builtin-sched.c | 6 +--
> > tools/perf/builtin-script.c | 30 ++++--------
> > tools/perf/builtin-stat.c | 4 +-
> > tools/perf/builtin-timechart.c | 2 +-
> > tools/perf/builtin-trace.c | 6 +--
> > tools/perf/perf.c | 12 ++---
> > .../shell/test_uprobe_from_different_cu.sh | 2 +-
> > tools/perf/tests/symbols.c | 2 +-
> > tools/perf/ui/browser.c | 2 +-
> > tools/perf/ui/browsers/scripts.c | 2 +-
> > tools/perf/ui/stdio/hist.c | 2 +-
> > tools/perf/util/amd-sample-raw.c | 4 +-
> > tools/perf/util/annotate.c | 2 +-
> > tools/perf/util/callchain.c | 2 +-
> > tools/perf/util/config.c | 12 ++---
> > tools/perf/util/map.c | 8 ++--
> > tools/perf/util/pmus.c | 2 +-
> > tools/perf/util/probe-event.c | 2 +-
> > tools/perf/util/sample-raw.c | 2 +-
> > tools/perf/util/symbol-elf.c | 4 +-
> > tools/perf/util/symbol.c | 3 +-
> > 39 files changed, 148 insertions(+), 117 deletions(-)
> >
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
2024-07-10 7:39 ` [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
2024-07-12 19:55 ` Namhyung Kim
@ 2024-07-15 9:34 ` James Clark
2024-07-15 9:35 ` James Clark
2024-07-15 10:27 ` James Clark
1 sibling, 2 replies; 22+ messages in thread
From: James Clark @ 2024-07-15 9:34 UTC (permalink / raw)
To: Chaitanya S Prakash, linux-perf-users, Arnaldo Carvalho de Melo,
Ian Rogers, Namhyung Kim
Cc: anshuman.khandual, Josh Poimboeuf, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Chenyuan Mi,
Masami Hiramatsu, Ravi Bangoria, Ahelenia Ziemiańska,
Colin Ian King, Changbin Du, Kan Liang, Athira Rajeev,
Tiezhu Yang, Alexey Dobriyan, Georg Müller, Liam Howlett,
bpf, coresight, linux-arm-kernel, linux-kernel, Peter Zijlstra,
Ingo Molnar
On 10/07/2024 8:39 am, Chaitanya S Prakash wrote:
> Gentle ping, are there any updates on the string clean up patch set?
>
> On 6/1/24 18:29, Chaitanya S Prakash wrote:
>> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>>
>> Perf treated all files beginning with "/tmp/perf-" as a map file despite
>> them always ending in ".map", this caused the test "perf probe of
>> function from different CU" to fail when Perf was built with NO_DWARF=1.
>> As the file was parsed as a map file, the probe...--funcs command output
>> garbage values instead of listing the functions in the binary. After
>> fixing the issue an additional check to test the output of the
>> probe...--funcs command has been added.
>>
>> Additionally, various functions within the codebase have been refactored
>> and restructured. The definition of str_has_suffix() has been adopted
>> from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
>> attempt to make the function more generic. The implementation has been
>> retained but the return values have been modified to resemble that of
>> str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
>> failure. In light of the new addition, "ends_with()", a locally defined
>> function used for checking if a string had a given suffix has been
>> deleted and str_has_suffix() has replaced its usage. A call to
>> strtailcmp() has also been replaced as str_has_suffix() seemed more
>> suited for that particular use case.
>>
>> Finally str_has_prefix() is adopted from the kernel and is added to
>> tools/lib/string.c, following which strstarts() is deleted and its use
>> has been replaced with str_has_prefix().
>>
>> This patch series has been tested on 6.10-rc1 mainline kernel, both on
>> arm64 and x86 platforms.
>>
>> Changes in V3:
>>
>> - Patch adding configs required by "perf probe of function from different
>> CU" was originally part of the series but has been merged in 6.10-rc1.
>>
>> https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
>>
>> - Restructure patches according to the maintainer trees.
>> - Add explanation for why '| grep "foo"' is used.
>> - Fix build errors for when perf is built with LLVM=1.
>>
>> Changes in V2:
>> https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
>>
>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
>> - Delete ends_with() and replace its usage with str_has_suffix()
>> - Replace an instance of strtailcmp() with str_has_suffix()
>> - Delete strstarts() from tools/include/linux/string.h and replace its
>> usage with str_has_prefix()
>>
>> Changes in V1:
>> https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
>>
>> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: John Garry <john.g.garry@oracle.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Ian Rogers <irogers@google.com>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
>> Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
>> Cc: Colin Ian King <colin.i.king@gmail.com>
>> Cc: Changbin Du <changbin.du@huawei.com>
>> Cc: Kan Liang <kan.liang@linux.intel.com>
>> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>> Cc: Georg Müller <georgmueller@gmx.net>
>> Cc: Liam Howlett <liam.howlett@oracle.com>
>> Cc: bpf@vger.kernel.org
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-perf-users@vger.kernel.org
>>
>> Chaitanya S Prakash (10):
>> tools lib: adopt str_has_suffix() from bpftool/gen.c
LGTM,
Reviewed-by: James Clark <james.clark@arm.com>
The below commit looked like it could be prone to an off by one error
but I debugged it to confirm that it's ok:
>> perf util: Delete ends_with() and replace its use with
>> str_has_suffix()
>> perf util: Replace an instance of strtailcmp() by str_has_suffix()
>> tools lib: Adopt str_has_prefix() from kernel
>> libsubcmd: Replace strstarts() usage with str_has_prefix()
>> objtool: Replace strstarts() usage with str_has_prefix()
>> perf tools: Replace strstarts() usage with str_has_prefix()
>> tools lib: Remove strstarts() as all its usecases have been replaced
>> by str_has_prefix()
>> perf tools: Only treat files as map files when they have the extension
>> .map
>> perf test: Check output of the probe ... --funcs command
>>
>> tools/include/linux/string.h | 12 ++---
>> tools/lib/string.c | 48 +++++++++++++++++++
>> tools/lib/subcmd/help.c | 2 +-
>> tools/lib/subcmd/parse-options.c | 18 +++----
>> tools/objtool/check.c | 2 +-
>> tools/perf/arch/arm/util/pmu.c | 4 +-
>> tools/perf/arch/x86/annotate/instructions.c | 14 +++---
>> tools/perf/arch/x86/util/env.c | 2 +-
>> tools/perf/builtin-c2c.c | 4 +-
>> tools/perf/builtin-config.c | 2 +-
>> tools/perf/builtin-daemon.c | 2 +-
>> tools/perf/builtin-ftrace.c | 2 +-
>> tools/perf/builtin-help.c | 6 +--
>> tools/perf/builtin-kmem.c | 2 +-
>> tools/perf/builtin-kvm.c | 14 +++---
>> tools/perf/builtin-kwork.c | 10 ++--
>> tools/perf/builtin-lock.c | 6 +--
>> tools/perf/builtin-mem.c | 4 +-
>> tools/perf/builtin-sched.c | 6 +--
>> tools/perf/builtin-script.c | 30 ++++--------
>> tools/perf/builtin-stat.c | 4 +-
>> tools/perf/builtin-timechart.c | 2 +-
>> tools/perf/builtin-trace.c | 6 +--
>> tools/perf/perf.c | 12 ++---
>> .../shell/test_uprobe_from_different_cu.sh | 2 +-
>> tools/perf/tests/symbols.c | 2 +-
>> tools/perf/ui/browser.c | 2 +-
>> tools/perf/ui/browsers/scripts.c | 2 +-
>> tools/perf/ui/stdio/hist.c | 2 +-
>> tools/perf/util/amd-sample-raw.c | 4 +-
>> tools/perf/util/annotate.c | 2 +-
>> tools/perf/util/callchain.c | 2 +-
>> tools/perf/util/config.c | 12 ++---
>> tools/perf/util/map.c | 8 ++--
>> tools/perf/util/pmus.c | 2 +-
>> tools/perf/util/probe-event.c | 2 +-
>> tools/perf/util/sample-raw.c | 2 +-
>> tools/perf/util/symbol-elf.c | 4 +-
>> tools/perf/util/symbol.c | 3 +-
>> 39 files changed, 148 insertions(+), 117 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
2024-07-15 9:34 ` James Clark
@ 2024-07-15 9:35 ` James Clark
2024-07-15 10:27 ` James Clark
1 sibling, 0 replies; 22+ messages in thread
From: James Clark @ 2024-07-15 9:35 UTC (permalink / raw)
To: Chaitanya S Prakash, linux-perf-users, Arnaldo Carvalho de Melo,
Ian Rogers, Namhyung Kim
Cc: anshuman.khandual, Josh Poimboeuf, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Chenyuan Mi,
Masami Hiramatsu, Ravi Bangoria, Ahelenia Ziemiańska,
Colin Ian King, Changbin Du, Kan Liang, Athira Rajeev,
Tiezhu Yang, Alexey Dobriyan, Georg Müller, Liam Howlett,
bpf, coresight, linux-arm-kernel, linux-kernel, Peter Zijlstra,
Ingo Molnar
On 15/07/2024 10:34 am, James Clark wrote:
>
>
> On 10/07/2024 8:39 am, Chaitanya S Prakash wrote:
>> Gentle ping, are there any updates on the string clean up patch set?
>>
>> On 6/1/24 18:29, Chaitanya S Prakash wrote:
>>> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>>>
>>> Perf treated all files beginning with "/tmp/perf-" as a map file despite
>>> them always ending in ".map", this caused the test "perf probe of
>>> function from different CU" to fail when Perf was built with NO_DWARF=1.
>>> As the file was parsed as a map file, the probe...--funcs command output
>>> garbage values instead of listing the functions in the binary. After
>>> fixing the issue an additional check to test the output of the
>>> probe...--funcs command has been added.
>>>
>>> Additionally, various functions within the codebase have been refactored
>>> and restructured. The definition of str_has_suffix() has been adopted
>>> from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
>>> attempt to make the function more generic. The implementation has been
>>> retained but the return values have been modified to resemble that of
>>> str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
>>> failure. In light of the new addition, "ends_with()", a locally defined
>>> function used for checking if a string had a given suffix has been
>>> deleted and str_has_suffix() has replaced its usage. A call to
>>> strtailcmp() has also been replaced as str_has_suffix() seemed more
>>> suited for that particular use case.
>>>
>>> Finally str_has_prefix() is adopted from the kernel and is added to
>>> tools/lib/string.c, following which strstarts() is deleted and its use
>>> has been replaced with str_has_prefix().
>>>
>>> This patch series has been tested on 6.10-rc1 mainline kernel, both on
>>> arm64 and x86 platforms.
>>>
>>> Changes in V3:
>>>
>>> - Patch adding configs required by "perf probe of function from
>>> different
>>> CU" was originally part of the series but has been merged in
>>> 6.10-rc1.
>>> https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
>>>
>>> - Restructure patches according to the maintainer trees.
>>> - Add explanation for why '| grep "foo"' is used.
>>> - Fix build errors for when perf is built with LLVM=1.
>>>
>>> Changes in V2:
>>> https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
>>>
>>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
>>> - Delete ends_with() and replace its usage with str_has_suffix()
>>> - Replace an instance of strtailcmp() with str_has_suffix()
>>> - Delete strstarts() from tools/include/linux/string.h and replace its
>>> usage with str_has_prefix()
>>>
>>> Changes in V1:
>>> https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
>>>
>>> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: James Clark <james.clark@arm.com>
>>> Cc: John Garry <john.g.garry@oracle.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Leo Yan <leo.yan@linaro.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Ian Rogers <irogers@google.com>
>>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>>> Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
>>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>>> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
>>> Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
>>> Cc: Colin Ian King <colin.i.king@gmail.com>
>>> Cc: Changbin Du <changbin.du@huawei.com>
>>> Cc: Kan Liang <kan.liang@linux.intel.com>
>>> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>>> Cc: Georg Müller <georgmueller@gmx.net>
>>> Cc: Liam Howlett <liam.howlett@oracle.com>
>>> Cc: bpf@vger.kernel.org
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-perf-users@vger.kernel.org
>>>
>>> Chaitanya S Prakash (10):
>>> tools lib: adopt str_has_suffix() from bpftool/gen.c
>
> LGTM,
>
> Reviewed-by: James Clark <james.clark@arm.com>
>
> The below commit looked like it could be prone to an off by one error
> but I debugged it to confirm that it's ok:
>
>>> perf util: Delete ends_with() and replace its use with
>>> str_has_suffix()
>
There were some trivial merge conflicts though, so I'm not sure if it's
worth sending a new version out or not.
>>> perf util: Replace an instance of strtailcmp() by str_has_suffix()
>>> tools lib: Adopt str_has_prefix() from kernel
>>> libsubcmd: Replace strstarts() usage with str_has_prefix()
>>> objtool: Replace strstarts() usage with str_has_prefix()
>>> perf tools: Replace strstarts() usage with str_has_prefix()
>>> tools lib: Remove strstarts() as all its usecases have been replaced
>>> by str_has_prefix()
>>> perf tools: Only treat files as map files when they have the
>>> extension
>>> .map
>>> perf test: Check output of the probe ... --funcs command
>>>
>>> tools/include/linux/string.h | 12 ++---
>>> tools/lib/string.c | 48 +++++++++++++++++++
>>> tools/lib/subcmd/help.c | 2 +-
>>> tools/lib/subcmd/parse-options.c | 18 +++----
>>> tools/objtool/check.c | 2 +-
>>> tools/perf/arch/arm/util/pmu.c | 4 +-
>>> tools/perf/arch/x86/annotate/instructions.c | 14 +++---
>>> tools/perf/arch/x86/util/env.c | 2 +-
>>> tools/perf/builtin-c2c.c | 4 +-
>>> tools/perf/builtin-config.c | 2 +-
>>> tools/perf/builtin-daemon.c | 2 +-
>>> tools/perf/builtin-ftrace.c | 2 +-
>>> tools/perf/builtin-help.c | 6 +--
>>> tools/perf/builtin-kmem.c | 2 +-
>>> tools/perf/builtin-kvm.c | 14 +++---
>>> tools/perf/builtin-kwork.c | 10 ++--
>>> tools/perf/builtin-lock.c | 6 +--
>>> tools/perf/builtin-mem.c | 4 +-
>>> tools/perf/builtin-sched.c | 6 +--
>>> tools/perf/builtin-script.c | 30 ++++--------
>>> tools/perf/builtin-stat.c | 4 +-
>>> tools/perf/builtin-timechart.c | 2 +-
>>> tools/perf/builtin-trace.c | 6 +--
>>> tools/perf/perf.c | 12 ++---
>>> .../shell/test_uprobe_from_different_cu.sh | 2 +-
>>> tools/perf/tests/symbols.c | 2 +-
>>> tools/perf/ui/browser.c | 2 +-
>>> tools/perf/ui/browsers/scripts.c | 2 +-
>>> tools/perf/ui/stdio/hist.c | 2 +-
>>> tools/perf/util/amd-sample-raw.c | 4 +-
>>> tools/perf/util/annotate.c | 2 +-
>>> tools/perf/util/callchain.c | 2 +-
>>> tools/perf/util/config.c | 12 ++---
>>> tools/perf/util/map.c | 8 ++--
>>> tools/perf/util/pmus.c | 2 +-
>>> tools/perf/util/probe-event.c | 2 +-
>>> tools/perf/util/sample-raw.c | 2 +-
>>> tools/perf/util/symbol-elf.c | 4 +-
>>> tools/perf/util/symbol.c | 3 +-
>>> 39 files changed, 148 insertions(+), 117 deletions(-)
>>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU"
2024-07-15 9:34 ` James Clark
2024-07-15 9:35 ` James Clark
@ 2024-07-15 10:27 ` James Clark
1 sibling, 0 replies; 22+ messages in thread
From: James Clark @ 2024-07-15 10:27 UTC (permalink / raw)
To: Chaitanya S Prakash, linux-perf-users, Arnaldo Carvalho de Melo,
Ian Rogers, Namhyung Kim
Cc: anshuman.khandual, Josh Poimboeuf, Suzuki K Poulose, Mike Leach,
John Garry, Will Deacon, Leo Yan, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Chenyuan Mi,
Masami Hiramatsu, Ravi Bangoria, Ahelenia Ziemiańska,
Colin Ian King, Changbin Du, Kan Liang, Athira Rajeev,
Tiezhu Yang, Alexey Dobriyan, Georg Müller, Liam Howlett,
bpf, coresight, linux-arm-kernel, linux-kernel, Peter Zijlstra,
Ingo Molnar
On 15/07/2024 10:34 am, James Clark wrote:
>
>
> On 10/07/2024 8:39 am, Chaitanya S Prakash wrote:
>> Gentle ping, are there any updates on the string clean up patch set?
>>
>> On 6/1/24 18:29, Chaitanya S Prakash wrote:
>>> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>>>
>>> Perf treated all files beginning with "/tmp/perf-" as a map file despite
>>> them always ending in ".map", this caused the test "perf probe of
>>> function from different CU" to fail when Perf was built with NO_DWARF=1.
>>> As the file was parsed as a map file, the probe...--funcs command output
>>> garbage values instead of listing the functions in the binary. After
>>> fixing the issue an additional check to test the output of the
>>> probe...--funcs command has been added.
>>>
>>> Additionally, various functions within the codebase have been refactored
>>> and restructured. The definition of str_has_suffix() has been adopted
>>> from tools/bpf/bpftool/gen.c and added to tools/lib/string.c in an
>>> attempt to make the function more generic. The implementation has been
>>> retained but the return values have been modified to resemble that of
>>> str_has_prefix(), i.e., return strlen(suffix) on success and 0 on
>>> failure. In light of the new addition, "ends_with()", a locally defined
>>> function used for checking if a string had a given suffix has been
>>> deleted and str_has_suffix() has replaced its usage. A call to
>>> strtailcmp() has also been replaced as str_has_suffix() seemed more
>>> suited for that particular use case.
>>>
>>> Finally str_has_prefix() is adopted from the kernel and is added to
>>> tools/lib/string.c, following which strstarts() is deleted and its use
>>> has been replaced with str_has_prefix().
>>>
>>> This patch series has been tested on 6.10-rc1 mainline kernel, both on
>>> arm64 and x86 platforms.
>>>
>>> Changes in V3:
>>>
>>> - Patch adding configs required by "perf probe of function from
>>> different
>>> CU" was originally part of the series but has been merged in
>>> 6.10-rc1.
>>> https://github.com/torvalds/linux/commit/6b718ac6874c2233b8dec369a8a377d6c5b638e6
>>>
>>> - Restructure patches according to the maintainer trees.
>>> - Add explanation for why '| grep "foo"' is used.
>>> - Fix build errors for when perf is built with LLVM=1.
>>>
>>> Changes in V2:
>>> https://lore.kernel.org/all/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com/
>>>
>>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
>>> - Delete ends_with() and replace its usage with str_has_suffix()
>>> - Replace an instance of strtailcmp() with str_has_suffix()
>>> - Delete strstarts() from tools/include/linux/string.h and replace its
>>> usage with str_has_prefix()
>>>
>>> Changes in V1:
>>> https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/
>>>
>>> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: James Clark <james.clark@arm.com>
>>> Cc: John Garry <john.g.garry@oracle.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Leo Yan <leo.yan@linaro.org>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Ian Rogers <irogers@google.com>
>>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>>> Cc: Chenyuan Mi <cymi20@fudan.edu.cn>
>>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>>> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
>>> Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
>>> Cc: Colin Ian King <colin.i.king@gmail.com>
>>> Cc: Changbin Du <changbin.du@huawei.com>
>>> Cc: Kan Liang <kan.liang@linux.intel.com>
>>> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> Cc: Alexey Dobriyan <adobriyan@gmail.com>
>>> Cc: Georg Müller <georgmueller@gmx.net>
>>> Cc: Liam Howlett <liam.howlett@oracle.com>
>>> Cc: bpf@vger.kernel.org
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-perf-users@vger.kernel.org
>>>
>>> Chaitanya S Prakash (10):
>>> tools lib: adopt str_has_suffix() from bpftool/gen.c
>
> LGTM,
>
> Reviewed-by: James Clark <james.clark@arm.com>
Reviewed-by: James Clark <james.clark@linaro.org>
Need to fix that...
>
> The below commit looked like it could be prone to an off by one error
> but I debugged it to confirm that it's ok:
>
>>> perf util: Delete ends_with() and replace its use with
>>> str_has_suffix()
>
>>> perf util: Replace an instance of strtailcmp() by str_has_suffix()
>>> tools lib: Adopt str_has_prefix() from kernel
>>> libsubcmd: Replace strstarts() usage with str_has_prefix()
>>> objtool: Replace strstarts() usage with str_has_prefix()
>>> perf tools: Replace strstarts() usage with str_has_prefix()
>>> tools lib: Remove strstarts() as all its usecases have been replaced
>>> by str_has_prefix()
>>> perf tools: Only treat files as map files when they have the
>>> extension
>>> .map
>>> perf test: Check output of the probe ... --funcs command
>>>
>>> tools/include/linux/string.h | 12 ++---
>>> tools/lib/string.c | 48 +++++++++++++++++++
>>> tools/lib/subcmd/help.c | 2 +-
>>> tools/lib/subcmd/parse-options.c | 18 +++----
>>> tools/objtool/check.c | 2 +-
>>> tools/perf/arch/arm/util/pmu.c | 4 +-
>>> tools/perf/arch/x86/annotate/instructions.c | 14 +++---
>>> tools/perf/arch/x86/util/env.c | 2 +-
>>> tools/perf/builtin-c2c.c | 4 +-
>>> tools/perf/builtin-config.c | 2 +-
>>> tools/perf/builtin-daemon.c | 2 +-
>>> tools/perf/builtin-ftrace.c | 2 +-
>>> tools/perf/builtin-help.c | 6 +--
>>> tools/perf/builtin-kmem.c | 2 +-
>>> tools/perf/builtin-kvm.c | 14 +++---
>>> tools/perf/builtin-kwork.c | 10 ++--
>>> tools/perf/builtin-lock.c | 6 +--
>>> tools/perf/builtin-mem.c | 4 +-
>>> tools/perf/builtin-sched.c | 6 +--
>>> tools/perf/builtin-script.c | 30 ++++--------
>>> tools/perf/builtin-stat.c | 4 +-
>>> tools/perf/builtin-timechart.c | 2 +-
>>> tools/perf/builtin-trace.c | 6 +--
>>> tools/perf/perf.c | 12 ++---
>>> .../shell/test_uprobe_from_different_cu.sh | 2 +-
>>> tools/perf/tests/symbols.c | 2 +-
>>> tools/perf/ui/browser.c | 2 +-
>>> tools/perf/ui/browsers/scripts.c | 2 +-
>>> tools/perf/ui/stdio/hist.c | 2 +-
>>> tools/perf/util/amd-sample-raw.c | 4 +-
>>> tools/perf/util/annotate.c | 2 +-
>>> tools/perf/util/callchain.c | 2 +-
>>> tools/perf/util/config.c | 12 ++---
>>> tools/perf/util/map.c | 8 ++--
>>> tools/perf/util/pmus.c | 2 +-
>>> tools/perf/util/probe-event.c | 2 +-
>>> tools/perf/util/sample-raw.c | 2 +-
>>> tools/perf/util/symbol-elf.c | 4 +-
>>> tools/perf/util/symbol.c | 3 +-
>>> 39 files changed, 148 insertions(+), 117 deletions(-)
>>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-07-15 10:27 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-01 12:59 [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 01/10] tools lib: adopt str_has_suffix() from bpftool/gen.c Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 02/10] perf util: Delete ends_with() and replace its use with str_has_suffix() Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 03/10] perf util: Replace an instance of strtailcmp() by str_has_suffix() Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 04/10] tools lib: Adopt str_has_prefix() from kernel Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 05/10] libsubcmd: Replace strstarts() usage with str_has_prefix() Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 06/10] objtool: " Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 07/10] perf tools: " Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 08/10] tools lib: Remove strstarts() as all its usecases have been replaced by str_has_prefix() Chaitanya S Prakash
2024-06-01 12:59 ` [PATCH V3 09/10] perf tools: Only treat files as map files when they have the extension .map Chaitanya S Prakash
2024-06-17 13:52 ` James Clark
2024-06-24 23:29 ` Namhyung Kim
2024-06-01 12:59 ` [PATCH V3 10/10] perf test: Check output of the probe ... --funcs command Chaitanya S Prakash
2024-06-24 23:30 ` Namhyung Kim
2024-06-25 12:40 ` James Clark
2024-06-25 19:08 ` Namhyung Kim
2024-06-26 3:57 ` Namhyung Kim
2024-07-10 7:39 ` [PATCH V3 00/10] perf tools: Fix test "perf probe of function from different CU" Chaitanya S Prakash
2024-07-12 19:55 ` Namhyung Kim
2024-07-15 9:34 ` James Clark
2024-07-15 9:35 ` James Clark
2024-07-15 10:27 ` James Clark
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).