From: Kan Liang <[email protected]>
The current code iterates all memory PMUs. It doesn't matter if the
system has only one memory PMU or multiple PMUs. The check of
perf_pmus__num_mem_pmus() is not required anymore.
The rec_tmp is not used in c2c and mem. Removing them as well.
Suggested-by: Leo Yan <[email protected]>
Tested-by: Leo Yan <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
---
tools/perf/builtin-c2c.c | 15 ++-------------
tools/perf/builtin-mem.c | 18 ++----------------
tools/perf/util/mem-events.c | 34 ++++++++++++----------------------
tools/perf/util/mem-events.h | 3 +--
4 files changed, 17 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 838481505e08..3bcb903b6b38 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -3245,9 +3245,8 @@ static const char * const *record_mem_usage = __usage_record;
static int perf_c2c__record(int argc, const char **argv)
{
- int rec_argc, i = 0, j, rec_tmp_nr = 0;
+ int rec_argc, i = 0, j;
const char **rec_argv;
- char **rec_tmp;
int ret;
bool all_user = false, all_kernel = false;
bool event_set = false;
@@ -3285,12 +3284,6 @@ static int perf_c2c__record(int argc, const char **argv)
if (!rec_argv)
return -1;
- rec_tmp = calloc(rec_argc + 1, sizeof(char *));
- if (!rec_tmp) {
- free(rec_argv);
- return -1;
- }
-
rec_argv[i++] = "record";
if (!event_set) {
@@ -3319,7 +3312,7 @@ static int perf_c2c__record(int argc, const char **argv)
rec_argv[i++] = "--phys-data";
rec_argv[i++] = "--sample-cpu";
- ret = perf_mem_events__record_args(rec_argv, &i, rec_tmp, &rec_tmp_nr);
+ ret = perf_mem_events__record_args(rec_argv, &i);
if (ret)
goto out;
@@ -3346,10 +3339,6 @@ static int perf_c2c__record(int argc, const char **argv)
ret = cmd_record(i, rec_argv);
out:
- for (i = 0; i < rec_tmp_nr; i++)
- free(rec_tmp[i]);
-
- free(rec_tmp);
free(rec_argv);
return ret;
}
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index ef64bae77ca7..1d92e309c97c 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -72,10 +72,9 @@ static const char * const *record_mem_usage = __usage;
static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
{
- int rec_argc, i = 0, j, tmp_nr = 0;
+ int rec_argc, i = 0, j;
int start, end;
const char **rec_argv;
- char **rec_tmp;
int ret;
bool all_user = false, all_kernel = false;
struct perf_mem_event *e;
@@ -116,15 +115,6 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
if (!rec_argv)
return -1;
- /*
- * Save the allocated event name strings.
- */
- rec_tmp = calloc(rec_argc + 1, sizeof(char *));
- if (!rec_tmp) {
- free(rec_argv);
- return -1;
- }
-
rec_argv[i++] = "record";
e = perf_pmu__mem_events_ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
@@ -163,7 +153,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
rec_argv[i++] = "--data-page-size";
start = i;
- ret = perf_mem_events__record_args(rec_argv, &i, rec_tmp, &tmp_nr);
+ ret = perf_mem_events__record_args(rec_argv, &i);
if (ret)
goto out;
end = i;
@@ -193,10 +183,6 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
ret = cmd_record(i, rec_argv);
out:
- for (i = 0; i < tmp_nr; i++)
- free(rec_tmp[i]);
-
- free(rec_tmp);
free(rec_argv);
return ret;
}
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 7d7df3d0b2b9..a20611b4fb1b 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -218,14 +218,14 @@ void perf_pmu__mem_events_list(struct perf_pmu *pmu)
}
}
-int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
- char **rec_tmp, int *tmp_nr)
+int perf_mem_events__record_args(const char **rec_argv, int *argv_nr)
{
const char *mnt = sysfs__mount();
struct perf_pmu *pmu = NULL;
- int i = *argv_nr, k = 0;
struct perf_mem_event *e;
-
+ int i = *argv_nr;
+ const char *s;
+ char *copy;
while ((pmu = perf_pmus__scan_mem(pmu)) != NULL) {
for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
@@ -240,30 +240,20 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
return -1;
}
- if (perf_pmus__num_mem_pmus() == 1) {
- rec_argv[i++] = "-e";
- rec_argv[i++] = perf_pmu__mem_events_name(j, pmu);
- } else {
- const char *s = perf_pmu__mem_events_name(j, pmu);
-
- if (!perf_pmu__mem_events_supported(mnt, pmu, e))
- continue;
+ s = perf_pmu__mem_events_name(j, pmu);
+ if (!s || !perf_pmu__mem_events_supported(mnt, pmu, e))
+ continue;
- rec_argv[i++] = "-e";
- if (s) {
- char *copy = strdup(s);
- if (!copy)
- return -1;
+ copy = strdup(s);
+ if (!copy)
+ return -1;
- rec_argv[i++] = copy;
- rec_tmp[k++] = copy;
- }
- }
+ rec_argv[i++] = "-e";
+ rec_argv[i++] = copy;
}
}
*argv_nr = i;
- *tmp_nr = k;
return 0;
}
diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
index f817a507b106..c97cd3caa766 100644
--- a/tools/perf/util/mem-events.h
+++ b/tools/perf/util/mem-events.h
@@ -46,8 +46,7 @@ struct perf_pmu *perf_mem_events_find_pmu(void);
bool is_mem_loads_aux_event(struct evsel *leader);
void perf_pmu__mem_events_list(struct perf_pmu *pmu);
-int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
- char **rec_tmp, int *tmp_nr);
+int perf_mem_events__record_args(const char **rec_argv, int *argv_nr);
int perf_mem__tlb_scnprintf(char *out, size_t sz, struct mem_info *mem_info);
int perf_mem__lvl_scnprintf(char *out, size_t sz, struct mem_info *mem_info);
--
2.35.1
On Tue, Jan 23, 2024 at 10:51 AM <[email protected]> wrote:
>
> From: Kan Liang <[email protected]>
>
> The current code iterates all memory PMUs. It doesn't matter if the
> system has only one memory PMU or multiple PMUs. The check of
> perf_pmus__num_mem_pmus() is not required anymore.
>
> The rec_tmp is not used in c2c and mem. Removing them as well.
>
> Suggested-by: Leo Yan <[email protected]>
> Tested-by: Leo Yan <[email protected]>
> Signed-off-by: Kan Liang <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Thanks for the cleanup!
Ian
> ---
> tools/perf/builtin-c2c.c | 15 ++-------------
> tools/perf/builtin-mem.c | 18 ++----------------
> tools/perf/util/mem-events.c | 34 ++++++++++++----------------------
> tools/perf/util/mem-events.h | 3 +--
> 4 files changed, 17 insertions(+), 53 deletions(-)
>
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 838481505e08..3bcb903b6b38 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -3245,9 +3245,8 @@ static const char * const *record_mem_usage = __usage_record;
>
> static int perf_c2c__record(int argc, const char **argv)
> {
> - int rec_argc, i = 0, j, rec_tmp_nr = 0;
> + int rec_argc, i = 0, j;
> const char **rec_argv;
> - char **rec_tmp;
> int ret;
> bool all_user = false, all_kernel = false;
> bool event_set = false;
> @@ -3285,12 +3284,6 @@ static int perf_c2c__record(int argc, const char **argv)
> if (!rec_argv)
> return -1;
>
> - rec_tmp = calloc(rec_argc + 1, sizeof(char *));
> - if (!rec_tmp) {
> - free(rec_argv);
> - return -1;
> - }
> -
> rec_argv[i++] = "record";
>
> if (!event_set) {
> @@ -3319,7 +3312,7 @@ static int perf_c2c__record(int argc, const char **argv)
> rec_argv[i++] = "--phys-data";
> rec_argv[i++] = "--sample-cpu";
>
> - ret = perf_mem_events__record_args(rec_argv, &i, rec_tmp, &rec_tmp_nr);
> + ret = perf_mem_events__record_args(rec_argv, &i);
> if (ret)
> goto out;
>
> @@ -3346,10 +3339,6 @@ static int perf_c2c__record(int argc, const char **argv)
>
> ret = cmd_record(i, rec_argv);
> out:
> - for (i = 0; i < rec_tmp_nr; i++)
> - free(rec_tmp[i]);
> -
> - free(rec_tmp);
> free(rec_argv);
> return ret;
> }
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index ef64bae77ca7..1d92e309c97c 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -72,10 +72,9 @@ static const char * const *record_mem_usage = __usage;
>
> static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> {
> - int rec_argc, i = 0, j, tmp_nr = 0;
> + int rec_argc, i = 0, j;
> int start, end;
> const char **rec_argv;
> - char **rec_tmp;
> int ret;
> bool all_user = false, all_kernel = false;
> struct perf_mem_event *e;
> @@ -116,15 +115,6 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> if (!rec_argv)
> return -1;
>
> - /*
> - * Save the allocated event name strings.
> - */
> - rec_tmp = calloc(rec_argc + 1, sizeof(char *));
> - if (!rec_tmp) {
> - free(rec_argv);
> - return -1;
> - }
> -
> rec_argv[i++] = "record";
>
> e = perf_pmu__mem_events_ptr(pmu, PERF_MEM_EVENTS__LOAD_STORE);
> @@ -163,7 +153,7 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
> rec_argv[i++] = "--data-page-size";
>
> start = i;
> - ret = perf_mem_events__record_args(rec_argv, &i, rec_tmp, &tmp_nr);
> + ret = perf_mem_events__record_args(rec_argv, &i);
> if (ret)
> goto out;
> end = i;
> @@ -193,10 +183,6 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem)
>
> ret = cmd_record(i, rec_argv);
> out:
> - for (i = 0; i < tmp_nr; i++)
> - free(rec_tmp[i]);
> -
> - free(rec_tmp);
> free(rec_argv);
> return ret;
> }
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 7d7df3d0b2b9..a20611b4fb1b 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -218,14 +218,14 @@ void perf_pmu__mem_events_list(struct perf_pmu *pmu)
> }
> }
>
> -int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
> - char **rec_tmp, int *tmp_nr)
> +int perf_mem_events__record_args(const char **rec_argv, int *argv_nr)
> {
> const char *mnt = sysfs__mount();
> struct perf_pmu *pmu = NULL;
> - int i = *argv_nr, k = 0;
> struct perf_mem_event *e;
> -
> + int i = *argv_nr;
> + const char *s;
> + char *copy;
>
> while ((pmu = perf_pmus__scan_mem(pmu)) != NULL) {
> for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
> @@ -240,30 +240,20 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
> return -1;
> }
>
> - if (perf_pmus__num_mem_pmus() == 1) {
> - rec_argv[i++] = "-e";
> - rec_argv[i++] = perf_pmu__mem_events_name(j, pmu);
> - } else {
> - const char *s = perf_pmu__mem_events_name(j, pmu);
> -
> - if (!perf_pmu__mem_events_supported(mnt, pmu, e))
> - continue;
> + s = perf_pmu__mem_events_name(j, pmu);
> + if (!s || !perf_pmu__mem_events_supported(mnt, pmu, e))
> + continue;
>
> - rec_argv[i++] = "-e";
> - if (s) {
> - char *copy = strdup(s);
> - if (!copy)
> - return -1;
> + copy = strdup(s);
> + if (!copy)
> + return -1;
>
> - rec_argv[i++] = copy;
> - rec_tmp[k++] = copy;
> - }
> - }
> + rec_argv[i++] = "-e";
> + rec_argv[i++] = copy;
> }
> }
>
> *argv_nr = i;
> - *tmp_nr = k;
> return 0;
> }
>
> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index f817a507b106..c97cd3caa766 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -46,8 +46,7 @@ struct perf_pmu *perf_mem_events_find_pmu(void);
> bool is_mem_loads_aux_event(struct evsel *leader);
>
> void perf_pmu__mem_events_list(struct perf_pmu *pmu);
> -int perf_mem_events__record_args(const char **rec_argv, int *argv_nr,
> - char **rec_tmp, int *tmp_nr);
> +int perf_mem_events__record_args(const char **rec_argv, int *argv_nr);
>
> int perf_mem__tlb_scnprintf(char *out, size_t sz, struct mem_info *mem_info);
> int perf_mem__lvl_scnprintf(char *out, size_t sz, struct mem_info *mem_info);
> --
> 2.35.1
>