| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 13 Jun 2024 20:43:10 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Madadi Vineeth Reddy <vineethr@...ux.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, Chen Yu <yu.c.chen@...el.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, acme@...hat.com,
linux-perf-users <linux-perf-users@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] perf sched map: Add command-name, fuzzy-name options
to filter the output map
Hello,
On Tue, Jun 11, 2024 at 1:47 AM Madadi Vineeth Reddy
<vineethr@...ux.ibm.com> wrote:
>
> Hi Namhyung,
>
> On 11/06/24 04:25, Namhyung Kim wrote:
> > Hello,
> >
> > On Sat, Jun 08, 2024 at 06:18:29PM +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(s)
> >> for which the map has to be shown. This would help in analyzing the
> >> CPU usage patterns easier for that specific command(s). Since multiple
> >> PID's might have the same command name, using command-name filter
> >> would be more useful for debugging.
> >>
> >> Multiple command names can be given with a comma separator without
> >> whitespace.
> >>
> >> The --fuzzy-name option can be used if fuzzy name matching is required.
> >> For example, "taskname" can be matched to any string that contains
> >> "taskname" as its substring.
> >>
> >> 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(s)
> >> 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.10.0-rc1
> >> ==========
> >> *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.10.0-rc1 + 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
> >
> > I still think some people are interested in sched-out time. For
> > example, we don't know when B0 was scheduled out in the above. There
> > could be other tasks between B0 and C0 on the CPU 0.
>
> Yes, you are right. When using the --command-name filter, there can be
> other tasks in between. This won't be a problem without the --command-name
> filtering, as no task will be missed, and we can be sure that the C0 sched-in
> time is the B0 sched-out time.
>
> I will add the sched-out time when using the --command-name option in v3.
>
> >
> >
> >> *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 <vineethr@...ux.ibm.com>
> >> Reviewed-and-tested-by: Athira Rajeev <atrajeev@...ux.vnet.ibm.com <mailto:atrajeev@...ux.vnet.ibm.com>>
> >>
> >> ---
[SNIP]
> >> @@ -1605,7 +1625,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 || command_matches(thread__comm_str(sched_in),
> >> + sched->map.command, sched->map.fuzzy)) {
> >
> > We usually align the indentation using the open parenthesis.
> > Maybe you can rename the function and pass the sched pointer directly
> > to reduce the argument.
>
> Sure, got it.
>
> >
> > bool sched_match_task(struct perf_sched *sched, const char *comm_str)
> > {
> > ...
> > }
> >
> > Or you could pass thread instead of comm_str and possibly support
> > matching with TID too.
>
> Do you want me to add another command line option to support matching
> with TID?
Maybe later.. if somebody wants. :)
Thanks,
Namhyung
>
> Thanks for all the suggestions. Will implement them and send v3.
>
> Thanks and Regards
> Madadi Vineeth Reddy
>
> >
> > Thanks,
> > Namhyung
> >
> >
> >> tr->shortname[0] = sched->next_shortname1;
> >> tr->shortname[1] = sched->next_shortname2;
> >>
> >> @@ -1618,10 +1639,19 @@ 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 && !command_matches(thread__comm_str(sched_in), sched->map.command,
> >> + sched->map.fuzzy))
> >> + 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 +1708,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 +3591,10 @@ 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(s)"),
> >> + OPT_BOOLEAN(0, "fuzzy-name", &sched.map.fuzzy,
> >> + "given command name can be partially matched (fuzzy matching)"),
> >> OPT_PARENT(sched_options)
> >> };
> >> const struct option timehist_options[] = {
> >> --
> >> 2.31.1
> >>
>
Powered by blists - more mailing lists