Messages in this thread | ![/](/images/icornerl.gif) | | From | Ian Rogers <> | Date | Wed, 24 Jan 2024 10:22:59 -0800 | Subject | Re: [PATCH V4 6/7] perf mem: Clean up perf_mem_events__record_args() |
| |
On Tue, Jan 23, 2024 at 10:51 AM <kan.liang@linux.intel.com> wrote: > > From: Kan Liang <kan.liang@linux.intel.com> > > 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 <leo.yan@linaro.org> > Tested-by: Leo Yan <leo.yan@linaro.org> > Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Reviewed-by: Ian Rogers <irogers@google.com>
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 >
| ![\](/images/icornerr.gif) |