2024-04-17 15:28:35

by Madadi Vineeth Reddy

[permalink] [raw]
Subject: [PATCH] perf sched map: Add command-name option to filter the output map

By default, perf sched map prints sched-in events for all the tasks
which may not be required all the time as it prints lot of symbols
and rows to the terminal.

With --command-name option, one could specify the specific command
for which the map has to be shown. This would help in analyzing the
CPU usage patterns easier for that specific command. Since multiple
PID's might have the same command name, using command-name filter
would be more useful for debugging.

For other tasks, instead of printing the symbol, ** is printed and
the same . is used to represent idle. ** is used instead of symbol
for other tasks because it helps in clear visualization of command
of interest and secondly the symbol itself doesn't mean anything
because the sched-in of that symbol will not be printed(first sched-in
contains pid and the corresponding symbol).

6.8.0
======
*A0 213864.670142 secs A0 => migration/0:18
*. 213864.670148 secs . => swapper:0
. *B0 213864.670217 secs B0 => migration/1:21
. *. 213864.670223 secs
. . *C0 213864.670247 secs C0 => migration/2:26
. . *. 213864.670252 secs

6.8.0 + patch (--command-name = schbench)
=============
** . ** *A0 213864.671055 secs A0 => schbench:104834
*B0 . . A0 213864.671156 secs B0 => schbench:104835
*C0 . . A0 213864.671187 secs C0 => schbench:104836
*D0 . . A0 213864.671219 secs D0 => schbench:104837
*E0 . . A0 213864.671250 secs E0 => schbench:104838
E0 . *D0 A0

This helps in visualizing how a benchmark like schbench is spread over
the available cpus while also knowing which cpus are idle(.) and which
are not(**). This will be more useful as number of CPUs increase.

Signed-off-by: Madadi Vineeth Reddy <[email protected]>
---
tools/perf/Documentation/perf-sched.txt | 4 ++++
tools/perf/builtin-sched.c | 17 ++++++++++++++---
2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
index 5fbe42bd599b..b04a37560935 100644
--- a/tools/perf/Documentation/perf-sched.txt
+++ b/tools/perf/Documentation/perf-sched.txt
@@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
--color-pids::
Highlight the given pids.

+--command-name::
+ Map output only for the given command name.
+ (** indicates other tasks while . is idle).
+
OPTIONS for 'perf sched timehist'
---------------------------------
-k::
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 0fce7d8986c0..e60836da53e5 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -156,6 +156,7 @@ struct perf_sched_map {
const char *color_pids_str;
struct perf_cpu_map *color_cpus;
const char *color_cpus_str;
+ const char *command;
struct perf_cpu_map *cpus;
const char *cpus_str;
};
@@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,

sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);

- printf(" ");
-
new_shortname = 0;
if (!tr->shortname[0]) {
if (!strcmp(thread__comm_str(sched_in), "swapper")) {
@@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
*/
tr->shortname[0] = '.';
tr->shortname[1] = ' ';
- } else {
+ } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
+ sched->map.command)) {
tr->shortname[0] = sched->next_shortname1;
tr->shortname[1] = sched->next_shortname2;

@@ -1618,10 +1618,18 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
else
sched->next_shortname2 = '0';
}
+ } else {
+ tr->shortname[0] = '*';
+ tr->shortname[1] = '*';
}
new_shortname = 1;
}

+ if (sched->map.command && strcmp(thread__comm_str(sched_in), sched->map.command))
+ goto skip;
+
+ printf(" ");
+
for (i = 0; i < cpus_nr; i++) {
struct perf_cpu cpu = {
.cpu = sched->map.comp ? sched->map.comp_cpus[i].cpu : i,
@@ -1678,6 +1686,7 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
out:
color_fprintf(stdout, color, "\n");

+skip:
thread__put(sched_in);

return 0;
@@ -3560,6 +3569,8 @@ int cmd_sched(int argc, const char **argv)
"highlight given CPUs in map"),
OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
"display given CPUs in map"),
+ OPT_STRING(0, "command-name", &sched.map.command, "command",
+ "map output only for the given command name"),
OPT_PARENT(sched_options)
};
const struct option timehist_options[] = {
--
2.39.1



2024-05-12 16:11:54

by Madadi Vineeth Reddy

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map

On 17/04/24 20:55, Madadi Vineeth Reddy wrote:
> By default, perf sched map prints sched-in events for all the tasks
> which may not be required all the time as it prints lot of symbols
> and rows to the terminal.
>
> With --command-name option, one could specify the specific command
> for which the map has to be shown. This would help in analyzing the
> CPU usage patterns easier for that specific command. Since multiple
> PID's might have the same command name, using command-name filter
> would be more useful for debugging.
>
> For other tasks, instead of printing the symbol, ** is printed and
> the same . is used to represent idle. ** is used instead of symbol
> for other tasks because it helps in clear visualization of command
> of interest and secondly the symbol itself doesn't mean anything
> because the sched-in of that symbol will not be printed(first sched-in
> contains pid and the corresponding symbol).
>
> 6.8.0
> ======
> *A0 213864.670142 secs A0 => migration/0:18
> *. 213864.670148 secs . => swapper:0
> . *B0 213864.670217 secs B0 => migration/1:21
> . *. 213864.670223 secs
> . . *C0 213864.670247 secs C0 => migration/2:26
> . . *. 213864.670252 secs
>
> 6.8.0 + patch (--command-name = schbench)
> =============
> ** . ** *A0 213864.671055 secs A0 => schbench:104834
> *B0 . . A0 213864.671156 secs B0 => schbench:104835
> *C0 . . A0 213864.671187 secs C0 => schbench:104836
> *D0 . . A0 213864.671219 secs D0 => schbench:104837
> *E0 . . A0 213864.671250 secs E0 => schbench:104838
> E0 . *D0 A0
>
> This helps in visualizing how a benchmark like schbench is spread over
> the available cpus while also knowing which cpus are idle(.) and which
> are not(**). This will be more useful as number of CPUs increase.
>
> Signed-off-by: Madadi Vineeth Reddy <[email protected]>
> ---
> tools/perf/Documentation/perf-sched.txt | 4 ++++
> tools/perf/builtin-sched.c | 17 ++++++++++++++---
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
> index 5fbe42bd599b..b04a37560935 100644
> --- a/tools/perf/Documentation/perf-sched.txt
> +++ b/tools/perf/Documentation/perf-sched.txt
> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
> --color-pids::
> Highlight the given pids.
>
> +--command-name::
> + Map output only for the given command name.
> + (** indicates other tasks while . is idle).
> +
> OPTIONS for 'perf sched timehist'
> ---------------------------------
> -k::
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 0fce7d8986c0..e60836da53e5 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -156,6 +156,7 @@ struct perf_sched_map {
> const char *color_pids_str;
> struct perf_cpu_map *color_cpus;
> const char *color_cpus_str;
> + const char *command;
> struct perf_cpu_map *cpus;
> const char *cpus_str;
> };
> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>
> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>
> - printf(" ");
> -
> new_shortname = 0;
> if (!tr->shortname[0]) {
> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> */
> tr->shortname[0] = '.';
> tr->shortname[1] = ' ';
> - } else {
> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
> + sched->map.command)) {
> tr->shortname[0] = sched->next_shortname1;
> tr->shortname[1] = sched->next_shortname2;
>
> @@ -1618,10 +1618,18 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> else
> sched->next_shortname2 = '0';
> }
> + } else {
> + tr->shortname[0] = '*';
> + tr->shortname[1] = '*';
> }
> new_shortname = 1;
> }
>
> + if (sched->map.command && strcmp(thread__comm_str(sched_in), sched->map.command))
> + goto skip;
> +
> + printf(" ");
> +
> for (i = 0; i < cpus_nr; i++) {
> struct perf_cpu cpu = {
> .cpu = sched->map.comp ? sched->map.comp_cpus[i].cpu : i,
> @@ -1678,6 +1686,7 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> out:
> color_fprintf(stdout, color, "\n");
>
> +skip:
> thread__put(sched_in);
>
> return 0;
> @@ -3560,6 +3569,8 @@ int cmd_sched(int argc, const char **argv)
> "highlight given CPUs in map"),
> OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
> "display given CPUs in map"),
> + OPT_STRING(0, "command-name", &sched.map.command, "command",
> + "map output only for the given command name"),
> OPT_PARENT(sched_options)
> };
> const struct option timehist_options[] = {

Ping.

Hi all, any comments on this patch?

Thanks and Regards
Madadi Vineeth Reddy

2024-06-06 14:38:05

by Athira Rajeev

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map



> On 17 Apr 2024, at 8:55 PM, Madadi Vineeth Reddy <[email protected]> wrote:
>
> By default, perf sched map prints sched-in events for all the tasks
> which may not be required all the time as it prints lot of symbols
> and rows to the terminal.
>
> With --command-name option, one could specify the specific command
> for which the map has to be shown. This would help in analyzing the
> CPU usage patterns easier for that specific command. Since multiple
> PID's might have the same command name, using command-name filter
> would be more useful for debugging.
>
> For other tasks, instead of printing the symbol, ** is printed and
> the same . is used to represent idle. ** is used instead of symbol
> for other tasks because it helps in clear visualization of command
> of interest and secondly the symbol itself doesn't mean anything
> because the sched-in of that symbol will not be printed(first sched-in
> contains pid and the corresponding symbol).
>
> 6.8.0
> ======
> *A0 213864.670142 secs A0 => migration/0:18
> *. 213864.670148 secs . => swapper:0
> . *B0 213864.670217 secs B0 => migration/1:21
> . *. 213864.670223 secs
> . . *C0 213864.670247 secs C0 => migration/2:26
> . . *. 213864.670252 secs
>
> 6.8.0 + patch (--command-name = schbench)
> =============
> ** . ** *A0 213864.671055 secs A0 => schbench:104834
> *B0 . . A0 213864.671156 secs B0 => schbench:104835
> *C0 . . A0 213864.671187 secs C0 => schbench:104836
> *D0 . . A0 213864.671219 secs D0 => schbench:104837
> *E0 . . A0 213864.671250 secs E0 => schbench:104838
> E0 . *D0 A0
>
> This helps in visualizing how a benchmark like schbench is spread over
> the available cpus while also knowing which cpus are idle(.) and which
> are not(**). This will be more useful as number of CPUs increase.
>
> Signed-off-by: Madadi Vineeth Reddy <[email protected]>

Tested the patch and looks good to me

Command to record: perf sched record sleep 5

With the patch, tested below and checked it filters and provides results for “sleep” and “perf”

perf sched map --command-name sleep
perf sched map --command-name perf

Reviewed-and-tested-by: Athira Rajeev <[email protected] <mailto:[email protected]>>

Thanks
Athira
> ---
> tools/perf/Documentation/perf-sched.txt | 4 ++++
> tools/perf/builtin-sched.c | 17 ++++++++++++++---
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
> index 5fbe42bd599b..b04a37560935 100644
> --- a/tools/perf/Documentation/perf-sched.txt
> +++ b/tools/perf/Documentation/perf-sched.txt
> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
> --color-pids::
> Highlight the given pids.
>
> +--command-name::
> + Map output only for the given command name.
> + (** indicates other tasks while . is idle).
> +
> OPTIONS for 'perf sched timehist'
> ---------------------------------
> -k::
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 0fce7d8986c0..e60836da53e5 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -156,6 +156,7 @@ struct perf_sched_map {
> const char *color_pids_str;
> struct perf_cpu_map *color_cpus;
> const char *color_cpus_str;
> + const char *command;
> struct perf_cpu_map *cpus;
> const char *cpus_str;
> };
> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>
> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>
> - printf(" ");
> -
> new_shortname = 0;
> if (!tr->shortname[0]) {
> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> */
> tr->shortname[0] = '.';
> tr->shortname[1] = ' ';
> - } else {
> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
> + sched->map.command)) {
> tr->shortname[0] = sched->next_shortname1;
> tr->shortname[1] = sched->next_shortname2;
>
> @@ -1618,10 +1618,18 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> else
> sched->next_shortname2 = '0';
> }
> + } else {
> + tr->shortname[0] = '*';
> + tr->shortname[1] = '*';
> }
> new_shortname = 1;
> }
>
> + if (sched->map.command && strcmp(thread__comm_str(sched_in), sched->map.command))
> + goto skip;
> +
> + printf(" ");
> +
> for (i = 0; i < cpus_nr; i++) {
> struct perf_cpu cpu = {
> .cpu = sched->map.comp ? sched->map.comp_cpus[i].cpu : i,
> @@ -1678,6 +1686,7 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> out:
> color_fprintf(stdout, color, "\n");
>
> +skip:
> thread__put(sched_in);
>
> return 0;
> @@ -3560,6 +3569,8 @@ int cmd_sched(int argc, const char **argv)
> "highlight given CPUs in map"),
> OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
> "display given CPUs in map"),
> + OPT_STRING(0, "command-name", &sched.map.command, "command",
> + "map output only for the given command name"),
> OPT_PARENT(sched_options)
> };
> const struct option timehist_options[] = {
> --
> 2.39.1
>
>


2024-06-07 00:50:41

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map

Hello,

Sorry for the late reply.

On Wed, Apr 17, 2024 at 08:55:21PM +0530, Madadi Vineeth Reddy wrote:
> By default, perf sched map prints sched-in events for all the tasks
> which may not be required all the time as it prints lot of symbols
> and rows to the terminal.
>
> With --command-name option, one could specify the specific command
> for which the map has to be shown. This would help in analyzing the
> CPU usage patterns easier for that specific command. Since multiple
> PID's might have the same command name, using command-name filter
> would be more useful for debugging.
>
> For other tasks, instead of printing the symbol, ** is printed and
> the same . is used to represent idle. ** is used instead of symbol
> for other tasks because it helps in clear visualization of command
> of interest and secondly the symbol itself doesn't mean anything
> because the sched-in of that symbol will not be printed(first sched-in
> contains pid and the corresponding symbol).
>
> 6.8.0
> ======
> *A0 213864.670142 secs A0 => migration/0:18
> *. 213864.670148 secs . => swapper:0
> . *B0 213864.670217 secs B0 => migration/1:21
> . *. 213864.670223 secs
> . . *C0 213864.670247 secs C0 => migration/2:26
> . . *. 213864.670252 secs
>
> 6.8.0 + patch (--command-name = schbench)
> =============
> ** . ** *A0 213864.671055 secs A0 => schbench:104834
> *B0 . . A0 213864.671156 secs B0 => schbench:104835
> *C0 . . A0 213864.671187 secs C0 => schbench:104836
> *D0 . . A0 213864.671219 secs D0 => schbench:104837
> *E0 . . A0 213864.671250 secs E0 => schbench:104838
> E0 . *D0 A0
>
> This helps in visualizing how a benchmark like schbench is spread over
> the available cpus while also knowing which cpus are idle(.) and which
> are not(**). This will be more useful as number of CPUs increase.

Yeah I think this is good! Thanks for working on this.

But I guess people want to see when the tasks were sched out as well.
Can you please add that too?

>
> Signed-off-by: Madadi Vineeth Reddy <[email protected]>
> ---
> tools/perf/Documentation/perf-sched.txt | 4 ++++
> tools/perf/builtin-sched.c | 17 ++++++++++++++---
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
> index 5fbe42bd599b..b04a37560935 100644
> --- a/tools/perf/Documentation/perf-sched.txt
> +++ b/tools/perf/Documentation/perf-sched.txt
> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
> --color-pids::
> Highlight the given pids.
>
> +--command-name::
> + Map output only for the given command name.
> + (** indicates other tasks while . is idle).

Probably we can support multiple names in CSV.

Thanks,
Namhyung


> +
> OPTIONS for 'perf sched timehist'
> ---------------------------------
> -k::
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 0fce7d8986c0..e60836da53e5 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -156,6 +156,7 @@ struct perf_sched_map {
> const char *color_pids_str;
> struct perf_cpu_map *color_cpus;
> const char *color_cpus_str;
> + const char *command;
> struct perf_cpu_map *cpus;
> const char *cpus_str;
> };
> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>
> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>
> - printf(" ");
> -
> new_shortname = 0;
> if (!tr->shortname[0]) {
> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> */
> tr->shortname[0] = '.';
> tr->shortname[1] = ' ';
> - } else {
> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
> + sched->map.command)) {
> tr->shortname[0] = sched->next_shortname1;
> tr->shortname[1] = sched->next_shortname2;
>
> @@ -1618,10 +1618,18 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> else
> sched->next_shortname2 = '0';
> }
> + } else {
> + tr->shortname[0] = '*';
> + tr->shortname[1] = '*';
> }
> new_shortname = 1;
> }
>
> + if (sched->map.command && strcmp(thread__comm_str(sched_in), sched->map.command))
> + goto skip;
> +
> + printf(" ");
> +
> for (i = 0; i < cpus_nr; i++) {
> struct perf_cpu cpu = {
> .cpu = sched->map.comp ? sched->map.comp_cpus[i].cpu : i,
> @@ -1678,6 +1686,7 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> out:
> color_fprintf(stdout, color, "\n");
>
> +skip:
> thread__put(sched_in);
>
> return 0;
> @@ -3560,6 +3569,8 @@ int cmd_sched(int argc, const char **argv)
> "highlight given CPUs in map"),
> OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
> "display given CPUs in map"),
> + OPT_STRING(0, "command-name", &sched.map.command, "command",
> + "map output only for the given command name"),
> OPT_PARENT(sched_options)
> };
> const struct option timehist_options[] = {
> --
> 2.39.1
>

2024-06-07 05:50:07

by Madadi Vineeth Reddy

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map

Hi Namhyung,

On 07/06/24 06:20, Namhyung Kim wrote:
> Hello,
>
> Sorry for the late reply.
>
> On Wed, Apr 17, 2024 at 08:55:21PM +0530, Madadi Vineeth Reddy wrote:
>> By default, perf sched map prints sched-in events for all the tasks
>> which may not be required all the time as it prints lot of symbols
>> and rows to the terminal.
>>
>> With --command-name option, one could specify the specific command
>> for which the map has to be shown. This would help in analyzing the
>> CPU usage patterns easier for that specific command. Since multiple
>> PID's might have the same command name, using command-name filter
>> would be more useful for debugging.
>>
>> For other tasks, instead of printing the symbol, ** is printed and
>> the same . is used to represent idle. ** is used instead of symbol
>> for other tasks because it helps in clear visualization of command
>> of interest and secondly the symbol itself doesn't mean anything
>> because the sched-in of that symbol will not be printed(first sched-in
>> contains pid and the corresponding symbol).
>>
>> 6.8.0
>> ======
>> *A0 213864.670142 secs A0 => migration/0:18
>> *. 213864.670148 secs . => swapper:0
>> . *B0 213864.670217 secs B0 => migration/1:21
>> . *. 213864.670223 secs
>> . . *C0 213864.670247 secs C0 => migration/2:26
>> . . *. 213864.670252 secs
>>
>> 6.8.0 + patch (--command-name = schbench)
>> =============
>> ** . ** *A0 213864.671055 secs A0 => schbench:104834
>> *B0 . . A0 213864.671156 secs B0 => schbench:104835
>> *C0 . . A0 213864.671187 secs C0 => schbench:104836
>> *D0 . . A0 213864.671219 secs D0 => schbench:104837
>> *E0 . . A0 213864.671250 secs E0 => schbench:104838
>> E0 . *D0 A0
>>
>> This helps in visualizing how a benchmark like schbench is spread over
>> the available cpus while also knowing which cpus are idle(.) and which
>> are not(**). This will be more useful as number of CPUs increase.
>
> Yeah I think this is good! Thanks for working on this.
>
> But I guess people want to see when the tasks were sched out as well.
> Can you please add that too?
>

In the current implementation, we will know when a task is scheduled out
implicitly.

For instance, from the above example,

** . ** *A0 213864.671055 secs A0 => schbench:104834
*B0 . . A0 213864.671156 secs B0 => schbench:104835
*C0 . . A0 213864.671187 secs C0 => schbench:104836

In CPU0 (first column), the sched-in of C0 in last line indicates that
B0 is scheduled out. Similarly down the line if A0 is scheduled out,
we will know it by some other task being scheduled-in.

I hope this is fine. Let me know if you have any ideas or suggestions.

>>
>> Signed-off-by: Madadi Vineeth Reddy <[email protected]>
>> ---
>> tools/perf/Documentation/perf-sched.txt | 4 ++++
>> tools/perf/builtin-sched.c | 17 ++++++++++++++---
>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
>> index 5fbe42bd599b..b04a37560935 100644
>> --- a/tools/perf/Documentation/perf-sched.txt
>> +++ b/tools/perf/Documentation/perf-sched.txt
>> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
>> --color-pids::
>> Highlight the given pids.
>>
>> +--command-name::
>> + Map output only for the given command name.
>> + (** indicates other tasks while . is idle).
>
> Probably we can support multiple names in CSV.
>

Yes, that's a good idea. I will implement it in v2. Thank you for taking a look.

Thanks and Regards
Madadi Vineeth Reddy

> Thanks,
> Namhyung
>
>
>> +
>> OPTIONS for 'perf sched timehist'
>> ---------------------------------
>> -k::
>> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
>> index 0fce7d8986c0..e60836da53e5 100644
>> --- a/tools/perf/builtin-sched.c
>> +++ b/tools/perf/builtin-sched.c
>> @@ -156,6 +156,7 @@ struct perf_sched_map {
>> const char *color_pids_str;
>> struct perf_cpu_map *color_cpus;
>> const char *color_cpus_str;
>> + const char *command;
>> struct perf_cpu_map *cpus;
>> const char *cpus_str;
>> };
>> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>>
>> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>>
>> - printf(" ");
>> -
>> new_shortname = 0;
>> if (!tr->shortname[0]) {
>> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
>> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>> */
>> tr->shortname[0] = '.';
>> tr->shortname[1] = ' ';
>> - } else {
>> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
>> + sched->map.command)) {
>> tr->shortname[0] = sched->next_shortname1;
>> tr->shortname[1] = sched->next_shortname2;
>>
>> @@ -1618,10 +1618,18 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>> else
>> sched->next_shortname2 = '0';
>> }
>> + } else {
>> + tr->shortname[0] = '*';
>> + tr->shortname[1] = '*';
>> }
>> new_shortname = 1;
>> }
>>
>> + if (sched->map.command && strcmp(thread__comm_str(sched_in), sched->map.command))
>> + goto skip;
>> +
>> + printf(" ");
>> +
>> for (i = 0; i < cpus_nr; i++) {
>> struct perf_cpu cpu = {
>> .cpu = sched->map.comp ? sched->map.comp_cpus[i].cpu : i,
>> @@ -1678,6 +1686,7 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>> out:
>> color_fprintf(stdout, color, "\n");
>>
>> +skip:
>> thread__put(sched_in);
>>
>> return 0;
>> @@ -3560,6 +3569,8 @@ int cmd_sched(int argc, const char **argv)
>> "highlight given CPUs in map"),
>> OPT_STRING(0, "cpus", &sched.map.cpus_str, "cpus",
>> "display given CPUs in map"),
>> + OPT_STRING(0, "command-name", &sched.map.command, "command",
>> + "map output only for the given command name"),
>> OPT_PARENT(sched_options)
>> };
>> const struct option timehist_options[] = {
>> --
>> 2.39.1
>>


2024-06-07 06:51:17

by Chen Yu

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map

On 2024-04-17 at 20:55:21 +0530, Madadi Vineeth Reddy wrote:
> By default, perf sched map prints sched-in events for all the tasks
> which may not be required all the time as it prints lot of symbols
> and rows to the terminal.
>
> With --command-name option, one could specify the specific command
> for which the map has to be shown. This would help in analyzing the
> CPU usage patterns easier for that specific command. Since multiple
> PID's might have the same command name, using command-name filter
> would be more useful for debugging.
>
> For other tasks, instead of printing the symbol, ** is printed and
> the same . is used to represent idle. ** is used instead of symbol
> for other tasks because it helps in clear visualization of command
> of interest and secondly the symbol itself doesn't mean anything
> because the sched-in of that symbol will not be printed(first sched-in
> contains pid and the corresponding symbol).
>
> 6.8.0
> ======
> *A0 213864.670142 secs A0 => migration/0:18
> *. 213864.670148 secs . => swapper:0
> . *B0 213864.670217 secs B0 => migration/1:21
> . *. 213864.670223 secs
> . . *C0 213864.670247 secs C0 => migration/2:26
> . . *. 213864.670252 secs
>
> 6.8.0 + patch (--command-name = schbench)
> =============
> ** . ** *A0 213864.671055 secs A0 => schbench:104834
> *B0 . . A0 213864.671156 secs B0 => schbench:104835
> *C0 . . A0 213864.671187 secs C0 => schbench:104836
> *D0 . . A0 213864.671219 secs D0 => schbench:104837
> *E0 . . A0 213864.671250 secs E0 => schbench:104838
> E0 . *D0 A0
>
> This helps in visualizing how a benchmark like schbench is spread over
> the available cpus while also knowing which cpus are idle(.) and which
> are not(**). This will be more useful as number of CPUs increase.
>
> Signed-off-by: Madadi Vineeth Reddy <[email protected]>
> ---
> tools/perf/Documentation/perf-sched.txt | 4 ++++
> tools/perf/builtin-sched.c | 17 ++++++++++++++---
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
> index 5fbe42bd599b..b04a37560935 100644
> --- a/tools/perf/Documentation/perf-sched.txt
> +++ b/tools/perf/Documentation/perf-sched.txt
> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
> --color-pids::
> Highlight the given pids.
>
> +--command-name::
> + Map output only for the given command name.
> + (** indicates other tasks while . is idle).
> +
> OPTIONS for 'perf sched timehist'
> ---------------------------------
> -k::
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 0fce7d8986c0..e60836da53e5 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -156,6 +156,7 @@ struct perf_sched_map {
> const char *color_pids_str;
> struct perf_cpu_map *color_cpus;
> const char *color_cpus_str;
> + const char *command;
> struct perf_cpu_map *cpus;
> const char *cpus_str;
> };
> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>
> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>
> - printf(" ");
> -
> new_shortname = 0;
> if (!tr->shortname[0]) {
> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
> */
> tr->shortname[0] = '.';
> tr->shortname[1] = ' ';
> - } else {
> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
> + sched->map.command)) {
>

I've tested on my machine and it looks good. One minor question, can we do
fuzzy name matching? Say, there are many tasks 'a-taskname-b',
and we can filter them via
perf sched map --command-name=taskname

thanks,
Chenyu

2024-06-07 07:24:54

by Madadi Vineeth Reddy

[permalink] [raw]
Subject: Re: [PATCH] perf sched map: Add command-name option to filter the output map

Hi Chen Yu,

On 07/06/24 12:19, Chen Yu wrote:
> On 2024-04-17 at 20:55:21 +0530, Madadi Vineeth Reddy wrote:
>> By default, perf sched map prints sched-in events for all the tasks
>> which may not be required all the time as it prints lot of symbols
>> and rows to the terminal.
>>
>> With --command-name option, one could specify the specific command
>> for which the map has to be shown. This would help in analyzing the
>> CPU usage patterns easier for that specific command. Since multiple
>> PID's might have the same command name, using command-name filter
>> would be more useful for debugging.
>>
>> For other tasks, instead of printing the symbol, ** is printed and
>> the same . is used to represent idle. ** is used instead of symbol
>> for other tasks because it helps in clear visualization of command
>> of interest and secondly the symbol itself doesn't mean anything
>> because the sched-in of that symbol will not be printed(first sched-in
>> contains pid and the corresponding symbol).
>>
>> 6.8.0
>> ======
>> *A0 213864.670142 secs A0 => migration/0:18
>> *. 213864.670148 secs . => swapper:0
>> . *B0 213864.670217 secs B0 => migration/1:21
>> . *. 213864.670223 secs
>> . . *C0 213864.670247 secs C0 => migration/2:26
>> . . *. 213864.670252 secs
>>
>> 6.8.0 + patch (--command-name = schbench)
>> =============
>> ** . ** *A0 213864.671055 secs A0 => schbench:104834
>> *B0 . . A0 213864.671156 secs B0 => schbench:104835
>> *C0 . . A0 213864.671187 secs C0 => schbench:104836
>> *D0 . . A0 213864.671219 secs D0 => schbench:104837
>> *E0 . . A0 213864.671250 secs E0 => schbench:104838
>> E0 . *D0 A0
>>
>> This helps in visualizing how a benchmark like schbench is spread over
>> the available cpus while also knowing which cpus are idle(.) and which
>> are not(**). This will be more useful as number of CPUs increase.
>>
>> Signed-off-by: Madadi Vineeth Reddy <[email protected]>
>> ---
>> tools/perf/Documentation/perf-sched.txt | 4 ++++
>> tools/perf/builtin-sched.c | 17 ++++++++++++++---
>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
>> index 5fbe42bd599b..b04a37560935 100644
>> --- a/tools/perf/Documentation/perf-sched.txt
>> +++ b/tools/perf/Documentation/perf-sched.txt
>> @@ -94,6 +94,10 @@ OPTIONS for 'perf sched map'
>> --color-pids::
>> Highlight the given pids.
>>
>> +--command-name::
>> + Map output only for the given command name.
>> + (** indicates other tasks while . is idle).
>> +
>> OPTIONS for 'perf sched timehist'
>> ---------------------------------
>> -k::
>> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
>> index 0fce7d8986c0..e60836da53e5 100644
>> --- a/tools/perf/builtin-sched.c
>> +++ b/tools/perf/builtin-sched.c
>> @@ -156,6 +156,7 @@ struct perf_sched_map {
>> const char *color_pids_str;
>> struct perf_cpu_map *color_cpus;
>> const char *color_cpus_str;
>> + const char *command;
>> struct perf_cpu_map *cpus;
>> const char *cpus_str;
>> };
>> @@ -1594,8 +1595,6 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>>
>> sched->curr_thread[this_cpu.cpu] = thread__get(sched_in);
>>
>> - printf(" ");
>> -
>> new_shortname = 0;
>> if (!tr->shortname[0]) {
>> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
>> @@ -1605,7 +1604,8 @@ static int map_switch_event(struct perf_sched *sched, struct evsel *evsel,
>> */
>> tr->shortname[0] = '.';
>> tr->shortname[1] = ' ';
>> - } else {
>> + } else if (!sched->map.command || !strcmp(thread__comm_str(sched_in),
>> + sched->map.command)) {
>>
>
> I've tested on my machine and it looks good. One minor question, can we do
> fuzzy name matching? Say, there are many tasks 'a-taskname-b',
> and we can filter them via
> perf sched map --command-name=taskname

Yes, maybe I will try to find a way to do both fuzzy name matching and exact name matching.
Thanks for taking a look. I will implement it in v2.

Thanks and Regards
Madadi Vineeth Reddy

>
> thanks,
> Chenyu