Received-SPF: pass (google.com: domain of linux-kernel+bounces-53270-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 5 Feb 2024 15:59:53 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Yang Jihong <yangjihong1@huawei.com>
Cc: peterz@infradead.org, mingo@redhat.com, namhyung@kernel.org,
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
	jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/5] perf sched: Move curr_thread initialization to
 perf_sched__map()
Message-ID: <ZcEwKf8Jmd6yciWa@x1>
References: <20240205104616.132417-1-yangjihong1@huawei.com>
 <20240205104616.132417-4-yangjihong1@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240205104616.132417-4-yangjihong1@huawei.com>

On Mon, Feb 05, 2024 at 10:46:14AM +0000, Yang Jihong wrote:
> The curr_thread is used only for the 'perf sched map'. Put initialization
> in perf_sched__map() to reduce unnecessary actions in other commands.
> 
> Simple functional testing:
> 
>   # perf sched record perf bench sched messaging
>   # Running 'sched/messaging' benchmark:
>   # 20 sender and receiver processes per group
>   # 10 groups == 400 processes run
> 
>        Total time: 0.197 [sec]
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 15.526 MB perf.data (140095 samples) ]
> 
>   # perf sched map
>     *A0                                                               451264.532445 secs A0 => migration/0:15
>     *.                                                                451264.532468 secs .  => swapper:0
>      .  *B0                                                           451264.532537 secs B0 => migration/1:21
>      .  *.                                                            451264.532560 secs
>      .   .  *C0                                                       451264.532644 secs C0 => migration/2:27
>      .   .  *.                                                        451264.532668 secs
>      .   .   .  *D0                                                   451264.532753 secs D0 => migration/3:33
>      .   .   .  *.                                                    451264.532778 secs
>      .   .   .   .  *E0                                               451264.532861 secs E0 => migration/4:39
>      .   .   .   .  *.                                                451264.532886 secs
>      .   .   .   .   .  *F0                                           451264.532973 secs F0 => migration/5:45
>   <SNIP>
>      A7  A7  A7  A7  A7 *A7  .   .   .   .   .   .   .   .   .   .    451264.790785 secs
>      A7  A7  A7  A7  A7  A7 *A7  .   .   .   .   .   .   .   .   .    451264.790858 secs
>      A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .   .   .   .   .   .    451264.790934 secs
>      A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .   .   .   .   .    451264.791004 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .   .   .   .    451264.791075 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .   .   .    451264.791143 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .   .    451264.791232 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .   .    451264.791336 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .   .    451264.791407 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7  .    451264.791484 secs
>      A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7  A7 *A7   451264.791553 secs
>   # echo $?
>   0
> 
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> ---
>  tools/perf/builtin-sched.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 26dbfa4aab61..54d79e560617 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -3266,9 +3266,13 @@ static int perf_sched__map(struct perf_sched *sched)
>  {
>  	int rc = -1;
>  
> -	if (setup_map_cpus(sched))
> +	sched->curr_thread = calloc(MAX_CPUS, sizeof(*(sched->curr_thread)));
> +	if (!sched->curr_thread)
>  		return rc;
>  
> +	if (setup_map_cpus(sched))
> +		goto out_free_curr_thread;
> +
>  	if (setup_color_pids(sched))
>  		goto out_free_map_cpus;
>  
> @@ -3291,6 +3295,9 @@ static int perf_sched__map(struct perf_sched *sched)
>  out_free_map_cpus:
>  	free(sched->map.comp_cpus);
>  	perf_cpu_map__put(sched->map.cpus);
> +
> +out_free_curr_thread:
> +	free(sched->curr_thread);

	zfree(&sched->curr_thread);

>  	return rc;
>  }
>  
> @@ -3576,11 +3583,6 @@ int cmd_sched(int argc, const char **argv)
>  	unsigned int i;
>  	int ret = 0;
>  
> -	sched.curr_thread = calloc(MAX_CPUS, sizeof(*sched.curr_thread));
> -	if (!sched.curr_thread) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
>  	sched.cpu_last_switched = calloc(MAX_CPUS, sizeof(*sched.cpu_last_switched));
>  	if (!sched.cpu_last_switched) {
>  		ret = -ENOMEM;
> @@ -3662,7 +3664,6 @@ int cmd_sched(int argc, const char **argv)
>  out:
>  	free(sched.curr_pid);
>  	free(sched.cpu_last_switched);
> -	free(sched.curr_thread);
>  
>  	return ret;
>  }
> -- 
> 2.34.1