Re: [PATCH v3 5/8] perf sched stats: Add support for live mode

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Mar 11, 2025 at 12:02:27PM +0000, Swapnil Sapkal wrote:
> The live mode works similar to simple `perf stat` command, by profiling
> the target and printing results on the terminal as soon as the target
> finishes.
> 
> Example usage:
> 
>   # perf sched stats -- sleep 10
> 
> Co-developed-by: Ravi Bangoria <ravi.bangoria@amd.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> Tested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
> ---
>  tools/perf/builtin-sched.c | 87 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index e2e7dbc4f0aa..9813e25b54b8 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -4364,6 +4364,91 @@ static int perf_sched__schedstat_report(struct perf_sched *sched)
>  	return err;
>  }
>  
> +static int process_synthesized_event_live(const struct perf_tool *tool __maybe_unused,
> +					  union perf_event *event,
> +					  struct perf_sample *sample __maybe_unused,
> +					  struct machine *machine __maybe_unused)
> +{
> +	return perf_sched__process_schedstat(NULL, event);
> +}
> +
> +static int perf_sched__schedstat_live(struct perf_sched *sched,
> +				      int argc, const char **argv)
> +{
> +	struct evlist *evlist;
> +	struct target *target;
> +	int reset = 0;
> +	int err = 0;
> +
> +	signal(SIGINT, sighandler);
> +	signal(SIGCHLD, sighandler);
> +	signal(SIGTERM, sighandler);
> +
> +	evlist = evlist__new();
> +	if (!evlist)
> +		return -ENOMEM;
> +
> +	/*
> +	 * `perf sched schedstat` does not support workload profiling (-p pid)
> +	 * since /proc/schedstat file contains cpu specific data only. Hence, a
> +	 * profile target is either set of cpus or systemwide, never a process.
> +	 * Note that, although `-- <workload>` is supported, profile data are
> +	 * still cpu/systemwide.
> +	 */
> +	target = zalloc(sizeof(struct target));

As I said, you can put it on stack.


> +	if (cpu_list)
> +		target->cpu_list = cpu_list;
> +	else
> +		target->system_wide = true;
> +
> +	if (argc) {
> +		err = evlist__prepare_workload(evlist, target, argv, false, NULL);
> +		if (err)
> +			goto out_target;
> +	}
> +
> +	if (cpu_list) {
> +		user_requested_cpus = perf_cpu_map__new(cpu_list);
> +		if (!user_requested_cpus)
> +			goto out_target;
> +	}

How about this instead?

	evlist__create_maps(evlist, target);

> +
> +	err = perf_event__synthesize_schedstat(&(sched->tool),
> +					       process_synthesized_event_live,
> +					       user_requested_cpus);
> +	if (err < 0)
> +		goto out_target;
> +
> +	err = enable_sched_schedstats(&reset);
> +	if (err < 0)
> +		goto out_target;
> +
> +	if (argc)
> +		evlist__start_workload(evlist);
> +
> +	/* wait for signal */
> +	pause();
> +
> +	if (reset) {
> +		err = disable_sched_schedstat();
> +		if (err < 0)
> +			goto out_target;
> +	}
> +
> +	err = perf_event__synthesize_schedstat(&(sched->tool),
> +					       process_synthesized_event_live,
> +					       user_requested_cpus);
> +	if (err)
> +		goto out_target;
> +
> +	setup_pager();
> +	show_schedstat_data(cpu_head);
> +	free_schedstat(cpu_head);
> +out_target:
> +	free(target);

	evlist__delete(evlist);

and unless you use evlist__create_maps().

	perf_cpu_map__put(user_requested_cpus);

Thanks,
Namhyung


> +	return err;
> +}
> +
>  static bool schedstat_events_exposed(void)
>  {
>  	/*
> @@ -4686,7 +4771,7 @@ int cmd_sched(int argc, const char **argv)
>  						     stats_usage, 0);
>  			return perf_sched__schedstat_report(&sched);
>  		}
> -		usage_with_options(stats_usage, stats_options);
> +		return perf_sched__schedstat_live(&sched, argc, argv);
>  	} else {
>  		usage_with_options(sched_usage, sched_options);
>  	}
> -- 
> 2.43.0
>