[PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Chaitanya S Prakash]

From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>

Defconfig doesn't provide all the necessary configs required for the
test "perf probe of function from different CU" to run successfully on
all platforms. Therefore the required configs have been added to
config fragments to resolve this issue. On further investigation it was
seen that the Perf treated all files beginning with "/tmp/perf-" as a
map file despite them always ending in ".map", this caused the test 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.9-rc2 mainline kernel, both on
arm64 and x86 platforms.

Changes in V2:
- 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()

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 (8):
  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
  tools: Delete strstarts() and replace its usage with str_has_prefix()
  perf tools: Enable configs required for
    test_uprobe_from_different_cu.sh
  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                            | 42 +++++++++++++++++++
 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 +++---
 tools/perf/tests/config-fragments/config      |  3 ++
 .../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                      |  4 +-
 40 files changed, 146 insertions(+), 117 deletions(-)

-- 
2.30.2

[PATCH V2 1/8] tools lib: adopt str_has_suffix() from bpftool/gen.c

[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..9cb515048239 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 is 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.30.2

Re: [PATCH V2 1/8] tools lib: adopt str_has_suffix() from bpftool/gen.c

[Masami Hiramatsu]

On Mon,  8 Apr 2024 11:52:23 +0530
Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:

> 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..9cb515048239 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 is 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

I think this should return boolean, because user will never check the
return value is strlen(@suffix). (and user always use the return value
as a boolean)

Thank you,

> + */
> +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.30.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH V2 1/8] tools lib: adopt str_has_suffix() from bpftool/gen.c

[Chaitanya S Prakash]

On 4/9/24 05:02, Masami Hiramatsu (Google) wrote:
> On Mon,  8 Apr 2024 11:52:23 +0530
> Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:
>
>> 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..9cb515048239 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 is 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
> I think this should return boolean, because user will never check the
> return value is strlen(@suffix). (and user always use the return value
> as a boolean)
>
> Thank you,

This is in accordance with Arnaldo's comments on V1 of the patch series.
Please find the link for the same below.

https://lore.kernel.org/all/ZdYPf3wMl35VemsL@x1/

>> + */
>> +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.30.2
>>
>>
>

Re: [PATCH V2 1/8] tools lib: adopt str_has_suffix() from bpftool/gen.c

[Arnaldo Carvalho de Melo]

On Thu, Apr 11, 2024 at 05:45:25PM +0530, Chaitanya S Prakash wrote:
> On 4/9/24 05:02, Masami Hiramatsu (Google) wrote:
> > On Mon,  8 Apr 2024 11:52:23 +0530
> > Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:
> > > and add it to tools/lib/string.c The function is modified to return
> > > strlen(suffix) on success and 0 on failure.
> > > +++ 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..9cb515048239 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 is 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
> > I think this should return boolean, because user will never check the
> > return value is strlen(@suffix). (and user always use the return value
> > as a boolean)
 
> This is in accordance with Arnaldo's comments on V1 of the patch series.
> Please find the link for the same below.
 
> https://lore.kernel.org/all/ZdYPf3wMl35VemsL@x1/

Yeah, when I saw str_has_prefix() I thought it too clever, but then, why
not, there are use cases, in the kernel, where that is useful.

And one thing with tools/ that we try hard is to reduce the differences
of programming in the kernel and in there, so that people used to
functions in the kernel use them in tools/ with the same semantics.

So in these "the user will never do things like this" moments I try to
look at the users in the codebase where we're lifting the function, and
in there _there_ are uses of the strlen() return, to my surprise.

- Arnaldo
 
> > > +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;

[PATCH V2 2/8] perf util: Delete ends_with() and replace its use with str_has_suffix()

[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 37088cc0ff1b..6cedd9cdde21 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3263,20 +3263,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;
@@ -3320,19 +3306,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) {
+	script_len = strlen(script_root);
+	suffix_len = str_has_suffix(script_root, suffix);
+	if (!suffix_len) {
 		free(script_root);
 		return NULL;
 	}
 
-	*str = '\0';
+	script_root[script_len - suffix_len] = '\0';
 	return script_root;
 }
 
@@ -3608,7 +3596,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") != 0;
 }
 
 static int has_required_arg(char *script_path)
-- 
2.30.2

[PATCH V2 3/8] perf util: Replace an instance of strtailcmp() by str_has_suffix()

[Chaitanya S Prakash]

From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>

This particular instance makes use strtailcmp() to check if the "path"
parameter is a loadable kernel module, it does so by checking if "path"
is followed by ".ko" suffix. As this use case is more suitable for
str_has suffix(), replace strtailcmp() with str_has_suffix().

The other calls to strtailcmp() do not seem equivalent to the
str_has_suffix() use case. As this function is not being hit in any of
the tests as well it would be rather risky to replace its use with
str_has_suffix(). Therefore, those calls 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 2a0ad9ecf0a2..59e700170541 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.30.2

Re: [PATCH V2 3/8] perf util: Replace an instance of strtailcmp() by str_has_suffix()

[Masami Hiramatsu]

On Mon,  8 Apr 2024 11:52:25 +0530
Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:

> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> 
> This particular instance makes use strtailcmp() to check if the "path"
> parameter is a loadable kernel module, it does so by checking if "path"
> is followed by ".ko" suffix. As this use case is more suitable for
> str_has suffix(), replace strtailcmp() with str_has_suffix().
> 
> The other calls to strtailcmp() do not seem equivalent to the
> str_has_suffix() use case. As this function is not being hit in any of
> the tests as well it would be rather risky to replace its use with
> str_has_suffix(). Therefore, those calls 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 2a0ad9ecf0a2..59e700170541 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"))

if (!module || str_has_suffix(path, ".ko"))

strtailcmp(a, b) returns 0 (false) if a is end with b. So above condition
checks @module is NULL (== kernel) or @path is end with ".ko" (== module).

Thank you,

>  			pr_warning("Rebuild with CONFIG_DEBUG_INFO=y, ");
>  		else
>  			pr_warning("Rebuild with -g, ");
> -- 
> 2.30.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH V2 4/8] tools lib: Adopt str_has_prefix() from kernel

[Chaitanya S Prakash]

From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>

Add str_has_prefix() to tools/include 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           | 16 ++++++++++++++++
 2 files changed, 18 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 9cb515048239..76b15ee6493e 100644
--- a/tools/lib/string.c
+++ b/tools/lib/string.c
@@ -252,3 +252,19 @@ size_t str_has_suffix(const char *str, const char *suffix)
 
 	return suffix_len;
 }
+
+/**
+ * str_has_prefix - Test is a string has a given prefix
+ * @str: The string to be tested
+ * @prefix: The string to see if @str starts with
+ *
+ * Returns:
+ * * strlen(@prefix) if @str starts with @prefix
+ * * 0 if @str does not start with @prefix
+ */
+size_t str_has_prefix(const char *str, const char *prefix)
+{
+	size_t prefix_len = strlen(prefix);
+
+	return strncmp(str, prefix, prefix_len) == 0 ? prefix_len : 0;
+}
-- 
2.30.2

Re: [PATCH V2 4/8] tools lib: Adopt str_has_prefix() from kernel

[Masami Hiramatsu]

On Mon,  8 Apr 2024 11:52:26 +0530
Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:

> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> 
> Add str_has_prefix() to tools/include 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           | 16 ++++++++++++++++
>  2 files changed, 18 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 9cb515048239..76b15ee6493e 100644
> --- a/tools/lib/string.c
> +++ b/tools/lib/string.c
> @@ -252,3 +252,19 @@ size_t str_has_suffix(const char *str, const char *suffix)
>  
>  	return suffix_len;
>  }
> +
> +/**
> + * str_has_prefix - Test is a string has a given prefix
> + * @str: The string to be tested
> + * @prefix: The string to see if @str starts with
> + *
> + * Returns:
> + * * strlen(@prefix) if @str starts with @prefix
> + * * 0 if @str does not start with @prefix

Here, too. Is there any reason to return strlen(@prefix)?

Thank you,

> + */
> +size_t str_has_prefix(const char *str, const char *prefix)
> +{
> +	size_t prefix_len = strlen(prefix);
> +
> +	return strncmp(str, prefix, prefix_len) == 0 ? prefix_len : 0;
> +}
> -- 
> 2.30.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH V2 5/8] tools: Delete strstarts() and replace its usage with str_has_prefix()

[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() and
replace its use with str_has_prefix().

Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
 tools/include/linux/string.h                | 10 ----------
 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                 |  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 ++--
 35 files changed, 86 insertions(+), 96 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);
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 9fa75943f2ed..652a8bd0f445 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);
 		}
@@ -967,10 +967,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);
 	}
 
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)
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 16b40f5d43db..acf805fee7e4 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -3351,9 +3351,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(&section->items, item) {
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 83954af36753..99fd0283d375 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 9714327fd0ea..35eeb0fbb8ab 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 0092b9b39611..0acef2ad3041 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 230461280e45..4edc2ebcfc1e 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2673,9 +2673,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,
@@ -2697,7 +2697,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 5b851e64e4a1..6efa1e1c9396 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 b248c433529a..1a492b73bee3 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3639,9 +3639,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);
@@ -3659,7 +3659,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 6cedd9cdde21..e2be836ab07a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -4061,13 +4061,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 6bba1a89d030..84156a58c103 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2556,11 +2556,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 90eaff8c0f6e..b27a63024bb0 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3121,7 +3121,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")) {
@@ -3587,7 +3587,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;
 		}
@@ -4990,7 +4990,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 921bee0a6437..1b09a3ea1fb3 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -103,7 +103,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)
@@ -232,7 +232,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);
@@ -269,7 +269,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)
@@ -496,7 +496,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);
@@ -507,7 +507,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");
@@ -529,7 +529,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 d208105919ed..62dffb951e65 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, text_dso->short_name))
+	if (dso != text_dso && str_has_prefix(dso->short_name, text_dso->short_name))
 		if (test_dso(dso) != TEST_OK)
 			return -1;
 
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 603d11283cbd..62c2f333760e 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -577,7 +577,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 ac002d907d81..45ecd6d46cf8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -3420,7 +3420,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 7517d16c02ec..00f897a9e828 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 14a5ea70d81e..9bf0d0d9c1a1 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 16505071d362..89cc3de0208f 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -46,7 +46,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 0b91f813c4fa..d0b203cf8573 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.30.2

Re: [PATCH V2 5/8] tools: Delete strstarts() and replace its usage with str_has_prefix()

[kernel test robot]

Hi Chaitanya,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on perf-tools/perf-tools linus/master v6.9-rc4 next-20240416]
[cannot apply to perf-tools-next/perf-tools-next acme/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chaitanya-S-Prakash/tools-lib-adopt-str_has_suffix-from-bpftool-gen-c/20240408-182552
base:   tip/perf/core
patch link:    https://lore.kernel.org/r/20240408062230.1949882-6-ChaitanyaS.Prakash%40arm.com
patch subject: [PATCH V2 5/8] tools: Delete strstarts() and replace its usage with str_has_prefix()
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <yujie.liu@intel.com>
| Closes: https://lore.kernel.org/r/202404171224.IlskYwJw-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ui/browser.c:580:6: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses]
     580 |         if (!str_has_prefix(var, "colors.") != 0)
         |             ^                               ~~
>> ui/browser.c:580:6: note: add parentheses after the '!' to evaluate the comparison first
     580 |         if (!str_has_prefix(var, "colors.") != 0)
         |             ^                                   
         |              (                                  )
>> ui/browser.c:580:6: note: add parentheses around left hand side expression to silence this warning
     580 |         if (!str_has_prefix(var, "colors.") != 0)
         |             ^
         |             (                              )
   1 error generated.
   make[7]: *** [tools/build/Makefile.build:105: tools/perf/ui/browser.o] Error 1
   make[6]: *** [tools/build/Makefile.build:158: ui] Error 2
   make[5]: *** [Makefile.perf:706: tools/perf/perf-in.o] Error 2
   make[4]: *** [Makefile.perf:264: sub-make] Error 2
   make[3]: *** [Makefile:70: all] Error 2

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH V2 6/8] perf tools: Enable configs required for test_uprobe_from_different_cu.sh

[Chaitanya S Prakash]

From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>

Test "perf probe of function from different CU" fails due to certain
configs not being enabled. Building the kernel with
CONFIG_KPROBE_EVENTS=y and CONFIG_UPROBE_EVENTS=y fixes the issue. As
CONFIG_KPROBE_EVENTS is dependent on CONFIG_KPROBES, enable it as well.
Some platforms enable these configs as a part of their defconfig, so
this change is only required for the ones that don't do so.

Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
---
 tools/perf/tests/config-fragments/config | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/tests/config-fragments/config b/tools/perf/tests/config-fragments/config
index c340b3195fca..4fca12851016 100644
--- a/tools/perf/tests/config-fragments/config
+++ b/tools/perf/tests/config-fragments/config
@@ -9,3 +9,6 @@ CONFIG_GENERIC_TRACER=y
 CONFIG_FTRACE=y
 CONFIG_FTRACE_SYSCALLS=y
 CONFIG_BRANCH_PROFILE_NONE=y
+CONFIG_KPROBES=y
+CONFIG_KPROBE_EVENTS=y
+CONFIG_UPROBE_EVENTS=y
-- 
2.30.2

Re: [PATCH V2 6/8] perf tools: Enable configs required for test_uprobe_from_different_cu.sh

[Masami Hiramatsu]

On Mon,  8 Apr 2024 11:52:28 +0530
Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:

> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> 
> Test "perf probe of function from different CU" fails due to certain
> configs not being enabled. Building the kernel with
> CONFIG_KPROBE_EVENTS=y and CONFIG_UPROBE_EVENTS=y fixes the issue. As
> CONFIG_KPROBE_EVENTS is dependent on CONFIG_KPROBES, enable it as well.
> Some platforms enable these configs as a part of their defconfig, so
> this change is only required for the ones that don't do so.
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> ---
>  tools/perf/tests/config-fragments/config | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/tests/config-fragments/config b/tools/perf/tests/config-fragments/config
> index c340b3195fca..4fca12851016 100644
> --- a/tools/perf/tests/config-fragments/config
> +++ b/tools/perf/tests/config-fragments/config
> @@ -9,3 +9,6 @@ CONFIG_GENERIC_TRACER=y
>  CONFIG_FTRACE=y
>  CONFIG_FTRACE_SYSCALLS=y
>  CONFIG_BRANCH_PROFILE_NONE=y
> +CONFIG_KPROBES=y
> +CONFIG_KPROBE_EVENTS=y
> +CONFIG_UPROBE_EVENTS=y
> -- 
> 2.30.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH V2 6/8] perf tools: Enable configs required for test_uprobe_from_different_cu.sh

[Arnaldo Carvalho de Melo]

On Tue, Apr 09, 2024 at 08:11:17AM +0900, Masami Hiramatsu wrote:
> On Mon,  8 Apr 2024 11:52:28 +0530
> Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:
> 
> > From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> > 
> > Test "perf probe of function from different CU" fails due to certain
> > configs not being enabled. Building the kernel with
> > CONFIG_KPROBE_EVENTS=y and CONFIG_UPROBE_EVENTS=y fixes the issue. As
> > CONFIG_KPROBE_EVENTS is dependent on CONFIG_KPROBES, enable it as well.
> > Some platforms enable these configs as a part of their defconfig, so
> > this change is only required for the ones that don't do so.
> > 
> 
> Looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Trying to cherry pick patches from this series to make progress.

- Arnaldo

> Thank you,
> 
> > Signed-off-by: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> > ---
> >  tools/perf/tests/config-fragments/config | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/perf/tests/config-fragments/config b/tools/perf/tests/config-fragments/config
> > index c340b3195fca..4fca12851016 100644
> > --- a/tools/perf/tests/config-fragments/config
> > +++ b/tools/perf/tests/config-fragments/config
> > @@ -9,3 +9,6 @@ CONFIG_GENERIC_TRACER=y
> >  CONFIG_FTRACE=y
> >  CONFIG_FTRACE_SYSCALLS=y
> >  CONFIG_BRANCH_PROFILE_NONE=y
> > +CONFIG_KPROBES=y
> > +CONFIG_KPROBE_EVENTS=y
> > +CONFIG_UPROBE_EVENTS=y
> > -- 
> > 2.30.2
> > 
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH V2 7/8] perf tools: Only treat files as map files when they have the extension .map

[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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 9ebdb8e13c0b..812684aa42ce 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1792,7 +1792,9 @@ int dso__load(struct dso *dso, struct map *map)
 	const char *map_path = dso->long_name;
 
 	mutex_lock(&dso->lock);
-	perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0;
+	perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0 &&
+		  str_has_suffix(dso->name, ".map") != 0;
+
 	if (perfmap) {
 		if (dso->nsinfo && (dso__find_perf_map(newmapname,
 		    sizeof(newmapname), &dso->nsinfo) == 0)) {
-- 
2.30.2

Re: [PATCH V2 7/8] perf tools: Only treat files as map files when they have the extension .map

[James Clark]

On 08/04/2024 07:22, 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 9ebdb8e13c0b..812684aa42ce 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1792,7 +1792,9 @@ int dso__load(struct dso *dso, struct map *map)
>  	const char *map_path = dso->long_name;
>  
>  	mutex_lock(&dso->lock);
> -	perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0;
> +	perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0 &&
> +		  str_has_suffix(dso->name, ".map") != 0;
> +
>  	if (perfmap) {
>  		if (dso->nsinfo && (dso__find_perf_map(newmapname,
>  		    sizeof(newmapname), &dso->nsinfo) == 0)) {

Reviewed-by: James Clark <james.clark@arm.com>

[PATCH V2 8/8] perf test: Check output of the probe ... --funcs command

[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.

An additional check to grep for "foo" has been added to test the --funcs
output.

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.30.2

Re: [PATCH V2 8/8] perf test: Check output of the probe ... --funcs command

[Masami Hiramatsu]

On Mon,  8 Apr 2024 11:52:30 +0530
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.
> 
> An additional check to grep for "foo" has been added to test the --funcs
> output.

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,

> 
> 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.30.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH V2 8/8] perf test: Check output of the probe ... --funcs command

[Arnaldo Carvalho de Melo]

On Tue, Apr 09, 2024 at 08:09:02AM +0900, Masami Hiramatsu wrote:
> On Mon,  8 Apr 2024 11:52:30 +0530
> 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.
> > 
> > An additional check to grep for "foo" has been added to test the --funcs
> > output.
> 
> Looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Cherry picked this one as well.

- Arnaldo

Re: [PATCH V2 8/8] perf test: Check output of the probe ... --funcs command

[Arnaldo Carvalho de Melo]

On Tue, Apr 09, 2024 at 08:09:02AM +0900, Masami Hiramatsu wrote:
> On Mon,  8 Apr 2024 11:52:30 +0530
> 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.
> > 
> > An additional check to grep for "foo" has been added to test the --funcs
> > output.
> 
> Looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

> > +++ 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

Have you tested this?

Because:

root@x1:~# perf probe -x /lib64/libc.so.6 malloc | grep malloc || echo failure
Added new event:
  probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)

You can now use it in all perf tools, such as:

	perf record -e probe_libc:malloc -aR sleep 1

failure
root@x1:~# perf probe -d *:*
Removed event: probe_libc:malloc
root@x1:~# perf probe -x /lib64/libc.so.6 malloc |& grep malloc || echo failure
  probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)
	perf record -e probe_libc:malloc -aR sleep 1
root@x1:~#


that plain '| grep "foo"' isn't enough, will always fail, as the output
isn't sent to stdout, but to stderr, so, with your patch:

root@x1:~# perf test different
121: test perf probe of function from different CU                   : FAILED!
root@x1:~# set -o vi
root@x1:~# perf test -v different
121: test perf probe of function from different CU:
--- start ---
test child forked, pid 938178
--- Cleaning up ---
"foo" does not hit any event.
  Error: Failed to delete events.
---- end(-1) ----
121: test perf probe of function from different CU                   : FAILED!
root@x1:~# 


We need to use '|&' to grep in both stdout and stderr, see how 'perf
probe' uses stderr:

root@x1:~# perf probe -d *:*
Removed event: probe_libc:malloc
root@x1:~# perf probe -x /lib64/libc.so.6 malloc > /dev/null
Added new event:
  probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)

You can now use it in all perf tools, such as:

	perf record -e probe_libc:malloc -aR sleep 1

root@x1:~


Using '|&' to grep:

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 82bc774a078a15d5..af8b1fa639f3daa4 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 | grep "foo"
+perf probe -x ${temp_dir}/testfile --funcs foo |& grep "foo"
 perf probe -x ${temp_dir}/testfile foo
 
 cleanup
⬢[acme@toolbox perf-tools-next]$

root@x1:~# perf test different
121: test perf probe of function from different CU                   : Ok
root@x1:~# set -o vi
root@x1:~# perf test -v different
121: test perf probe of function from different CU                   : Ok
root@x1:~# perf test -vv different
121: test perf probe of function from different CU:
--- start ---
test child forked, pid 1048947
Added new event:
  probe_testfile:foo   (on foo in /tmp/perf-uprobe-different-cu-sh.5qg9UX2dUJ/testfile)

You can now use it in all perf tools, such as:

	perf record -e probe_testfile:foo -aR sleep 1

--- Cleaning up ---
Removed event: probe_testfile:foo
---- end(0) ----
121: test perf probe of function from different CU                   : Ok
root@x1:~#

I'm amending the patch with this fix.


Thanks,

- Arnaldo

Re: [PATCH V2 8/8] perf test: Check output of the probe ... --funcs command

[Chaitanya S Prakash]

On 4/17/24 23:56, Arnaldo Carvalho de Melo wrote:
> On Tue, Apr 09, 2024 at 08:09:02AM +0900, Masami Hiramatsu wrote:
>> On Mon,  8 Apr 2024 11:52:30 +0530
>> 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.
>>>
>>> An additional check to grep for "foo" has been added to test the --funcs
>>> output.
>> Looks good to me.
>>
>> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>>> +++ 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
> Have you tested this?
>
> Because:
>
> root@x1:~# perf probe -x /lib64/libc.so.6 malloc | grep malloc || echo failure
> Added new event:
>    probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)
>
> You can now use it in all perf tools, such as:
>
> 	perf record -e probe_libc:malloc -aR sleep 1
>
> failure
> root@x1:~# perf probe -d *:*
> Removed event: probe_libc:malloc
> root@x1:~# perf probe -x /lib64/libc.so.6 malloc |& grep malloc || echo failure
>    probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)
> 	perf record -e probe_libc:malloc -aR sleep 1
> root@x1:~#
>
>
> that plain '| grep "foo"' isn't enough, will always fail, as the output
> isn't sent to stdout, but to stderr, so, with your patch:
>
> root@x1:~# perf test different
> 121: test perf probe of function from different CU                   : FAILED!
> root@x1:~# set -o vi
> root@x1:~# perf test -v different
> 121: test perf probe of function from different CU:
> --- start ---
> test child forked, pid 938178
> --- Cleaning up ---
> "foo" does not hit any event.
>    Error: Failed to delete events.
> ---- end(-1) ----
> 121: test perf probe of function from different CU                   : FAILED!
> root@x1:~#
>
>
> We need to use '|&' to grep in both stdout and stderr, see how 'perf
> probe' uses stderr:
>
> root@x1:~# perf probe -d *:*
> Removed event: probe_libc:malloc
> root@x1:~# perf probe -x /lib64/libc.so.6 malloc > /dev/null
> Added new event:
>    probe_libc:malloc    (on malloc in /usr/lib64/libc.so.6)
>
> You can now use it in all perf tools, such as:
>
> 	perf record -e probe_libc:malloc -aR sleep 1
>
> root@x1:~
>
>
> Using '|&' to grep:
>
> 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 82bc774a078a15d5..af8b1fa639f3daa4 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 | grep "foo"
> +perf probe -x ${temp_dir}/testfile --funcs foo |& grep "foo"
>   perf probe -x ${temp_dir}/testfile foo
>   
>   cleanup
> ⬢[acme@toolbox perf-tools-next]$
>
> root@x1:~# perf test different
> 121: test perf probe of function from different CU                   : Ok
> root@x1:~# set -o vi
> root@x1:~# perf test -v different
> 121: test perf probe of function from different CU                   : Ok
> root@x1:~# perf test -vv different
> 121: test perf probe of function from different CU:
> --- start ---
> test child forked, pid 1048947
> Added new event:
>    probe_testfile:foo   (on foo in /tmp/perf-uprobe-different-cu-sh.5qg9UX2dUJ/testfile)
>
> You can now use it in all perf tools, such as:
>
> 	perf record -e probe_testfile:foo -aR sleep 1
>
> --- Cleaning up ---
> Removed event: probe_testfile:foo
> ---- end(0) ----
> 121: test perf probe of function from different CU                   : Ok
> root@x1:~#
>
> I'm amending the patch with this fix.
>
>
> Thanks,
>
> - Arnaldo

Hi Arnaldo, line 80 of the test makes use of the --funcs option with
perf probe, which redirects the output to stdout itself. So this change
might not be required.

[~/workspace/linux/tools/perf]$ sudo ./perf probe -x 
/lib/x86_64-linux-gnu/libc.so.6 --funcs malloc | grep malloc || echo failure
malloc

Thanks,

- Chaitanya

>

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Alexey Dobriyan]

On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> - Delete ends_with() and replace its usage with str_has_suffix()

> - Delete strstarts() from tools/include/linux/string.h and replace its
>   usage with str_has_prefix()

It should be the other way: starts_with is normal in userspace.
C++, Python, Java, C# all have it. JavaScript too!

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Chaitanya S Prakash]

On 4/9/24 11:02, Alexey Dobriyan wrote:
> On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
>> - Delete ends_with() and replace its usage with str_has_suffix()
>> - Delete strstarts() from tools/include/linux/string.h and replace its
>>    usage with str_has_prefix()
> It should be the other way: starts_with is normal in userspace.
> C++, Python, Java, C# all have it. JavaScript too!

This is done in accordance with Ian's comments on V1 of this patch
series. Please find the link to the same below.

https://lore.kernel.org/all/CAP-5=fUFmeoTjLuZTgcaV23iGQU1AdddG+7Rw=d6buMU007+1Q@mail.gmail.com/

>

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Alexey Dobriyan]

On Thu, Apr 11, 2024 at 05:40:04PM +0530, Chaitanya S Prakash wrote:
> 
> On 4/9/24 11:02, Alexey Dobriyan wrote:
> > On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> > > - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> > > - Delete ends_with() and replace its usage with str_has_suffix()
> > > - Delete strstarts() from tools/include/linux/string.h and replace its
> > >    usage with str_has_prefix()
> > It should be the other way: starts_with is normal in userspace.
> > C++, Python, Java, C# all have it. JavaScript too!
> 
> This is done in accordance with Ian's comments on V1 of this patch
> series. Please find the link to the same below.

Yes, but str_has_suffix() doesn't make sense in the wider context.

> https://lore.kernel.org/all/CAP-5=fUFmeoTjLuZTgcaV23iGQU1AdddG+7Rw=d6buMU007+1Q@mail.gmail.com/

	The naming ends_with makes sense but there is also strstarts and
	str_has_prefix, perhaps str_has_suffix would be the most consistent
	and intention revealing name?

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[James Clark]

On 14/04/2024 12:41, Alexey Dobriyan wrote:
> On Thu, Apr 11, 2024 at 05:40:04PM +0530, Chaitanya S Prakash wrote:
>>
>> On 4/9/24 11:02, Alexey Dobriyan wrote:
>>> On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
>>>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
>>>> - Delete ends_with() and replace its usage with str_has_suffix()
>>>> - Delete strstarts() from tools/include/linux/string.h and replace its
>>>>    usage with str_has_prefix()
>>> It should be the other way: starts_with is normal in userspace.
>>> C++, Python, Java, C# all have it. JavaScript too!
>>
>> This is done in accordance with Ian's comments on V1 of this patch
>> series. Please find the link to the same below.
> 
> Yes, but str_has_suffix() doesn't make sense in the wider context.
> 
>> https://lore.kernel.org/all/CAP-5=fUFmeoTjLuZTgcaV23iGQU1AdddG+7Rw=d6buMU007+1Q@mail.gmail.com/
> 
> 	The naming ends_with makes sense but there is also strstarts and
> 	str_has_prefix, perhaps str_has_suffix would be the most consistent
> 	and intention revealing name?
> 

Hi Alexey,

From a brief check it looks like str_has_prefix() is already quite
common with 94 uses. So the path of least resistance is to make
everything self consistent and add str_has_suffix().

I agree it's a bit of a mouthful and not so common in other languages.
Once this more complicated set gets through we could always do a simple
search and replace change it to anything we like. But it would touch
_lots_ of different drivers and trees, so it would be hard to justify.

Thanks
James

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Arnaldo Carvalho de Melo]

On Wed, Apr 17, 2024 at 02:24:33PM +0100, James Clark wrote:
> On 14/04/2024 12:41, Alexey Dobriyan wrote:
> > On Thu, Apr 11, 2024 at 05:40:04PM +0530, Chaitanya S Prakash wrote:
> >> On 4/9/24 11:02, Alexey Dobriyan wrote:
> >>> On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> >>>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> >>>> - Delete ends_with() and replace its usage with str_has_suffix()
> >>>> - Delete strstarts() from tools/include/linux/string.h and replace its
> >>>>    usage with str_has_prefix()
> >>> It should be the other way: starts_with is normal in userspace.
> >>> C++, Python, Java, C# all have it. JavaScript too!
> >>
> >> This is done in accordance with Ian's comments on V1 of this patch
> >> series. Please find the link to the same below.
> > 
> > Yes, but str_has_suffix() doesn't make sense in the wider context.
> > 
> >> https://lore.kernel.org/all/CAP-5=fUFmeoTjLuZTgcaV23iGQU1AdddG+7Rw=d6buMU007+1Q@mail.gmail.com/
> > 
> > 	The naming ends_with makes sense but there is also strstarts and
> > 	str_has_prefix, perhaps str_has_suffix would be the most consistent
> > 	and intention revealing name?
 
> From a brief check it looks like str_has_prefix() is already quite
> common with 94 uses. So the path of least resistance is to make
> everything self consistent and add str_has_suffix().
 
> I agree it's a bit of a mouthful and not so common in other languages.
> Once this more complicated set gets through we could always do a simple
> search and replace change it to anything we like. But it would touch
> _lots_ of different drivers and trees, so it would be hard to justify.

Right, we try to follow the kernel APIs to make tools/perf more familiar
to kernel developers, this return strlen() thing on str_has_prefix()
looked too clever for me at first, but since we need to do it anyway,
and return !0 to indicate it has the prefix and there are usecase...

The bpftool people agreed as well, so in tools/ it seems we're in
general agreement about using str_has_{prefix,suffix}().

- Arnaldo

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Chaitanya S Prakash]

Link to V1: 
https://lore.kernel.org/all/20240220042957.2022391-1-ChaitanyaS.Prakash@arm.com/

On 4/8/24 11:52, Chaitanya S Prakash wrote:
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
>
> Defconfig doesn't provide all the necessary configs required for the
> test "perf probe of function from different CU" to run successfully on
> all platforms. Therefore the required configs have been added to
> config fragments to resolve this issue. On further investigation it was
> seen that the Perf treated all files beginning with "/tmp/perf-" as a
> map file despite them always ending in ".map", this caused the test 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.9-rc2 mainline kernel, both on
> arm64 and x86 platforms.
>
> Changes in V2:
> - 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()
>
> 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 (8):
>    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
>    tools: Delete strstarts() and replace its usage with str_has_prefix()
>    perf tools: Enable configs required for
>      test_uprobe_from_different_cu.sh
>    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                            | 42 +++++++++++++++++++
>   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 +++---
>   tools/perf/tests/config-fragments/config      |  3 ++
>   .../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                      |  4 +-
>   40 files changed, 146 insertions(+), 117 deletions(-)
>

Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"

[Arnaldo Carvalho de Melo]

On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> From: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
> 
> Defconfig doesn't provide all the necessary configs required for the
> test "perf probe of function from different CU" to run successfully on
> all platforms. Therefore the required configs have been added to
> config fragments to resolve this issue. On further investigation it was
> seen that the Perf treated all files beginning with "/tmp/perf-" as a
> map file despite them always ending in ".map", this caused the test 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.9-rc2 mainline kernel, both on
> arm64 and x86 platforms.
> 
> Changes in V2:
> - 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()
> 
> 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 (8):
>   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
>   tools: Delete strstarts() and replace its usage with str_has_prefix()
>   perf tools: Enable configs required for
>     test_uprobe_from_different_cu.sh
>   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                            | 42 +++++++++++++++++++
>  tools/lib/subcmd/help.c                       |  2 +-
>  tools/lib/subcmd/parse-options.c              | 18 ++++----
>  tools/objtool/check.c                         |  2 +-

Try not mixing things that are maintained by different people into the
same patch kit, for instance, in my case its failing with:

Cover: ./v2_20240408_chaitanyas_prakash_perf_tools_fix_test_perf_probe_of_function_from_different_cu.cover
 Link: https://lore.kernel.org/r/20240408062230.1949882-1-ChaitanyaS.Prakash@arm.com
 Base: not specified
       git am ./v2_20240408_chaitanyas_prakash_perf_tools_fix_test_perf_probe_of_function_from_different_cu.mbx
⬢[acme@toolbox perf-tools-next]$        git am ./v2_20240408_chaitanyas_prakash_perf_tools_fix_test_perf_probe_of_function_from_different_cu.mbx
Applying: tools lib: adopt str_has_suffix() from bpftool/gen.c
Applying: perf util: Delete ends_with() and replace its use with str_has_suffix()
Applying: perf util: Replace an instance of strtailcmp() by str_has_suffix()
Applying: tools lib: Adopt str_has_prefix() from kernel
Applying: tools: Delete strstarts() and replace its usage with str_has_prefix()
error: patch failed: tools/objtool/check.c:2535
error: tools/objtool/check.c: patch does not apply
Patch failed at 0005 tools: Delete strstarts() and replace its usage with str_has_prefix()
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
⬢[acme@toolbox perf-tools-next]$

So I'm checking if removing the objtool part makes this work.

- Arnaldo

>  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 +++---
>  tools/perf/tests/config-fragments/config      |  3 ++
>  .../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                      |  4 +-
>  40 files changed, 146 insertions(+), 117 deletions(-)
> 
> -- 
> 2.30.2
> 
>