* [PATCH v2] perf tracepoint: Don't scan all tracepoints to test if one exists
@ 2024-05-09 15:32 Ian Rogers
2024-05-09 21:43 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 2+ messages in thread
From: Ian Rogers @ 2024-05-09 15:32 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, linux-perf-users,
linux-kernel
In is_valid_tracepoint, rather than scanning
"/sys/kernel/tracing/events/*/*" skipping any path where
"/sys/kernel/tracing/events/*/*/id" doesn't exist, and then testing if
"*:*" matches the tracepoint name, just use the given tracepoint name
replace the ':' with '/' and see if the id file exists. This turns a
nested directory search into a single file available test.
Rather than return 1 for valid and 0 for invalid, return true and
false.
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2. Fix potential uninitialized use on memory allocation failure path.
---
tools/perf/util/tracepoint.c | 56 ++++++++++++++----------------------
tools/perf/util/tracepoint.h | 3 +-
2 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/tools/perf/util/tracepoint.c b/tools/perf/util/tracepoint.c
index 92dd8b455b90..bb06d10fbe37 100644
--- a/tools/perf/util/tracepoint.c
+++ b/tools/perf/util/tracepoint.c
@@ -4,10 +4,12 @@
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
+#include <stdlib.h>
#include <sys/param.h>
#include <unistd.h>
#include <api/fs/tracing_path.h>
+#include "fncache.h"
int tp_event_has_id(const char *dir_path, struct dirent *evt_dir)
{
@@ -26,39 +28,25 @@ int tp_event_has_id(const char *dir_path, struct dirent *evt_dir)
/*
* Check whether event is in <debugfs_mount_point>/tracing/events
*/
-int is_valid_tracepoint(const char *event_string)
+bool is_valid_tracepoint(const char *event_string)
{
- DIR *sys_dir, *evt_dir;
- struct dirent *sys_dirent, *evt_dirent;
- char evt_path[MAXPATHLEN];
- char *dir_path;
-
- sys_dir = tracing_events__opendir();
- if (!sys_dir)
- return 0;
-
- for_each_subsystem(sys_dir, sys_dirent) {
- dir_path = get_events_file(sys_dirent->d_name);
- if (!dir_path)
- continue;
- evt_dir = opendir(dir_path);
- if (!evt_dir)
- goto next;
-
- for_each_event(dir_path, evt_dir, evt_dirent) {
- snprintf(evt_path, MAXPATHLEN, "%s:%s",
- sys_dirent->d_name, evt_dirent->d_name);
- if (!strcmp(evt_path, event_string)) {
- closedir(evt_dir);
- put_events_file(dir_path);
- closedir(sys_dir);
- return 1;
- }
- }
- closedir(evt_dir);
-next:
- put_events_file(dir_path);
- }
- closedir(sys_dir);
- return 0;
+ char *dst, *path = malloc(strlen(event_string) + 4); /* Space for "/id\0". */
+ const char *src;
+ bool have_file = false; /* Conservatively return false if memory allocation failed. */
+
+ if (!path)
+ return false;
+
+ /* Copy event_string replacing the ':' with '/'. */
+ for (src = event_string, dst = path; *src; src++, dst++)
+ *dst = (*src == ':') ? '/' : *src;
+ /* Add "/id\0". */
+ memcpy(dst, "/id", 4);
+
+ dst = get_events_file(path);
+ if (dst)
+ have_file = file_available(dst);
+ free(dst);
+ free(path);
+ return have_file;
}
diff --git a/tools/perf/util/tracepoint.h b/tools/perf/util/tracepoint.h
index c4a110fe87d7..65ccb01fc312 100644
--- a/tools/perf/util/tracepoint.h
+++ b/tools/perf/util/tracepoint.h
@@ -4,6 +4,7 @@
#include <dirent.h>
#include <string.h>
+#include <stdbool.h>
int tp_event_has_id(const char *dir_path, struct dirent *evt_dir);
@@ -20,6 +21,6 @@ int tp_event_has_id(const char *dir_path, struct dirent *evt_dir);
(strcmp(sys_dirent->d_name, ".")) && \
(strcmp(sys_dirent->d_name, "..")))
-int is_valid_tracepoint(const char *event_string);
+bool is_valid_tracepoint(const char *event_string);
#endif /* __PERF_TRACEPOINT_H */
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] perf tracepoint: Don't scan all tracepoints to test if one exists
2024-05-09 15:32 [PATCH v2] perf tracepoint: Don't scan all tracepoints to test if one exists Ian Rogers
@ 2024-05-09 21:43 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-09 21:43 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
linux-perf-users, linux-kernel
On Thu, May 09, 2024 at 08:32:45AM -0700, Ian Rogers wrote:
> In is_valid_tracepoint, rather than scanning
> "/sys/kernel/tracing/events/*/*" skipping any path where
> "/sys/kernel/tracing/events/*/*/id" doesn't exist, and then testing if
> "*:*" matches the tracepoint name, just use the given tracepoint name
> replace the ':' with '/' and see if the id file exists. This turns a
> nested directory search into a single file available test.
>
> Rather than return 1 for valid and 0 for invalid, return true and
> false.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2. Fix potential uninitialized use on memory allocation failure path.
Thanks, applied to perf-tools-next,
- Arnaldo
> ---
> tools/perf/util/tracepoint.c | 56 ++++++++++++++----------------------
> tools/perf/util/tracepoint.h | 3 +-
> 2 files changed, 24 insertions(+), 35 deletions(-)
>
> diff --git a/tools/perf/util/tracepoint.c b/tools/perf/util/tracepoint.c
> index 92dd8b455b90..bb06d10fbe37 100644
> --- a/tools/perf/util/tracepoint.c
> +++ b/tools/perf/util/tracepoint.c
> @@ -4,10 +4,12 @@
> #include <errno.h>
> #include <fcntl.h>
> #include <stdio.h>
> +#include <stdlib.h>
> #include <sys/param.h>
> #include <unistd.h>
>
> #include <api/fs/tracing_path.h>
> +#include "fncache.h"
>
> int tp_event_has_id(const char *dir_path, struct dirent *evt_dir)
> {
> @@ -26,39 +28,25 @@ int tp_event_has_id(const char *dir_path, struct dirent *evt_dir)
> /*
> * Check whether event is in <debugfs_mount_point>/tracing/events
> */
> -int is_valid_tracepoint(const char *event_string)
> +bool is_valid_tracepoint(const char *event_string)
> {
> - DIR *sys_dir, *evt_dir;
> - struct dirent *sys_dirent, *evt_dirent;
> - char evt_path[MAXPATHLEN];
> - char *dir_path;
> -
> - sys_dir = tracing_events__opendir();
> - if (!sys_dir)
> - return 0;
> -
> - for_each_subsystem(sys_dir, sys_dirent) {
> - dir_path = get_events_file(sys_dirent->d_name);
> - if (!dir_path)
> - continue;
> - evt_dir = opendir(dir_path);
> - if (!evt_dir)
> - goto next;
> -
> - for_each_event(dir_path, evt_dir, evt_dirent) {
> - snprintf(evt_path, MAXPATHLEN, "%s:%s",
> - sys_dirent->d_name, evt_dirent->d_name);
> - if (!strcmp(evt_path, event_string)) {
> - closedir(evt_dir);
> - put_events_file(dir_path);
> - closedir(sys_dir);
> - return 1;
> - }
> - }
> - closedir(evt_dir);
> -next:
> - put_events_file(dir_path);
> - }
> - closedir(sys_dir);
> - return 0;
> + char *dst, *path = malloc(strlen(event_string) + 4); /* Space for "/id\0". */
> + const char *src;
> + bool have_file = false; /* Conservatively return false if memory allocation failed. */
> +
> + if (!path)
> + return false;
> +
> + /* Copy event_string replacing the ':' with '/'. */
> + for (src = event_string, dst = path; *src; src++, dst++)
> + *dst = (*src == ':') ? '/' : *src;
> + /* Add "/id\0". */
> + memcpy(dst, "/id", 4);
> +
> + dst = get_events_file(path);
> + if (dst)
> + have_file = file_available(dst);
> + free(dst);
> + free(path);
> + return have_file;
> }
> diff --git a/tools/perf/util/tracepoint.h b/tools/perf/util/tracepoint.h
> index c4a110fe87d7..65ccb01fc312 100644
> --- a/tools/perf/util/tracepoint.h
> +++ b/tools/perf/util/tracepoint.h
> @@ -4,6 +4,7 @@
>
> #include <dirent.h>
> #include <string.h>
> +#include <stdbool.h>
>
> int tp_event_has_id(const char *dir_path, struct dirent *evt_dir);
>
> @@ -20,6 +21,6 @@ int tp_event_has_id(const char *dir_path, struct dirent *evt_dir);
> (strcmp(sys_dirent->d_name, ".")) && \
> (strcmp(sys_dirent->d_name, "..")))
>
> -int is_valid_tracepoint(const char *event_string);
> +bool is_valid_tracepoint(const char *event_string);
>
> #endif /* __PERF_TRACEPOINT_H */
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-09 21:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-09 15:32 [PATCH v2] perf tracepoint: Don't scan all tracepoints to test if one exists Ian Rogers
2024-05-09 21:43 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.