2014-10-06 21:26:13

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Heya, please check if this is OK.

This was while looking the set of methods used by rasd, trying to reduce
it to the bare minimum.

Perhaps even that cpu_map__new() one can be ditched, leaving to use the
default of a NULL cpumap that will end up being one with -1, i.e. all
cpus.

- Arnaldo

>From 8dc34bd2824c7843182f4fc6deabaf573e42e806 Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <[email protected]>
Date: Mon, 6 Oct 2014 15:43:42 -0300
Subject: [PATCH] rasd: Use perf_evlist__open() instead of open coded
equivalent

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
src/rasd.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/rasd.c b/src/rasd.c
index 06ccdcdd9d56..fb33fca131fa 100644
--- a/src/rasd.c
+++ b/src/rasd.c
@@ -241,7 +241,6 @@ static void daemonize(void)

int main()
{
- struct perf_evsel *c;
struct thread_map *threads;
struct cpu_map *cpus;
int i;
@@ -278,12 +277,9 @@ int main()
perf_evlist__set_maps(evlist, cpus, threads);

/* Open events */
- evlist__for_each(evlist, c) {
- /* On all online cpus by default, system wide tracing */
- if (perf_evsel__open(c, evlist->cpus, NULL) < 0)
- err("opening tracepoint, are you root?");
- }
- perf_evlist__set_id_pos(evlist);
+ /* On all online cpus by default, system wide tracing */
+ if (perf_evlist__open(evlist) < 0)
+ err("opening tracepoint, are you root?");

/* mmap buffers */
if (perf_evlist__mmap(evlist, 4 /* opts->mmap_pages */, false) < 0)
--
1.9.3


2014-10-07 08:45:31

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Hi Arnaldo,

On 6 October 2014 23:26, Arnaldo Carvalho de Melo <[email protected]> wrote:
> Heya, please check if this is OK.
>
> This was while looking the set of methods used by rasd, trying to reduce
> it to the bare minimum.
>
> Perhaps even that cpu_map__new() one can be ditched, leaving to use the
> default of a NULL cpumap that will end up being one with -1, i.e. all
> cpus.
>
> - Arnaldo
>
> From 8dc34bd2824c7843182f4fc6deabaf573e42e806 Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <[email protected]>
> Date: Mon, 6 Oct 2014 15:43:42 -0300
> Subject: [PATCH] rasd: Use perf_evlist__open() instead of open coded
> equivalent
>
> Cc: Borislav Petkov <[email protected]>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> src/rasd.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/src/rasd.c b/src/rasd.c
> index 06ccdcdd9d56..fb33fca131fa 100644
> --- a/src/rasd.c
> +++ b/src/rasd.c
> @@ -241,7 +241,6 @@ static void daemonize(void)
>
> int main()
> {
> - struct perf_evsel *c;
> struct thread_map *threads;
> struct cpu_map *cpus;
> int i;
> @@ -278,12 +277,9 @@ int main()
> perf_evlist__set_maps(evlist, cpus, threads);
>
> /* Open events */
> - evlist__for_each(evlist, c) {
> - /* On all online cpus by default, system wide tracing */
> - if (perf_evsel__open(c, evlist->cpus, NULL) < 0)
> - err("opening tracepoint, are you root?");
> - }
> - perf_evlist__set_id_pos(evlist);
> + /* On all online cpus by default, system wide tracing */
> + if (perf_evlist__open(evlist) < 0)
> + err("opening tracepoint, are you root?");

That way the system wide tracing does not work. perf_evlist__open uses
a non-NULL thread mapping and so it only traces the events generated
by the daemon itself.

perf top uses a similar approach but uses a lot of code in machine.c
(machine__synthesize_threads) and util/event.c
(perf_event__synthesize_threads) to synthesize the threads etc., which
we want to avoid in the minimalistic approach of rasd.

Maybe I missed something about the system wide tracing, any suggestion
is welcome.

Thanks for looking!

Jean

>
> /* mmap buffers */
> if (perf_evlist__mmap(evlist, 4 /* opts->mmap_pages */, false) < 0)
> --
> 1.9.3
>

2014-10-07 13:32:45

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Tue, Oct 07, 2014 at 10:45:14AM +0200, Jean Pihet escreveu:
> > --- a/src/rasd.c
> > - evlist__for_each(evlist, c) {
> > - /* On all online cpus by default, system wide tracing */
> > - if (perf_evsel__open(c, evlist->cpus, NULL) < 0)
> > - err("opening tracepoint, are you root?");
> > - }
> > - perf_evlist__set_id_pos(evlist);
> > + /* On all online cpus by default, system wide tracing */
> > + if (perf_evlist__open(evlist) < 0)
> > + err("opening tracepoint, are you root?");

> That way the system wide tracing does not work. perf_evlist__open uses
> a non-NULL thread mapping and so it only traces the events generated
> by the daemon itself.

> perf top uses a similar approach but uses a lot of code in machine.c
> (machine__synthesize_threads) and util/event.c
> (perf_event__synthesize_threads) to synthesize the threads etc., which
> we want to avoid in the minimalistic approach of rasd.

Humm, I will look into making it support this usecase, the point of
evsel/evlist is to try to abstract away as much as possible, leaving
just a few methods to be used by tools.

I think the default for perf_evlist__open() should be the most useful
for the majority of tools, which I thought would serve rasd well. It is
not the case, so I'll into how the tools that currently use
perf_evlist__open() behave and try to get a sane default in place.

Hopefully we will completely remove the need to set up any thread or cpu
map, as what you want is syswide tracing, right?

- Arnaldo

> Maybe I missed something about the system wide tracing, any suggestion
> is welcome.
>
> Thanks for looking!
>
> Jean
>
> >
> > /* mmap buffers */
> > if (perf_evlist__mmap(evlist, 4 /* opts->mmap_pages */, false) < 0)
> > --
> > 1.9.3
> >

2014-10-07 14:04:40

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Tue, Oct 07, 2014 at 10:32:36AM -0300, Arnaldo Carvalho de Melo wrote:
> Hopefully we will completely remove the need to set up any thread or cpu
> map, as what you want is syswide tracing, right?

Yeah, I was questioning the need to open at least one thread even for
system-wide tracing.

See, source libraries are the new great thing for collaborative
development - f*ck "cloud"!

:-D

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-10-07 14:17:19

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Tue, Oct 07, 2014 at 04:04:33PM +0200, Borislav Petkov escreveu:
> On Tue, Oct 07, 2014 at 10:32:36AM -0300, Arnaldo Carvalho de Melo wrote:
> > Hopefully we will completely remove the need to set up any thread or cpu
> > map, as what you want is syswide tracing, right?

> Yeah, I was questioning the need to open at least one thread even for
> system-wide tracing.

Right, evsel/evlist evolved out of tons of refactoring steps moving
stuff out of open coded tools (record, report, top) while trying to make
it work for those tools, sometimes this resulted in sub-optimal defaults
(or plain bugs) like needing a thread_map and/or a cpu_map to do syswide
tracing, need to get that clarified and sorted out.

- Arnaldo

> See, source libraries are the new great thing for collaborative
> development - f*ck "cloud"!

> :-D

:-)

- Arnaldo

2014-10-10 20:07:14

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Tue, Oct 07, 2014 at 04:04:33PM +0200, Borislav Petkov escreveu:
> On Tue, Oct 07, 2014 at 10:32:36AM -0300, Arnaldo Carvalho de Melo wrote:
> > Hopefully we will completely remove the need to set up any thread or cpu
> > map, as what you want is syswide tracing, right?
>
> Yeah, I was questioning the need to open at least one thread even for
> system-wide tracing.

Ok, so what is now at my perf/hists branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git

should do the trick, the patch below makes rasd.c use it. I'll try
building rasd.c with it and checking if it works.

Never having tried this, what are the requisites to test it? Some
specific hardware and a kernel with the right tracepoint? I guess some
recent 3.17-rc kernel is all that I need?

- Arnaldo

---

[PATCH] rasd: Use perf_evlist__open() instead of open coded equivalent

Cc: Borislav Petkov <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
src/rasd.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/src/rasd.c b/src/rasd.c
index 06ccdcdd9d56..d3df9868388c 100644
--- a/src/rasd.c
+++ b/src/rasd.c
@@ -241,9 +241,6 @@ static void daemonize(void)

int main()
{
- struct perf_evsel *c;
- struct thread_map *threads;
- struct cpu_map *cpus;
int i;

/* Run the process as a daemon */
@@ -266,24 +263,10 @@ int main()
if (read_config_file())
err("error reading config file");

- /* Set cpu and threads maps */
- threads = thread_map__new(-1, getpid(), UINT_MAX);
- if (threads == NULL)
- err("allocating threads_map\n");
-
- cpus = cpu_map__new(NULL);
- if (cpus == NULL)
- err("allocating cpu_map\n");
-
- perf_evlist__set_maps(evlist, cpus, threads);
-
/* Open events */
- evlist__for_each(evlist, c) {
- /* On all online cpus by default, system wide tracing */
- if (perf_evsel__open(c, evlist->cpus, NULL) < 0)
- err("opening tracepoint, are you root?");
- }
- perf_evlist__set_id_pos(evlist);
+ /* On all online cpus by default, system wide tracing */
+ if (perf_evlist__open(evlist) < 0)
+ err("opening tracepoint, are you root?");

/* mmap buffers */
if (perf_evlist__mmap(evlist, 4 /* opts->mmap_pages */, false) < 0)
--
1.9.3

2014-10-10 20:29:08

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Fri, Oct 10, 2014 at 05:07:08PM -0300, Arnaldo Carvalho de Melo wrote:
> Ok, so what is now at my perf/hists branch at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
>
> should do the trick, the patch below makes rasd.c use it.

Cool.

> I'll try lding rasd.c with it and checking if it works.
>
> Never having tried this, what are the requisites to test it? Some
> specific hardware and a kernel with the right tracepoint? I guess some
> recent 3.17-rc kernel is all that I need?

Well, you'd need the part of Jean's patches which adds the event to evlist:

https://lkml.kernel.org/r/[email protected]

AFAICT, you could apply patches 1-5 and replace 6 with yours. Now,
rasd.cfg has the mce:mce_record tracepoint which rasd opens but you
probably want to put a tracepoint which is much easier to exercise,
maybe some syscall or whatever.

I think that should do it but we won't know until we've tried it.

HTH and thanks a lot for doing this!

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-10-10 20:42:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Fri, Oct 10, 2014 at 10:28:54PM +0200, Borislav Petkov escreveu:
> On Fri, Oct 10, 2014 at 05:07:08PM -0300, Arnaldo Carvalho de Melo wrote:
> > I'll try lding rasd.c with it and checking if it works.

> > Never having tried this, what are the requisites to test it? Some
> > specific hardware and a kernel with the right tracepoint? I guess some
> > recent 3.17-rc kernel is all that I need?

> Well, you'd need the part of Jean's patches which adds the event to evlist:

> https://lkml.kernel.org/r/[email protected]

> AFAICT, you could apply patches 1-5 and replace 6 with yours. Now,
> rasd.cfg has the mce:mce_record tracepoint which rasd opens but you
> probably want to put a tracepoint which is much easier to exercise,
> maybe some syscall or whatever.

Right, stoopid me, no need for some specific tracepoint, just to see
that whatever tp it is, it will show up in "rasd"'s event loop. Ok, I'll
try that later.

Next stuff I probably will do is to move the bare minimum used by rasd
to tools/lib/api/perf/, i.e. there will be:

tools/lib/api/perf/evsel.c
tools/perf/util/evsel.c

Both will share the perf_evsel__ namespace (which I thought at some
point to make just: evsel__<METHOD_NAME>, wdyt?).

That way we just make public the bare minimum that already proved to be
useful outside tools/perf/ and over time we move stuff from
tools/perf/util/evsel.c (and from other tools in or out perf's repo)
into the lib.

> I think that should do it but we won't know until we've tried it.
>
> HTH and thanks a lot for doing this!

Np, had to be done at some point :)

- Arnaldo

> --
> Regards/Gruss,
> Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --

2014-10-10 20:44:45

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Fri, Oct 10, 2014 at 05:41:58PM -0300, Arnaldo Carvalho de Melo wrote:
> Right, stoopid me, no need for some specific tracepoint, just to see
> that whatever tp it is, it will show up in "rasd"'s event loop. Ok, I'll
> try that later.
>
> Next stuff I probably will do is to move the bare minimum used by rasd
> to tools/lib/api/perf/, i.e. there will be:
>
> tools/lib/api/perf/evsel.c
> tools/perf/util/evsel.c
>
> Both will share the perf_evsel__ namespace (which I thought at some
> point to make just: evsel__<METHOD_NAME>, wdyt?).
>
> That way we just make public the bare minimum that already proved to be
> useful outside tools/perf/ and over time we move stuff from
> tools/perf/util/evsel.c (and from other tools in or out perf's repo)
> into the lib.

Abso-f*cking-lutely!

:-D

> Np, had to be done at some point :)

Yeah, cool. Let me know what I should do to help out.

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-10-13 07:29:59

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Hi Arnaldo, Borislav,

On 10 October 2014 22:44, Borislav Petkov <[email protected]> wrote:
> On Fri, Oct 10, 2014 at 05:41:58PM -0300, Arnaldo Carvalho de Melo wrote:
>> Right, stoopid me, no need for some specific tracepoint, just to see
>> that whatever tp it is, it will show up in "rasd"'s event loop. Ok, I'll
>> try that later.
>>
>> Next stuff I probably will do is to move the bare minimum used by rasd
>> to tools/lib/api/perf/, i.e. there will be:
>>
>> tools/lib/api/perf/evsel.c
>> tools/perf/util/evsel.c
>>
>> Both will share the perf_evsel__ namespace (which I thought at some
>> point to make just: evsel__<METHOD_NAME>, wdyt?).
Makes perfectly sense

>>
>> That way we just make public the bare minimum that already proved to be
>> useful outside tools/perf/ and over time we move stuff from
>> tools/perf/util/evsel.c (and from other tools in or out perf's repo)
>> into the lib.
That is great!

About rasd usage, there are some details at
https://wiki.linaro.org/LEG/Engineering/Kernel/perfAndRAS#rasd_implementation.

Thx,
Jean

>
> Abso-f*cking-lutely!
>
> :-D
>
>> Np, had to be done at some point :)
>
> Yeah, cool. Let me know what I should do to help out.
>
> Thanks.
>
> --
> Regards/Gruss,
> Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --

2014-10-14 13:56:58

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Mon, Oct 13, 2014 at 09:29:57AM +0200, Jean Pihet wrote:
> Hi Arnaldo, Borislav,
>
> On 10 October 2014 22:44, Borislav Petkov <[email protected]> wrote:
> > On Fri, Oct 10, 2014 at 05:41:58PM -0300, Arnaldo Carvalho de Melo wrote:
> >> Right, stoopid me, no need for some specific tracepoint, just to see
> >> that whatever tp it is, it will show up in "rasd"'s event loop. Ok, I'll
> >> try that later.
> >>
> >> Next stuff I probably will do is to move the bare minimum used by rasd
> >> to tools/lib/api/perf/, i.e. there will be:
> >>
> >> tools/lib/api/perf/evsel.c
> >> tools/perf/util/evsel.c
> >>
> >> Both will share the perf_evsel__ namespace (which I thought at some
> >> point to make just: evsel__<METHOD_NAME>, wdyt?).
> Makes perfectly sense
>
> >>
> >> That way we just make public the bare minimum that already proved to be
> >> useful outside tools/perf/ and over time we move stuff from
> >> tools/perf/util/evsel.c (and from other tools in or out perf's repo)
> >> into the lib.
> That is great!
>
> About rasd usage, there are some details at
> https://wiki.linaro.org/LEG/Engineering/Kernel/perfAndRAS#rasd_implementation.

heya,
sorry for late reply.. I was on vacation last week..

I read the rasd sources and realized we could poke this
from another angle.. AFAIU the work the rasd does is following:
- reads config file and opens configured tracepoints
- reads samples comming from those tracepoints and displays/writes
this data to the console/file
- is there more?

If I'm not missing anything, this is quite usefull/common usage
pattern which would deserve new perf command.

I can see the analogy with ftrace debugfs interface
- choose/enable tracepoints
- cat .../tracing/trace-pipe

and there could be '-d' for the command to act as daemon.

thoughts? ;-)

jirka

2014-10-14 14:02:31

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Tue, Oct 14, 2014 at 03:56:31PM +0200, Jiri Olsa escreveu:
> On Mon, Oct 13, 2014 at 09:29:57AM +0200, Jean Pihet wrote:
> > On 10 October 2014 22:44, Borislav Petkov <[email protected]> wrote:
> > > On Fri, Oct 10, 2014 at 05:41:58PM -0300, Arnaldo Carvalho de Melo wrote:
> > >> That way we just make public the bare minimum that already proved to be
> > >> useful outside tools/perf/ and over time we move stuff from
> > >> tools/perf/util/evsel.c (and from other tools in or out perf's repo)
> > >> into the lib.
> > That is great!

> > About rasd usage, there are some details at
> > https://wiki.linaro.org/LEG/Engineering/Kernel/perfAndRAS#rasd_implementation.

> sorry for late reply.. I was on vacation last week..

> I read the rasd sources and realized we could poke this
> from another angle.. AFAIU the work the rasd does is following:
> - reads config file and opens configured tracepoints
> - reads samples comming from those tracepoints and displays/writes
> this data to the console/file
> - is there more?

> If I'm not missing anything, this is quite usefull/common usage
> pattern which would deserve new perf command.

> I can see the analogy with ftrace debugfs interface
> - choose/enable tracepoints
> - cat .../tracing/trace-pipe

> and there could be '-d' for the command to act as daemon.

Right, this is to be integrated into 'trace', i.e. to be able to ask for
more events, some with callchains, some without, etc.

We may even have something really bare bones that does what rasd.c does
right now, to show how one can write a tool using the exported
interfaces in tools/lib/api/, but I think that what rasd is _right now_,
is just an attempt to use the libraries using as few as possible apis,
right? I.e. more "meat" will be added there, no?

Anyway, its beeing an interesting exercise and will result in a lot of
untanglement, which is good in any case.

- Arnaldo

2014-10-14 14:19:56

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On 10/14/14, 7:56 AM, Jiri Olsa wrote:
> I read the rasd sources and realized we could poke this
> from another angle.. AFAIU the work the rasd does is following:
> - reads config file and opens configured tracepoints
> - reads samples comming from those tracepoints and displays/writes
> this data to the console/file
> - is there more?
>
> If I'm not missing anything, this is quite usefull/common usage
> pattern which would deserve new perf command.
>
> I can see the analogy with ftrace debugfs interface
> - choose/enable tracepoints
> - cat .../tracing/trace-pipe
>
> and there could be '-d' for the command to act as daemon.

Yes, this parallels a new use case on my end. Right now I am running
perf record ... | perf script. With the tracepoints and filters involved
it is a LOT of typing - and still collects more than is needed (I don't
need MMAP events for example, only COMM events). I am leaning towards a
new perf sub-command but from my scheduling timehist and daemon commands
I know there is a lot of overhead that goes with that. A perf library
with a stable API would make this a lot easier. (AFAIK the python
bindings do not currently support opening events, it is mainly an
analysis option.)

David

2014-10-14 14:22:39

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Tue, Oct 14, 2014 at 11:02:23AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 14, 2014 at 03:56:31PM +0200, Jiri Olsa escreveu:
> > On Mon, Oct 13, 2014 at 09:29:57AM +0200, Jean Pihet wrote:
> > > On 10 October 2014 22:44, Borislav Petkov <[email protected]> wrote:
> > > > On Fri, Oct 10, 2014 at 05:41:58PM -0300, Arnaldo Carvalho de Melo wrote:
> > > >> That way we just make public the bare minimum that already proved to be
> > > >> useful outside tools/perf/ and over time we move stuff from
> > > >> tools/perf/util/evsel.c (and from other tools in or out perf's repo)
> > > >> into the lib.
> > > That is great!
>
> > > About rasd usage, there are some details at
> > > https://wiki.linaro.org/LEG/Engineering/Kernel/perfAndRAS#rasd_implementation.
>
> > sorry for late reply.. I was on vacation last week..
>
> > I read the rasd sources and realized we could poke this
> > from another angle.. AFAIU the work the rasd does is following:
> > - reads config file and opens configured tracepoints
> > - reads samples comming from those tracepoints and displays/writes
> > this data to the console/file
> > - is there more?
>
> > If I'm not missing anything, this is quite usefull/common usage
> > pattern which would deserve new perf command.
>
> > I can see the analogy with ftrace debugfs interface
> > - choose/enable tracepoints
> > - cat .../tracing/trace-pipe
>
> > and there could be '-d' for the command to act as daemon.
>
> Right, this is to be integrated into 'trace', i.e. to be able to ask for
> more events, some with callchains, some without, etc.
>
> We may even have something really bare bones that does what rasd.c does
> right now, to show how one can write a tool using the exported
> interfaces in tools/lib/api/, but I think that what rasd is _right now_,
> is just an attempt to use the libraries using as few as possible apis,
> right? I.e. more "meat" will be added there, no?

right.. is there more planned for RAS daemon to do? because at this
point it looks to me more like new perf functionality (in trace or
some other new command)

>
> Anyway, its beeing an interesting exercise and will result in a lot of
> untanglement, which is good in any case.

yea, I dont mind untangling the interface.. I noticed in the thread
people want this anyway - simple 'events open' interface.. I often
need something simple for test programs

jirka

2014-10-14 15:17:24

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On Tue, Oct 14, 2014 at 04:22:13PM +0200, Jiri Olsa wrote:
> right.. is there more planned for RAS daemon to do? because at this
> point it looks to me more like new perf functionality (in trace or
> some other new command)
>
> >
> > Anyway, its beeing an interesting exercise and will result in a lot of
> > untanglement, which is good in any case.
>
> yea, I dont mind untangling the interface.. I noticed in the thread
> people want this anyway - simple 'events open' interface.. I often
> need something simple for test programs

Right, we simply need a way to get the tracepoint data into userspace.
Nothing else. And right now I cannot think about needing any other perf
tool functionality. But who knows.., if we need more, we'll untangle
more.

:-)

--
Regards/Gruss,
Boris.
--

2014-10-14 15:20:35

by Jean Pihet

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

On 14 October 2014 17:17, Borislav Petkov <[email protected]> wrote:
> On Tue, Oct 14, 2014 at 04:22:13PM +0200, Jiri Olsa wrote:
>> right.. is there more planned for RAS daemon to do? because at this
>> point it looks to me more like new perf functionality (in trace or
>> some other new command)
>>
>> >
>> > Anyway, its beeing an interesting exercise and will result in a lot of
>> > untanglement, which is good in any case.
>>
>> yea, I dont mind untangling the interface.. I noticed in the thread
>> people want this anyway - simple 'events open' interface.. I often
>> need something simple for test programs
>
> Right, we simply need a way to get the tracepoint data into userspace.
> Nothing else. And right now I cannot think about needing any other perf
> tool functionality. But who knows.., if we need more, we'll untangle
> more.
The persistent events stuff still needs some work and is needed by rasd.

>
> :-)
;-P

>
> --
> Regards/Gruss,
> Boris.
> --

Cheers,
Jean

2014-10-14 17:10:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/1] rasd: Use perf_evlist__open() instead of open coded

Em Tue, Oct 14, 2014 at 08:19:52AM -0600, David Ahern escreveu:
> On 10/14/14, 7:56 AM, Jiri Olsa wrote:
> >I read the rasd sources and realized we could poke this
> >from another angle.. AFAIU the work the rasd does is following:
> > - reads config file and opens configured tracepoints
> > - reads samples comming from those tracepoints and displays/writes
> > this data to the console/file
> > - is there more?
> >
> >If I'm not missing anything, this is quite usefull/common usage
> >pattern which would deserve new perf command.
> >
> >I can see the analogy with ftrace debugfs interface
> > - choose/enable tracepoints
> > - cat .../tracing/trace-pipe
> >
> >and there could be '-d' for the command to act as daemon.
>
> Yes, this parallels a new use case on my end. Right now I am running perf
> record ... | perf script. With the tracepoints and filters involved it is a
> LOT of typing - and still collects more than is needed (I don't need MMAP
> events for example, only COMM events). I am leaning towards a new perf
> sub-command but from my scheduling timehist and daemon commands I know there
> is a lot of overhead that goes with that. A perf library with a stable API
> would make this a lot easier. (AFAIK the python bindings do not currently
> support opening events, it is mainly an analysis option.)

Yeah, please take a look at this branch:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/hists

Trying to use the rasd.c opportunity to go over the boilerplate to
diminish it by having defaults for syswide tracing, etc.

I'm now working on moving the most minimalistic set of evsel.c/evlist.c
into tools/lib/api/perf/

- Arnaldo