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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAM9d7cjRJUfSPOAdVgcB=UHShPCxY8d3ryEZjzqdiuGhOKd4Lg@mail.gmail.com>
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ