linux-kernel - Re: [PATCH] perf: teach perf inject to merge sched_stat_* and sched

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: <20121016102529.GA6175@paralelels.com>
Date:	Tue, 16 Oct 2012 14:25:29 +0400
From:	Andrew Vagin <avagin@...allels.com>
To:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
CC:	Andrew Vagin <avagin@...nvz.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...hat.com>,
	Andi Kleen <andi@...stfloor.org>,
	David Ahern <dsahern@...il.com>
Subject: Re: [PATCH] perf: teach perf inject to merge sched_stat_* and
 sched_switch events (v3)

On Tue, Oct 16, 2012 at 01:27:13AM +0V400, Frederic Weisbecker wrote:
> 2012/9/18 Andrew Vagin <avagin@...nvz.org>:
> > You may want to know where and how long a task is sleeping. A callchain
> > may be found in sched_switch and a time slice in stat_iowait, so I add
> > handler in perf inject for merging this events.
> >
...
> 
> I'm ok with it, so Acked-by: Frederic Weisbecker <fweisbec@...il.com>
> 
> I just have some suggestions below.

Hello Arnaldo,

The fixed version of this patch is attached to this message.
All other patches of the series are in the branch "sleep" of your tree.

Could you move this series in a main branch for including to the
mainstream kernel?

Thanks.

> 
> > Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> > Cc: Paul Mackerras <paulus@...ba.org>,
> > Cc: Ingo Molnar <mingo@...hat.com
> > Cc: Andi Kleen <andi@...stfloor.org>
> > Cc: David Ahern <dsahern@...il.com>
> > Signed-off-by: Andrew Vagin <avagin@...nvz.org>
> > ---
> >  tools/perf/Documentation/perf-inject.txt |    4 ++
> >  tools/perf/builtin-inject.c              |   86 ++++++++++++++++++++++++++++++
> >  2 files changed, 90 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-inject.txt b/tools/perf/Documentation/perf-inject.txt
> > index 6be2101..c04e0c6 100644
> > --- a/tools/perf/Documentation/perf-inject.txt
> > +++ b/tools/perf/Documentation/perf-inject.txt
> > @@ -35,6 +35,10 @@ OPTIONS
> >  -o::
> >  --output=::
> >          Output file name. (default: stdout)
> > +-s::
> > +--sched-stat::
> > +       Merge sched_stat and sched_switch for getting events where and how long
> > +       tasks slept.
> 
> Please provide some more explanations here. I fear it's not very clear
> for the user. May be tell about the fact it results in sched_switch
> events weighted with the time slept.
> 
> [...]
> > +static int perf_event__sched_stat(struct perf_tool *tool,
> > +                                     union perf_event *event,
> > +                                     struct perf_sample *sample,
> > +                                     struct perf_evsel *evsel,
> > +                                     struct machine *machine)
> > +{
> > +       const char *evname = NULL;
> > +       uint32_t size;
> > +       struct event_entry *ent;
> > +       union perf_event *event_sw = NULL;
> > +       struct perf_sample sample_sw;
> > +       int sched_process_exit;
> > +
> > +       size = event->header.size;
> > +
> > +       evname = evsel->tp_format->name;
> > +
> > +       sched_process_exit = !strcmp(evname, "sched_process_exit");
> > +
> > +       if (!strcmp(evname, "sched_switch") ||  sched_process_exit) {
> > +               list_for_each_entry(ent, &samples, node)
> > +                       if (sample->tid == ent->tid)
> 
> Make sure you have PERF_SAMPLE_TID.
> 
> Thanks.

View attachment "0001-perf-teach-perf-inject-to-merge-sched_stat_-and-sche.patch" of type "text/plain" (6106 bytes)