[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZcEwlR9itdN6tIL1@x1>
Date: Mon, 5 Feb 2024 16:01:41 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Yang Jihong <yangjihong1@...wei.com>
Cc: peterz@...radead.org, mingo@...hat.com, namhyung@...nel.org,
mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] perf sched: Move curr_pid and cpu_last_switched
initialization to perf_sched__{lat|map|replay}()
On Mon, Feb 05, 2024 at 10:46:15AM +0000, Yang Jihong wrote:
> +static int setup_cpus_switch_event(struct perf_sched *sched)
> +{
> + unsigned int i;
> +
> + sched->cpu_last_switched = calloc(MAX_CPUS, sizeof(*(sched->cpu_last_switched)));
> + if (!sched->cpu_last_switched)
> + return -1;
> +
> + sched->curr_pid = malloc(MAX_CPUS * sizeof(*(sched->curr_pid)));
> + if (!sched->curr_pid) {
> + free(sched->cpu_last_switched);
zfree(&sched->cpu_last_switched);
> + return -1;
> + }
> +
> + for (i = 0; i < MAX_CPUS; i++)
> + sched->curr_pid[i] = -1;
> +
> + return 0;
> +}
> +
> +static void free_cpus_switch_event(struct perf_sched *sched)
> +{
> + free(sched->curr_pid);
> + free(sched->cpu_last_switched);
zfree(&sched->curr_pid);
zfree(&sched->cpu_last_switched);
> +}
> +
> static int perf_sched__lat(struct perf_sched *sched)
> {
> + int rc = -1;
> struct rb_node *next;
>
> setup_pager();
>
> + if (setup_cpus_switch_event(sched))
> + return rc;
> +
> if (perf_sched__read_events(sched))
> - return -1;
> + goto out_free_cpus_switch_event;
>
> perf_sched__merge_lat(sched);
> perf_sched__sort_lat(sched);
> @@ -3203,7 +3233,11 @@ static int perf_sched__lat(struct perf_sched *sched)
> print_bad_events(sched);
> printf("\n");
>
> - return 0;
> + rc = 0;
> +
> +out_free_cpus_switch_event:
> + free_cpus_switch_event(sched);
> + return rc;
> }
>
> static int setup_map_cpus(struct perf_sched *sched)
> @@ -3270,9 +3304,12 @@ static int perf_sched__map(struct perf_sched *sched)
> if (!sched->curr_thread)
> return rc;
>
> - if (setup_map_cpus(sched))
> + if (setup_cpus_switch_event(sched))
> goto out_free_curr_thread;
>
> + if (setup_map_cpus(sched))
> + goto out_free_cpus_switch_event;
> +
> if (setup_color_pids(sched))
> goto out_free_map_cpus;
>
> @@ -3296,6 +3333,9 @@ static int perf_sched__map(struct perf_sched *sched)
> free(sched->map.comp_cpus);
> perf_cpu_map__put(sched->map.cpus);
>
> +out_free_cpus_switch_event:
> + free_cpus_switch_event(sched);
> +
> out_free_curr_thread:
> free(sched->curr_thread);
> return rc;
> @@ -3309,6 +3349,10 @@ static int perf_sched__replay(struct perf_sched *sched)
> mutex_init(&sched->start_work_mutex);
> mutex_init(&sched->work_done_wait_mutex);
>
> + ret = setup_cpus_switch_event(sched);
> + if (ret)
> + goto out_mutex_destroy;
> +
> calibrate_run_measurement_overhead(sched);
> calibrate_sleep_measurement_overhead(sched);
>
> @@ -3316,7 +3360,7 @@ static int perf_sched__replay(struct perf_sched *sched)
>
> ret = perf_sched__read_events(sched);
> if (ret)
> - goto out_mutex_destroy;
> + goto out_free_cpus_switch_event;
>
> printf("nr_run_events: %ld\n", sched->nr_run_events);
> printf("nr_sleep_events: %ld\n", sched->nr_sleep_events);
> @@ -3342,6 +3386,9 @@ static int perf_sched__replay(struct perf_sched *sched)
> sched->thread_funcs_exit = true;
> destroy_tasks(sched);
>
> +out_free_cpus_switch_event:
> + free_cpus_switch_event(sched);
> +
> out_mutex_destroy:
> mutex_destroy(&sched->start_work_mutex);
> mutex_destroy(&sched->work_done_wait_mutex);
> @@ -3580,21 +3627,7 @@ int cmd_sched(int argc, const char **argv)
> .switch_event = replay_switch_event,
> .fork_event = replay_fork_event,
> };
> - unsigned int i;
> - int ret = 0;
> -
> - sched.cpu_last_switched = calloc(MAX_CPUS, sizeof(*sched.cpu_last_switched));
> - if (!sched.cpu_last_switched) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - sched.curr_pid = malloc(MAX_CPUS * sizeof(*sched.curr_pid));
> - if (!sched.curr_pid) {
> - ret = -ENOMEM;
> - goto out;
> - }
> - for (i = 0; i < MAX_CPUS; i++)
> - sched.curr_pid[i] = -1;
> + int ret;
>
> argc = parse_options_subcommand(argc, argv, sched_options, sched_subcommands,
> sched_usage, PARSE_OPT_STOP_AT_NON_OPTION);
> @@ -3605,9 +3638,9 @@ int cmd_sched(int argc, const char **argv)
> * Aliased to 'perf script' for now:
> */
> if (!strcmp(argv[0], "script")) {
> - ret = cmd_script(argc, argv);
> + return cmd_script(argc, argv);
> } else if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
> - ret = __cmd_record(argc, argv);
> + return __cmd_record(argc, argv);
> } else if (strlen(argv[0]) > 2 && strstarts("latency", argv[0])) {
> sched.tp_handler = &lat_ops;
> if (argc > 1) {
> @@ -3616,7 +3649,7 @@ int cmd_sched(int argc, const char **argv)
> usage_with_options(latency_usage, latency_options);
> }
> setup_sorting(&sched, latency_options, latency_usage);
> - ret = perf_sched__lat(&sched);
> + return perf_sched__lat(&sched);
> } else if (!strcmp(argv[0], "map")) {
> if (argc) {
> argc = parse_options(argc, argv, map_options, map_usage, 0);
> @@ -3625,7 +3658,7 @@ int cmd_sched(int argc, const char **argv)
> }
> sched.tp_handler = &map_ops;
> setup_sorting(&sched, latency_options, latency_usage);
> - ret = perf_sched__map(&sched);
> + return perf_sched__map(&sched);
> } else if (strlen(argv[0]) > 2 && strstarts("replay", argv[0])) {
> sched.tp_handler = &replay_ops;
> if (argc) {
> @@ -3633,7 +3666,7 @@ int cmd_sched(int argc, const char **argv)
> if (argc)
> usage_with_options(replay_usage, replay_options);
> }
> - ret = perf_sched__replay(&sched);
> + return perf_sched__replay(&sched);
> } else if (!strcmp(argv[0], "timehist")) {
> if (argc) {
> argc = parse_options(argc, argv, timehist_options,
> @@ -3649,21 +3682,16 @@ int cmd_sched(int argc, const char **argv)
> parse_options_usage(NULL, timehist_options, "w", true);
> if (sched.show_next)
> parse_options_usage(NULL, timehist_options, "n", true);
> - ret = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
> ret = symbol__validate_sym_arguments();
> if (ret)
> - goto out;
> + return ret;
>
> - ret = perf_sched__timehist(&sched);
> + return perf_sched__timehist(&sched);
> } else {
> usage_with_options(sched_usage, sched_options);
> }
>
> -out:
> - free(sched.curr_pid);
> - free(sched.cpu_last_switched);
> -
> - return ret;
> + return 0;
> }
> --
> 2.34.1
Powered by blists - more mailing lists