Re: [PATCH v8 2/8] perf evlist: Introduce aux evlist

[Wangnan (F)]

On 2016/6/21 4:36, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 20, 2016 at 10:47:19AM +0000, Wang Nan escreveu:
> > An auxiliary evlist is created by perf_evlist__new_aux() using an
> > existing evlist as its parent. An auxiliary evlist can have its own
> > 'struct perf_mmap', but can't have any other data. User should use its
> > parent instead when accessing other data.
> >
> > Auxiliary evlists are containers of 'struct perf_mmap'. It is introduced
> > to allow its parent evlist to map different events into separated mmaps.
> >
> > Following commits create an auxiliary evlist for overwritable
> > events, because overwritable events need a read only and backwards ring
> > buffer, which is different from normal events.
> >
> > To achieve this goal, this patch carefully changes 'evlist' to
> > 'evlist->parent' in all functions in the path of 'perf_evlist__mmap_ex',
> > except 'evlist->mmap' related operations, to make sure all evlist
> > modifications (like pollfd and event id hash tables) goes to original
> > evlist.
> >
> > A 'evlist->parent' pointer is added to 'struct perf_evlist' and points to
> > the evlist itself for normal evlists.
> >
> > Children of one evlist are linked into it so one can find all children
> > from its parent.
> > To avoid potential complexity, forbid creating aux evlist from another
> > aux evlist.
> >
> > Improve perf_evlist__munmap_filtered(), so when recording, if an event
> > is terminated, unmap mmaps, from parent and children.
>   
> > Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> > Cc: He Kuang <hekuang@xxxxxxxxxx>
> > Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> > Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> > Cc: Zefan Li <lizefan@xxxxxxxxxx>
> > Cc: pi3orama@xxxxxxx
> > ---
> >   tools/perf/util/evlist.c | 49 +++++++++++++++++++++++++++++++++++++-----------
> >   tools/perf/util/evlist.h | 12 ++++++++++++
> >   2 files changed, 50 insertions(+), 11 deletions(-)

[SNIP]

> > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > index 68cb136..5b50692 100644
> > --- a/tools/perf/util/evlist.h
> > +++ b/tools/perf/util/evlist.h
> > @@ -37,6 +37,10 @@ struct perf_mmap {
> >   
> >   struct perf_evlist {
> >   	struct list_head entries;
> > +	union {
> > +		struct list_head children;
> > +		struct list_head list;
> This is something new, right, i.e. having a list of evlists from a
> parent evlist, one where there can be at most one level, i.e. no
> grandchildrens...
>
> Is this strictly needed? I.e. the existing code calling
> perf_evlist__munmap_filtered() has just the parent pointer and thus, to
> not change it we need to have a way to go the children?
>
> Also, can you se a parent with more than one child?

If we further restrict aux evlist and parent list, allow only one aux
evlist for a parent, we can avoid the linked list. Simply linking parent
and child using pointers would be enough.

However, I plan to introduce multiple children for a parent. There are
two possible aux evlists:

  1. Trigger aux evlist: we can use BPF script to measure events, and 
do something
     (dumpping, printing, computing, reporting...) when it detect something
     (for example, FPS drop to 10 frames/sec). We can create a bpf-output
     event and put it in a separated aux evlist with buffering turned off,
     so once BPF script generate a event perf receives it immediately.

  2. Non-sampling aux evlist: we can use a dummy event to collect fork, 
comm
     and mmap{,2} events in a separated aux evlist. We can use this 
evlist to
     maintain system state during recording, so we can totally solve 
the problem
     describe in patch 8/8.

Thank you.