Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-09 Thread Namhyung Kim 
Hi Adrian,

On Thu, Oct 08, 2015 at 07:07:43PM +0300, Adrian Hunter wrote:
> On 7/10/2015 12:06 p.m., Namhyung Kim wrote:
> >Hi Adrian,
> >
> >On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  
> >wrote:
> >>On 06/10/15 12:03, Namhyung Kim wrote:
> >>>Hi Adrian,
> >>>
> >>>On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  
> >>>wrote:
> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
> >Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> >>Since it's gonna share struct mmap with dummy tracking evsel to track
> >>meta events only, let's move auxtrace out of struct perf_mmap.
> >Is this moving around _strictly_ needed?
> 
> Also, what if you wanted to capture AUX data and tracking together.
> >>>
> >>>Hmm.. I don't know what's the problem.  It should be orthogonal and
> >>>support doing that together IMHO.  Maybe I'm missing something about
> >>>the aux data processing and Intel PT.  I'll take a look at it..
> >>>
> >>
> >>It is only orthogonal if you assume we will never want to support parallel
> >>processing with Intel PT.
> >
> >We'll definitely want it. :)
> >
> >>
> >>The only change that needs to be made is not to assume there is only 1
> >>tracking event.
> 
> Sorry for the slow reply.

No problem at all.  JFYI I'm travelling now.. :)


> 
> >
> >IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
> >track task/mmap and another is to track context switches.  The latter
> >is basically a light-weight version of the sched_switch event, right?
> 
> Yes
> 
> >
> >For parallel processing, each cpu needs to keep current thread to
> >synthesize events from auxtrace data.  So if it processed the switch
> >events before processing samples, it'd need to build long lists of
> >current thread per cpu.  IMHO it'd be better to process the switch
> >events with samples using multi-thread rather than processing them
> >prior to samples.
> 
> That is a good point.
> 
> But that would be limited to dividing the data by cpu.  It would be more
> useful to divide it any which way.  Does 'perf report' care if the
> data is not in order?

It doesn't as long as it could find a correct thread/dso/symbol ...

Btw I thought it'd also work if the targets are tasks since it'd still
be able to follow context switches of the tasks as switch events are
recorded along with the auxtrace events per task, no?

> 
> >So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
> >sched_switch) events.  The tracking dummy events would be recorded on
> >the tracking mmaps and switch (dummy) event would be recorded on the
> >main mmaps.  This way we can parallelize the auxtrace processing
> >without the list of current thread IMHO.
> >
> >Do I miss something?
> 
> Thinking about it now, it would probably make sense to put the AUX
> event with the tracking events as well, so the data can be queued up
> ready for processing, then the AUX index would not be needed.  But of
> course, if there were no other events, then there would be no main
> mmap at all.

Hmm.. let me try to follow. :)

So we can have 3 types of mmap in this case:

  1. track mmap for task/mmap events - it'll be saved in a separate
 file (in the meantime).
  2. main mmap for samples - it'll be saved in per-index (cpu or task)
 file.  For Intel PT, the switch events will be saved here too.
  3. auxtrace mmap - it'll be saved in per-index file (with switch events).

> 
> From that point of view, I guess I don't need to worry about splitting
> up the mmaps at all, just process them more than once if need be.

OK. I don't follow.. Can you elaborate it more?  Do you think it's not
necessary to use two dummy events?  What can be processed more than
once?

> 
> >
> >>
> >>IMHO there could be separate mmap_params also, which would allow for
> >>different mmap sizes for the tracking and main mmaps.
> >
> >Currently, the tracking mmap size is fixed at an arbitrary size
> >(128KiB) regardless of the main mmaps.  I can add an option to change
> >the tracking mmap size too.
> 
> I meant more from the program point of view, to allow different parameters.
> Such as allowing one mmap to be PROT_READ and the other PROT_READ|PROT_WRITE
> i.e. collect all the tracking events but let the other events overwrite
> - perhaps as some kind of snapshot mode like we do with Intel PT.

Ah, I see.

> 
> It seemed to me that it would be more flexible to put evsels into mmap
> groups.  Then those groups could have any events or be used in various ways.
> I also thought it might make the mmap code more readable, instead of having
> lots of "if tracking event do something different".

Hmm.. good idea.  I'll think about it.

> 
> On the other hand, it is just a thought.  As I mentioned above, I realized
> I could probably manage without splitting the mmaps.

It'd be nice if you'd explain your thoughts in more detail.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-09 Thread Namhyung Kim 
Hi Adrian,

On Thu, Oct 08, 2015 at 07:07:43PM +0300, Adrian Hunter wrote:
> On 7/10/2015 12:06 p.m., Namhyung Kim wrote:
> >Hi Adrian,
> >
> >On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  
> >wrote:
> >>On 06/10/15 12:03, Namhyung Kim wrote:
> >>>Hi Adrian,
> >>>
> >>>On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  
> >>>wrote:
> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
> >Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> >>Since it's gonna share struct mmap with dummy tracking evsel to track
> >>meta events only, let's move auxtrace out of struct perf_mmap.
> >Is this moving around _strictly_ needed?
> 
> Also, what if you wanted to capture AUX data and tracking together.
> >>>
> >>>Hmm.. I don't know what's the problem.  It should be orthogonal and
> >>>support doing that together IMHO.  Maybe I'm missing something about
> >>>the aux data processing and Intel PT.  I'll take a look at it..
> >>>
> >>
> >>It is only orthogonal if you assume we will never want to support parallel
> >>processing with Intel PT.
> >
> >We'll definitely want it. :)
> >
> >>
> >>The only change that needs to be made is not to assume there is only 1
> >>tracking event.
> 
> Sorry for the slow reply.

No problem at all.  JFYI I'm travelling now.. :)


> 
> >
> >IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
> >track task/mmap and another is to track context switches.  The latter
> >is basically a light-weight version of the sched_switch event, right?
> 
> Yes
> 
> >
> >For parallel processing, each cpu needs to keep current thread to
> >synthesize events from auxtrace data.  So if it processed the switch
> >events before processing samples, it'd need to build long lists of
> >current thread per cpu.  IMHO it'd be better to process the switch
> >events with samples using multi-thread rather than processing them
> >prior to samples.
> 
> That is a good point.
> 
> But that would be limited to dividing the data by cpu.  It would be more
> useful to divide it any which way.  Does 'perf report' care if the
> data is not in order?

It doesn't as long as it could find a correct thread/dso/symbol ...

Btw I thought it'd also work if the targets are tasks since it'd still
be able to follow context switches of the tasks as switch events are
recorded along with the auxtrace events per task, no?

> 
> >So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
> >sched_switch) events.  The tracking dummy events would be recorded on
> >the tracking mmaps and switch (dummy) event would be recorded on the
> >main mmaps.  This way we can parallelize the auxtrace processing
> >without the list of current thread IMHO.
> >
> >Do I miss something?
> 
> Thinking about it now, it would probably make sense to put the AUX
> event with the tracking events as well, so the data can be queued up
> ready for processing, then the AUX index would not be needed.  But of
> course, if there were no other events, then there would be no main
> mmap at all.

Hmm.. let me try to follow. :)

So we can have 3 types of mmap in this case:

  1. track mmap for task/mmap events - it'll be saved in a separate
 file (in the meantime).
  2. main mmap for samples - it'll be saved in per-index (cpu or task)
 file.  For Intel PT, the switch events will be saved here too.
  3. auxtrace mmap - it'll be saved in per-index file (with switch events).

> 
> From that point of view, I guess I don't need to worry about splitting
> up the mmaps at all, just process them more than once if need be.

OK. I don't follow.. Can you elaborate it more?  Do you think it's not
necessary to use two dummy events?  What can be processed more than
once?

> 
> >
> >>
> >>IMHO there could be separate mmap_params also, which would allow for
> >>different mmap sizes for the tracking and main mmaps.
> >
> >Currently, the tracking mmap size is fixed at an arbitrary size
> >(128KiB) regardless of the main mmaps.  I can add an option to change
> >the tracking mmap size too.
> 
> I meant more from the program point of view, to allow different parameters.
> Such as allowing one mmap to be PROT_READ and the other PROT_READ|PROT_WRITE
> i.e. collect all the tracking events but let the other events overwrite
> - perhaps as some kind of snapshot mode like we do with Intel PT.

Ah, I see.

> 
> It seemed to me that it would be more flexible to put evsels into mmap
> groups.  Then those groups could have any events or be used in various ways.
> I also thought it might make the mmap code more readable, instead of having
> lots of "if tracking event do something different".

Hmm.. good idea.  I'll think about it.

> 
> On the other hand, it is just a thought.  As I mentioned above, I realized
> I could probably manage without splitting the mmaps.

It'd be nice if you'd explain your thoughts in more detail.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-08 Thread Adrian Hunter 

On 7/10/2015 12:06 p.m., Namhyung Kim wrote:

Hi Adrian,

On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  wrote:

On 06/10/15 12:03, Namhyung Kim wrote:

Hi Adrian,

On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:

On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Since it's gonna share struct mmap with dummy tracking evsel to track
meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?


Also, what if you wanted to capture AUX data and tracking together.


Hmm.. I don't know what's the problem.  It should be orthogonal and
support doing that together IMHO.  Maybe I'm missing something about
the aux data processing and Intel PT.  I'll take a look at it..



It is only orthogonal if you assume we will never want to support parallel
processing with Intel PT.


We'll definitely want it. :)



The only change that needs to be made is not to assume there is only 1
tracking event.


Sorry for the slow reply.



IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
track task/mmap and another is to track context switches.  The latter
is basically a light-weight version of the sched_switch event, right?


Yes



For parallel processing, each cpu needs to keep current thread to
synthesize events from auxtrace data.  So if it processed the switch
events before processing samples, it'd need to build long lists of
current thread per cpu.  IMHO it'd be better to process the switch
events with samples using multi-thread rather than processing them
prior to samples.


That is a good point.

But that would be limited to dividing the data by cpu.  It would be more
useful to divide it any which way.  Does 'perf report' care if the
data is not in order?


So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
sched_switch) events.  The tracking dummy events would be recorded on
the tracking mmaps and switch (dummy) event would be recorded on the
main mmaps.  This way we can parallelize the auxtrace processing
without the list of current thread IMHO.

Do I miss something?


Thinking about it now, it would probably make sense to put the AUX
event with the tracking events as well, so the data can be queued up
ready for processing, then the AUX index would not be needed.  But of
course, if there were no other events, then there would be no main
mmap at all.

From that point of view, I guess I don't need to worry about splitting
up the mmaps at all, just process them more than once if need be.





IMHO there could be separate mmap_params also, which would allow for
different mmap sizes for the tracking and main mmaps.


Currently, the tracking mmap size is fixed at an arbitrary size
(128KiB) regardless of the main mmaps.  I can add an option to change
the tracking mmap size too.


I meant more from the program point of view, to allow different parameters.
Such as allowing one mmap to be PROT_READ and the other PROT_READ|PROT_WRITE
i.e. collect all the tracking events but let the other events overwrite
- perhaps as some kind of snapshot mode like we do with Intel PT.

It seemed to me that it would be more flexible to put evsels into mmap
groups.  Then those groups could have any events or be used in various ways.
I also thought it might make the mmap code more readable, instead of having
lots of "if tracking event do something different".

On the other hand, it is just a thought.  As I mentioned above, I realized
I could probably manage without splitting the mmaps.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-08 Thread Jiri Olsa 
On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.

after applying this one I got:

[jolsa@krava perf]$ ./perf record  kill
failed to mmap with 13 (Permission denied)

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-08 Thread Jiri Olsa 
On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.

after applying this one I got:

[jolsa@krava perf]$ ./perf record  kill
failed to mmap with 13 (Permission denied)

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-08 Thread Adrian Hunter 

On 7/10/2015 12:06 p.m., Namhyung Kim wrote:

Hi Adrian,

On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  wrote:

On 06/10/15 12:03, Namhyung Kim wrote:

Hi Adrian,

On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:

On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Since it's gonna share struct mmap with dummy tracking evsel to track
meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?


Also, what if you wanted to capture AUX data and tracking together.


Hmm.. I don't know what's the problem.  It should be orthogonal and
support doing that together IMHO.  Maybe I'm missing something about
the aux data processing and Intel PT.  I'll take a look at it..



It is only orthogonal if you assume we will never want to support parallel
processing with Intel PT.


We'll definitely want it. :)



The only change that needs to be made is not to assume there is only 1
tracking event.


Sorry for the slow reply.



IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
track task/mmap and another is to track context switches.  The latter
is basically a light-weight version of the sched_switch event, right?


Yes



For parallel processing, each cpu needs to keep current thread to
synthesize events from auxtrace data.  So if it processed the switch
events before processing samples, it'd need to build long lists of
current thread per cpu.  IMHO it'd be better to process the switch
events with samples using multi-thread rather than processing them
prior to samples.


That is a good point.

But that would be limited to dividing the data by cpu.  It would be more
useful to divide it any which way.  Does 'perf report' care if the
data is not in order?


So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
sched_switch) events.  The tracking dummy events would be recorded on
the tracking mmaps and switch (dummy) event would be recorded on the
main mmaps.  This way we can parallelize the auxtrace processing
without the list of current thread IMHO.

Do I miss something?


Thinking about it now, it would probably make sense to put the AUX
event with the tracking events as well, so the data can be queued up
ready for processing, then the AUX index would not be needed.  But of
course, if there were no other events, then there would be no main
mmap at all.

From that point of view, I guess I don't need to worry about splitting
up the mmaps at all, just process them more than once if need be.





IMHO there could be separate mmap_params also, which would allow for
different mmap sizes for the tracking and main mmaps.


Currently, the tracking mmap size is fixed at an arbitrary size
(128KiB) regardless of the main mmaps.  I can add an option to change
the tracking mmap size too.


I meant more from the program point of view, to allow different parameters.
Such as allowing one mmap to be PROT_READ and the other PROT_READ|PROT_WRITE
i.e. collect all the tracking events but let the other events overwrite
- perhaps as some kind of snapshot mode like we do with Intel PT.

It seemed to me that it would be more flexible to put evsels into mmap
groups.  Then those groups could have any events or be used in various ways.
I also thought it might make the mmap code more readable, instead of having
lots of "if tracking event do something different".

On the other hand, it is just a thought.  As I mentioned above, I realized
I could probably manage without splitting the mmaps.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-07 Thread Namhyung Kim 
Hi Adrian,

On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  wrote:
> On 06/10/15 12:03, Namhyung Kim wrote:
>> Hi Adrian,
>>
>> On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  
>> wrote:
>>> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
 Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.
 Is this moving around _strictly_ needed?
>>>
>>> Also, what if you wanted to capture AUX data and tracking together.
>>
>> Hmm.. I don't know what's the problem.  It should be orthogonal and
>> support doing that together IMHO.  Maybe I'm missing something about
>> the aux data processing and Intel PT.  I'll take a look at it..
>>
>
> It is only orthogonal if you assume we will never want to support parallel
> processing with Intel PT.

We'll definitely want it. :)

>
> The only change that needs to be made is not to assume there is only 1
> tracking event.

IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
track task/mmap and another is to track context switches.  The latter
is basically a light-weight version of the sched_switch event, right?

For parallel processing, each cpu needs to keep current thread to
synthesize events from auxtrace data.  So if it processed the switch
events before processing samples, it'd need to build long lists of
current thread per cpu.  IMHO it'd be better to process the switch
events with samples using multi-thread rather than processing them
prior to samples.

So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
sched_switch) events.  The tracking dummy events would be recorded on
the tracking mmaps and switch (dummy) event would be recorded on the
main mmaps.  This way we can parallelize the auxtrace processing
without the list of current thread IMHO.

Do I miss something?

>
> IMHO there could be separate mmap_params also, which would allow for
> different mmap sizes for the tracking and main mmaps.

Currently, the tracking mmap size is fixed at an arbitrary size
(128KiB) regardless of the main mmaps.  I can add an option to change
the tracking mmap size too.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-07 Thread Namhyung Kim 
Hi Adrian,

On Tue, Oct 6, 2015 at 6:26 PM, Adrian Hunter  wrote:
> On 06/10/15 12:03, Namhyung Kim wrote:
>> Hi Adrian,
>>
>> On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  
>> wrote:
>>> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
 Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.
 Is this moving around _strictly_ needed?
>>>
>>> Also, what if you wanted to capture AUX data and tracking together.
>>
>> Hmm.. I don't know what's the problem.  It should be orthogonal and
>> support doing that together IMHO.  Maybe I'm missing something about
>> the aux data processing and Intel PT.  I'll take a look at it..
>>
>
> It is only orthogonal if you assume we will never want to support parallel
> processing with Intel PT.

We'll definitely want it. :)

>
> The only change that needs to be made is not to assume there is only 1
> tracking event.

IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to
track task/mmap and another is to track context switches.  The latter
is basically a light-weight version of the sched_switch event, right?

For parallel processing, each cpu needs to keep current thread to
synthesize events from auxtrace data.  So if it processed the switch
events before processing samples, it'd need to build long lists of
current thread per cpu.  IMHO it'd be better to process the switch
events with samples using multi-thread rather than processing them
prior to samples.

So how about this?  It'd use *always* 2 dummy (or 1 dummy + 1
sched_switch) events.  The tracking dummy events would be recorded on
the tracking mmaps and switch (dummy) event would be recorded on the
main mmaps.  This way we can parallelize the auxtrace processing
without the list of current thread IMHO.

Do I miss something?

>
> IMHO there could be separate mmap_params also, which would allow for
> different mmap sizes for the tracking and main mmaps.

Currently, the tracking mmap size is fixed at an arbitrary size
(128KiB) regardless of the main mmaps.  I can add an option to change
the tracking mmap size too.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Adrian Hunter 
On 06/10/15 12:03, Namhyung Kim wrote:
> Hi Adrian,
> 
> On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:
>> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
 Since it's gonna share struct mmap with dummy tracking evsel to track
 meta events only, let's move auxtrace out of struct perf_mmap.
>>> Is this moving around _strictly_ needed?
>>
>> Also, what if you wanted to capture AUX data and tracking together.
> 
> Hmm.. I don't know what's the problem.  It should be orthogonal and
> support doing that together IMHO.  Maybe I'm missing something about
> the aux data processing and Intel PT.  I'll take a look at it..
> 

It is only orthogonal if you assume we will never want to support parallel
processing with Intel PT.

The only change that needs to be made is not to assume there is only 1
tracking event.

IMHO there could be separate mmap_params also, which would allow for
different mmap sizes for the tracking and main mmaps.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
Hi Adrian,

On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:
> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>>> Since it's gonna share struct mmap with dummy tracking evsel to track
>>> meta events only, let's move auxtrace out of struct perf_mmap.
>> Is this moving around _strictly_ needed?
>
> Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem.  It should be orthogonal and
support doing that together IMHO.  Maybe I'm missing something about
the aux data processing and Intel PT.  I'll take a look at it..


>
> In addition, currently Intel PT can have either 1 dummy event for tracking
> plus sched_switch
> or 2 dummy events to allow for system-wide tracking of context switches.
> i.e. there
> are multiple tracking events.

Again, I don't have an idea what's going on this area. I need to look
at the code and think about what I can do.

Thanks for your review!
Namhyung



>
>>
>> - Arnaldo
>>
>>> Cc: Adrian Hunter 
>>> Signed-off-by: Namhyung Kim 
>>> ---
>>>  tools/perf/builtin-record.c |  4 ++--
>>>  tools/perf/util/evlist.c| 30 +-
>>>  tools/perf/util/evlist.h|  2 +-
>>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index 5e01c070dbf2..0accac6e0812 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
>>> record *rec)
>>>
>>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>>  struct auxtrace_mmap *mm =
>>> ->evlist->mmap[i].auxtrace_mmap;
>>> +>evlist->auxtrace_mmap[i];
>>>
>>>  if (!mm->base)
>>>  continue;
>>> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>>>  int rc = 0;
>>>
>>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>> -struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
>>> +struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>>>
>>>  if (rec->evlist->mmap[i].base) {
>>>  if (record__mmap_read(rec, i) != 0) {
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index e46adcd5b408..042dffc67986 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
>>> perf_evlist *evlist, int idx)
>>>  return event;
>>>  }
>>>
>>> -static bool perf_mmap__empty(struct perf_mmap *md)
>>> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>>>  {
>>> -return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
>>> +struct perf_mmap *md = >mmap[idx];
>>> +
>>> +return perf_mmap__read_head(md) == md->prev &&
>>> +evlist->auxtrace_mmap[idx].base == NULL;
>>>  }
>>>
>>>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
>>> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
>>> *evlist, int idx)
>>>  perf_mmap__write_tail(md, old);
>>>  }
>>>
>>> -if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
>>> +if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
>>> idx))
>>>  perf_evlist__mmap_put(evlist, idx);
>>>  }
>>>
>>> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
>>> *evlist, int idx)
>>>  evlist->mmap[idx].base = NULL;
>>>  atomic_set(>mmap[idx].refcnt, 0);
>>>  }
>>> -auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
>>> +auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>>>  }
>>>
>>>  void perf_evlist__munmap(struct perf_evlist *evlist)
>>> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
>>> *evlist)
>>>  if (cpu_map__empty(evlist->cpus))
>>>  evlist->nr_mmaps = thread_map__nr(evlist->threads);
>>>  evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
>>> -return evlist->mmap != NULL ? 0 : -ENOMEM;
>>> +if (evlist->mmap == NULL)
>>> +return -ENOMEM;
>>> +evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
>>> +   sizeof(struct auxtrace_mmap));
>>> +if (evlist->auxtrace_mmap == NULL) {
>>> +zfree(>mmap);
>>> +return -ENOMEM;
>>> +}
>>> +return 0;
>>>  }
>>>
>>>  struct mmap_params {
>>> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
>>> *evlist, int idx,
>>>  return -1;
>>>  }
>>>
>>> -if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
>>> ->auxtrace_mp, evlist->mmap[idx].base, fd))
>>> -return -1;
>>> -
>>>  return 0;
>>>  }
>>>
>>> @@ -963,6 +970,11 @@ static int

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
On Sat, Oct 3, 2015 at 3:45 AM, Arnaldo Carvalho de Melo
 wrote:
> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>> Since it's gonna share struct mmap with dummy tracking evsel to track
>> meta events only, let's move auxtrace out of struct perf_mmap.
>
> Is this moving around _strictly_ needed?

In the later patch, I added another perf_mmap instance for dummy
tracking events. So keeping auxtrace_mmap in the perf_mmap is a
duplication.  It's not strictly needed but will waste some memory.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
On Mon, Oct 05, 2015 at 03:14:34PM +0200, Jiri Olsa wrote:
> On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> > *evlist, int idx)
> > perf_mmap__write_tail(md, old);
> > }
> >  
> > -   if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> > +   if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> > idx))
> > perf_evlist__mmap_put(evlist, idx);
> >  }
> >  
> > @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> > *evlist, int idx)
> > evlist->mmap[idx].base = NULL;
> > atomic_set(>mmap[idx].refcnt, 0);
> > }
> > -   auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> > +   auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
> >  }
> >  
> >  void perf_evlist__munmap(struct perf_evlist *evlist)
> > @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> > *evlist)
> > if (cpu_map__empty(evlist->cpus))
> > evlist->nr_mmaps = thread_map__nr(evlist->threads);
> > evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> > -   return evlist->mmap != NULL ? 0 : -ENOMEM;
> > +   if (evlist->mmap == NULL)
> > +   return -ENOMEM;
> > +   evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> > +  sizeof(struct auxtrace_mmap));
> > +   if (evlist->auxtrace_mmap == NULL) {
> > +   zfree(>mmap);
> > +   return -ENOMEM;
> > +   }
> 
> can't see evlist->auxtrace_mmap being freed 

Ooops, will add.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
On Mon, Oct 05, 2015 at 03:14:34PM +0200, Jiri Olsa wrote:
> On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> > *evlist, int idx)
> > perf_mmap__write_tail(md, old);
> > }
> >  
> > -   if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> > +   if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> > idx))
> > perf_evlist__mmap_put(evlist, idx);
> >  }
> >  
> > @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> > *evlist, int idx)
> > evlist->mmap[idx].base = NULL;
> > atomic_set(>mmap[idx].refcnt, 0);
> > }
> > -   auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> > +   auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
> >  }
> >  
> >  void perf_evlist__munmap(struct perf_evlist *evlist)
> > @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> > *evlist)
> > if (cpu_map__empty(evlist->cpus))
> > evlist->nr_mmaps = thread_map__nr(evlist->threads);
> > evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> > -   return evlist->mmap != NULL ? 0 : -ENOMEM;
> > +   if (evlist->mmap == NULL)
> > +   return -ENOMEM;
> > +   evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> > +  sizeof(struct auxtrace_mmap));
> > +   if (evlist->auxtrace_mmap == NULL) {
> > +   zfree(>mmap);
> > +   return -ENOMEM;
> > +   }
> 
> can't see evlist->auxtrace_mmap being freed 

Ooops, will add.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
Hi Adrian,

On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:
> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>>> Since it's gonna share struct mmap with dummy tracking evsel to track
>>> meta events only, let's move auxtrace out of struct perf_mmap.
>> Is this moving around _strictly_ needed?
>
> Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem.  It should be orthogonal and
support doing that together IMHO.  Maybe I'm missing something about
the aux data processing and Intel PT.  I'll take a look at it..


>
> In addition, currently Intel PT can have either 1 dummy event for tracking
> plus sched_switch
> or 2 dummy events to allow for system-wide tracking of context switches.
> i.e. there
> are multiple tracking events.

Again, I don't have an idea what's going on this area. I need to look
at the code and think about what I can do.

Thanks for your review!
Namhyung



>
>>
>> - Arnaldo
>>
>>> Cc: Adrian Hunter 
>>> Signed-off-by: Namhyung Kim 
>>> ---
>>>  tools/perf/builtin-record.c |  4 ++--
>>>  tools/perf/util/evlist.c| 30 +-
>>>  tools/perf/util/evlist.h|  2 +-
>>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index 5e01c070dbf2..0accac6e0812 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
>>> record *rec)
>>>
>>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>>  struct auxtrace_mmap *mm =
>>> ->evlist->mmap[i].auxtrace_mmap;
>>> +>evlist->auxtrace_mmap[i];
>>>
>>>  if (!mm->base)
>>>  continue;
>>> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>>>  int rc = 0;
>>>
>>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>> -struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
>>> +struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>>>
>>>  if (rec->evlist->mmap[i].base) {
>>>  if (record__mmap_read(rec, i) != 0) {
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index e46adcd5b408..042dffc67986 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
>>> perf_evlist *evlist, int idx)
>>>  return event;
>>>  }
>>>
>>> -static bool perf_mmap__empty(struct perf_mmap *md)
>>> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>>>  {
>>> -return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
>>> +struct perf_mmap *md = >mmap[idx];
>>> +
>>> +return perf_mmap__read_head(md) == md->prev &&
>>> +evlist->auxtrace_mmap[idx].base == NULL;
>>>  }
>>>
>>>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
>>> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
>>> *evlist, int idx)
>>>  perf_mmap__write_tail(md, old);
>>>  }
>>>
>>> -if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
>>> +if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
>>> idx))
>>>  perf_evlist__mmap_put(evlist, idx);
>>>  }
>>>
>>> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
>>> *evlist, int idx)
>>>  evlist->mmap[idx].base = NULL;
>>>  atomic_set(>mmap[idx].refcnt, 0);
>>>  }
>>> -auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
>>> +auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>>>  }
>>>
>>>  void perf_evlist__munmap(struct perf_evlist *evlist)
>>> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
>>> *evlist)
>>>  if (cpu_map__empty(evlist->cpus))
>>>  evlist->nr_mmaps = thread_map__nr(evlist->threads);
>>>  evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
>>> -return evlist->mmap != NULL ? 0 : -ENOMEM;
>>> +if (evlist->mmap == NULL)
>>> +return -ENOMEM;
>>> +evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
>>> +   sizeof(struct auxtrace_mmap));
>>> +if (evlist->auxtrace_mmap == NULL) {
>>> +zfree(>mmap);
>>> +return -ENOMEM;
>>> +}
>>> +return 0;
>>>  }
>>>
>>>  struct mmap_params {
>>> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
>>> *evlist, int idx,
>>>  return -1;
>>>  }
>>>
>>> -if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
>>> ->auxtrace_mp, evlist->mmap[idx].base, fd))
>>> -return -1;
>>> -
>>>  return 0;

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Namhyung Kim 
On Sat, Oct 3, 2015 at 3:45 AM, Arnaldo Carvalho de Melo
 wrote:
> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>> Since it's gonna share struct mmap with dummy tracking evsel to track
>> meta events only, let's move auxtrace out of struct perf_mmap.
>
> Is this moving around _strictly_ needed?

In the later patch, I added another perf_mmap instance for dummy
tracking events. So keeping auxtrace_mmap in the perf_mmap is a
duplication.  It's not strictly needed but will waste some memory.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-06 Thread Adrian Hunter 
On 06/10/15 12:03, Namhyung Kim wrote:
> Hi Adrian,
> 
> On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter  wrote:
>> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
>>> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
 Since it's gonna share struct mmap with dummy tracking evsel to track
 meta events only, let's move auxtrace out of struct perf_mmap.
>>> Is this moving around _strictly_ needed?
>>
>> Also, what if you wanted to capture AUX data and tracking together.
> 
> Hmm.. I don't know what's the problem.  It should be orthogonal and
> support doing that together IMHO.  Maybe I'm missing something about
> the aux data processing and Intel PT.  I'll take a look at it..
> 

It is only orthogonal if you assume we will never want to support parallel
processing with Intel PT.

The only change that needs to be made is not to assume there is only 1
tracking event.

IMHO there could be separate mmap_params also, which would allow for
different mmap sizes for the tracking and main mmaps.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-05 Thread Jiri Olsa 
On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:

SNIP

> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> *evlist, int idx)
>   perf_mmap__write_tail(md, old);
>   }
>  
> - if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> + if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> idx))
>   perf_evlist__mmap_put(evlist, idx);
>  }
>  
> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> *evlist, int idx)
>   evlist->mmap[idx].base = NULL;
>   atomic_set(>mmap[idx].refcnt, 0);
>   }
> - auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> + auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>  }
>  
>  void perf_evlist__munmap(struct perf_evlist *evlist)
> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> *evlist)
>   if (cpu_map__empty(evlist->cpus))
>   evlist->nr_mmaps = thread_map__nr(evlist->threads);
>   evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> - return evlist->mmap != NULL ? 0 : -ENOMEM;
> + if (evlist->mmap == NULL)
> + return -ENOMEM;
> + evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> +sizeof(struct auxtrace_mmap));
> + if (evlist->auxtrace_mmap == NULL) {
> + zfree(>mmap);
> + return -ENOMEM;
> + }

can't see evlist->auxtrace_mmap being freed 

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-05 Thread Adrian Hunter 
On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>> Since it's gonna share struct mmap with dummy tracking evsel to track
>> meta events only, let's move auxtrace out of struct perf_mmap.
> Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

In addition, currently Intel PT can have either 1 dummy event for tracking
plus sched_switch
or 2 dummy events to allow for system-wide tracking of context switches.
i.e. there
are multiple tracking events.

>
> - Arnaldo
>  
>> Cc: Adrian Hunter 
>> Signed-off-by: Namhyung Kim 
>> ---
>>  tools/perf/builtin-record.c |  4 ++--
>>  tools/perf/util/evlist.c| 30 +-
>>  tools/perf/util/evlist.h|  2 +-
>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 5e01c070dbf2..0accac6e0812 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
>> record *rec)
>>  
>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>  struct auxtrace_mmap *mm =
>> ->evlist->mmap[i].auxtrace_mmap;
>> +>evlist->auxtrace_mmap[i];
>>  
>>  if (!mm->base)
>>  continue;
>> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>>  int rc = 0;
>>  
>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>> -struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
>> +struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>>  
>>  if (rec->evlist->mmap[i].base) {
>>  if (record__mmap_read(rec, i) != 0) {
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index e46adcd5b408..042dffc67986 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
>> perf_evlist *evlist, int idx)
>>  return event;
>>  }
>>  
>> -static bool perf_mmap__empty(struct perf_mmap *md)
>> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>>  {
>> -return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
>> +struct perf_mmap *md = >mmap[idx];
>> +
>> +return perf_mmap__read_head(md) == md->prev &&
>> +evlist->auxtrace_mmap[idx].base == NULL;
>>  }
>>  
>>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
>> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
>> *evlist, int idx)
>>  perf_mmap__write_tail(md, old);
>>  }
>>  
>> -if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
>> +if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
>> idx))
>>  perf_evlist__mmap_put(evlist, idx);
>>  }
>>  
>> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
>> *evlist, int idx)
>>  evlist->mmap[idx].base = NULL;
>>  atomic_set(>mmap[idx].refcnt, 0);
>>  }
>> -auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
>> +auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>>  }
>>  
>>  void perf_evlist__munmap(struct perf_evlist *evlist)
>> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
>> *evlist)
>>  if (cpu_map__empty(evlist->cpus))
>>  evlist->nr_mmaps = thread_map__nr(evlist->threads);
>>  evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
>> -return evlist->mmap != NULL ? 0 : -ENOMEM;
>> +if (evlist->mmap == NULL)
>> +return -ENOMEM;
>> +evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
>> +   sizeof(struct auxtrace_mmap));
>> +if (evlist->auxtrace_mmap == NULL) {
>> +zfree(>mmap);
>> +return -ENOMEM;
>> +}
>> +return 0;
>>  }
>>  
>>  struct mmap_params {
>> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
>> *evlist, int idx,
>>  return -1;
>>  }
>>  
>> -if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
>> ->auxtrace_mp, evlist->mmap[idx].base, fd))
>> -return -1;
>> -
>>  return 0;
>>  }
>>  
>> @@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct 
>> perf_evlist *evlist, int idx,
>>  *output = fd;
>>  if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
>>  return -1;
>> +
>> +if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
>> +>auxtrace_mp,
>> +evlist->mmap[idx].base, fd))
>> +return -1;
>>  } else {
>>  if (ioctl(fd,

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-05 Thread Adrian Hunter 
On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>> Since it's gonna share struct mmap with dummy tracking evsel to track
>> meta events only, let's move auxtrace out of struct perf_mmap.
> Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

In addition, currently Intel PT can have either 1 dummy event for tracking
plus sched_switch
or 2 dummy events to allow for system-wide tracking of context switches.
i.e. there
are multiple tracking events.

>
> - Arnaldo
>  
>> Cc: Adrian Hunter 
>> Signed-off-by: Namhyung Kim 
>> ---
>>  tools/perf/builtin-record.c |  4 ++--
>>  tools/perf/util/evlist.c| 30 +-
>>  tools/perf/util/evlist.h|  2 +-
>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 5e01c070dbf2..0accac6e0812 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
>> record *rec)
>>  
>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>  struct auxtrace_mmap *mm =
>> ->evlist->mmap[i].auxtrace_mmap;
>> +>evlist->auxtrace_mmap[i];
>>  
>>  if (!mm->base)
>>  continue;
>> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>>  int rc = 0;
>>  
>>  for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>> -struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
>> +struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>>  
>>  if (rec->evlist->mmap[i].base) {
>>  if (record__mmap_read(rec, i) != 0) {
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index e46adcd5b408..042dffc67986 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
>> perf_evlist *evlist, int idx)
>>  return event;
>>  }
>>  
>> -static bool perf_mmap__empty(struct perf_mmap *md)
>> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>>  {
>> -return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
>> +struct perf_mmap *md = >mmap[idx];
>> +
>> +return perf_mmap__read_head(md) == md->prev &&
>> +evlist->auxtrace_mmap[idx].base == NULL;
>>  }
>>  
>>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
>> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
>> *evlist, int idx)
>>  perf_mmap__write_tail(md, old);
>>  }
>>  
>> -if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
>> +if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
>> idx))
>>  perf_evlist__mmap_put(evlist, idx);
>>  }
>>  
>> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
>> *evlist, int idx)
>>  evlist->mmap[idx].base = NULL;
>>  atomic_set(>mmap[idx].refcnt, 0);
>>  }
>> -auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
>> +auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>>  }
>>  
>>  void perf_evlist__munmap(struct perf_evlist *evlist)
>> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
>> *evlist)
>>  if (cpu_map__empty(evlist->cpus))
>>  evlist->nr_mmaps = thread_map__nr(evlist->threads);
>>  evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
>> -return evlist->mmap != NULL ? 0 : -ENOMEM;
>> +if (evlist->mmap == NULL)
>> +return -ENOMEM;
>> +evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
>> +   sizeof(struct auxtrace_mmap));
>> +if (evlist->auxtrace_mmap == NULL) {
>> +zfree(>mmap);
>> +return -ENOMEM;
>> +}
>> +return 0;
>>  }
>>  
>>  struct mmap_params {
>> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
>> *evlist, int idx,
>>  return -1;
>>  }
>>  
>> -if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
>> ->auxtrace_mp, evlist->mmap[idx].base, fd))
>> -return -1;
>> -
>>  return 0;
>>  }
>>  
>> @@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct 
>> perf_evlist *evlist, int idx,
>>  *output = fd;
>>  if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
>>  return -1;
>> +
>> +if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
>> +>auxtrace_mp,
>> +evlist->mmap[idx].base, fd))
>> +return -1;
>>  }

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-05 Thread Jiri Olsa 
On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:

SNIP

> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> *evlist, int idx)
>   perf_mmap__write_tail(md, old);
>   }
>  
> - if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> + if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> idx))
>   perf_evlist__mmap_put(evlist, idx);
>  }
>  
> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> *evlist, int idx)
>   evlist->mmap[idx].base = NULL;
>   atomic_set(>mmap[idx].refcnt, 0);
>   }
> - auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> + auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>  }
>  
>  void perf_evlist__munmap(struct perf_evlist *evlist)
> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> *evlist)
>   if (cpu_map__empty(evlist->cpus))
>   evlist->nr_mmaps = thread_map__nr(evlist->threads);
>   evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> - return evlist->mmap != NULL ? 0 : -ENOMEM;
> + if (evlist->mmap == NULL)
> + return -ENOMEM;
> + evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> +sizeof(struct auxtrace_mmap));
> + if (evlist->auxtrace_mmap == NULL) {
> + zfree(>mmap);
> + return -ENOMEM;
> + }

can't see evlist->auxtrace_mmap being freed 

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-02 Thread Arnaldo Carvalho de Melo 
Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

- Arnaldo
 
> Cc: Adrian Hunter 
> Signed-off-by: Namhyung Kim 
> ---
>  tools/perf/builtin-record.c |  4 ++--
>  tools/perf/util/evlist.c| 30 +-
>  tools/perf/util/evlist.h|  2 +-
>  3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 5e01c070dbf2..0accac6e0812 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
> record *rec)
>  
>   for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>   struct auxtrace_mmap *mm =
> - >evlist->mmap[i].auxtrace_mmap;
> + >evlist->auxtrace_mmap[i];
>  
>   if (!mm->base)
>   continue;
> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>   int rc = 0;
>  
>   for (i = 0; i < rec->evlist->nr_mmaps; i++) {
> - struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
> + struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>  
>   if (rec->evlist->mmap[i].base) {
>   if (record__mmap_read(rec, i) != 0) {
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e46adcd5b408..042dffc67986 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
> perf_evlist *evlist, int idx)
>   return event;
>  }
>  
> -static bool perf_mmap__empty(struct perf_mmap *md)
> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>  {
> - return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
> + struct perf_mmap *md = >mmap[idx];
> +
> + return perf_mmap__read_head(md) == md->prev &&
> + evlist->auxtrace_mmap[idx].base == NULL;
>  }
>  
>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> *evlist, int idx)
>   perf_mmap__write_tail(md, old);
>   }
>  
> - if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> + if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> idx))
>   perf_evlist__mmap_put(evlist, idx);
>  }
>  
> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> *evlist, int idx)
>   evlist->mmap[idx].base = NULL;
>   atomic_set(>mmap[idx].refcnt, 0);
>   }
> - auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> + auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>  }
>  
>  void perf_evlist__munmap(struct perf_evlist *evlist)
> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> *evlist)
>   if (cpu_map__empty(evlist->cpus))
>   evlist->nr_mmaps = thread_map__nr(evlist->threads);
>   evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> - return evlist->mmap != NULL ? 0 : -ENOMEM;
> + if (evlist->mmap == NULL)
> + return -ENOMEM;
> + evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> +sizeof(struct auxtrace_mmap));
> + if (evlist->auxtrace_mmap == NULL) {
> + zfree(>mmap);
> + return -ENOMEM;
> + }
> + return 0;
>  }
>  
>  struct mmap_params {
> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
> *evlist, int idx,
>   return -1;
>   }
>  
> - if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
> - >auxtrace_mp, evlist->mmap[idx].base, fd))
> - return -1;
> -
>   return 0;
>  }
>  
> @@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct 
> perf_evlist *evlist, int idx,
>   *output = fd;
>   if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
>   return -1;
> +
> + if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
> + >auxtrace_mp,
> + evlist->mmap[idx].base, fd))
> + return -1;
>   } else {
>   if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
>   return -1;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 414e383885f5..51574ce8ac69 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -30,7 +30,6 @@ struct perf_mmap {
>   int  mask;
>   atomic_t refcnt;
>   u64  prev;
> -

Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-02 Thread Arnaldo Carvalho de Melo 
Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
> Since it's gonna share struct mmap with dummy tracking evsel to track
> meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

- Arnaldo
 
> Cc: Adrian Hunter 
> Signed-off-by: Namhyung Kim 
> ---
>  tools/perf/builtin-record.c |  4 ++--
>  tools/perf/util/evlist.c| 30 +-
>  tools/perf/util/evlist.h|  2 +-
>  3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 5e01c070dbf2..0accac6e0812 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct 
> record *rec)
>  
>   for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>   struct auxtrace_mmap *mm =
> - >evlist->mmap[i].auxtrace_mmap;
> + >evlist->auxtrace_mmap[i];
>  
>   if (!mm->base)
>   continue;
> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>   int rc = 0;
>  
>   for (i = 0; i < rec->evlist->nr_mmaps; i++) {
> - struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
> + struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
>  
>   if (rec->evlist->mmap[i].base) {
>   if (record__mmap_read(rec, i) != 0) {
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e46adcd5b408..042dffc67986 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
> perf_evlist *evlist, int idx)
>   return event;
>  }
>  
> -static bool perf_mmap__empty(struct perf_mmap *md)
> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>  {
> - return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
> + struct perf_mmap *md = >mmap[idx];
> +
> + return perf_mmap__read_head(md) == md->prev &&
> + evlist->auxtrace_mmap[idx].base == NULL;
>  }
>  
>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist 
> *evlist, int idx)
>   perf_mmap__write_tail(md, old);
>   }
>  
> - if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
> + if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
> idx))
>   perf_evlist__mmap_put(evlist, idx);
>  }
>  
> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
> *evlist, int idx)
>   evlist->mmap[idx].base = NULL;
>   atomic_set(>mmap[idx].refcnt, 0);
>   }
> - auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
> + auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
>  }
>  
>  void perf_evlist__munmap(struct perf_evlist *evlist)
> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
> *evlist)
>   if (cpu_map__empty(evlist->cpus))
>   evlist->nr_mmaps = thread_map__nr(evlist->threads);
>   evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
> - return evlist->mmap != NULL ? 0 : -ENOMEM;
> + if (evlist->mmap == NULL)
> + return -ENOMEM;
> + evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
> +sizeof(struct auxtrace_mmap));
> + if (evlist->auxtrace_mmap == NULL) {
> + zfree(>mmap);
> + return -ENOMEM;
> + }
> + return 0;
>  }
>  
>  struct mmap_params {
> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist 
> *evlist, int idx,
>   return -1;
>   }
>  
> - if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
> - >auxtrace_mp, evlist->mmap[idx].base, fd))
> - return -1;
> -
>   return 0;
>  }
>  
> @@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct 
> perf_evlist *evlist, int idx,
>   *output = fd;
>   if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
>   return -1;
> +
> + if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
> + >auxtrace_mp,
> + evlist->mmap[idx].base, fd))
> + return -1;
>   } else {
>   if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
>   return -1;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 414e383885f5..51574ce8ac69 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -30,7 +30,6 @@ struct perf_mmap {
>   int  mask;
>   atomic_t

[RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-01 Thread Namhyung Kim 
Since it's gonna share struct mmap with dummy tracking evsel to track
meta events only, let's move auxtrace out of struct perf_mmap.

Cc: Adrian Hunter 
Signed-off-by: Namhyung Kim 
---
 tools/perf/builtin-record.c |  4 ++--
 tools/perf/util/evlist.c| 30 +-
 tools/perf/util/evlist.h|  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5e01c070dbf2..0accac6e0812 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record 
*rec)
 
for (i = 0; i < rec->evlist->nr_mmaps; i++) {
struct auxtrace_mmap *mm =
-   >evlist->mmap[i].auxtrace_mmap;
+   >evlist->auxtrace_mmap[i];
 
if (!mm->base)
continue;
@@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
int rc = 0;
 
for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-   struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
+   struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
 
if (rec->evlist->mmap[i].base) {
if (record__mmap_read(rec, i) != 0) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e46adcd5b408..042dffc67986 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
perf_evlist *evlist, int idx)
return event;
 }
 
-static bool perf_mmap__empty(struct perf_mmap *md)
+static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
 {
-   return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
+   struct perf_mmap *md = >mmap[idx];
+
+   return perf_mmap__read_head(md) == md->prev &&
+   evlist->auxtrace_mmap[idx].base == NULL;
 }
 
 static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, 
int idx)
perf_mmap__write_tail(md, old);
}
 
-   if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
+   if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
idx))
perf_evlist__mmap_put(evlist, idx);
 }
 
@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
*evlist, int idx)
evlist->mmap[idx].base = NULL;
atomic_set(>mmap[idx].refcnt, 0);
}
-   auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
+   auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)
@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
*evlist)
if (cpu_map__empty(evlist->cpus))
evlist->nr_mmaps = thread_map__nr(evlist->threads);
evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-   return evlist->mmap != NULL ? 0 : -ENOMEM;
+   if (evlist->mmap == NULL)
+   return -ENOMEM;
+   evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+  sizeof(struct auxtrace_mmap));
+   if (evlist->auxtrace_mmap == NULL) {
+   zfree(>mmap);
+   return -ENOMEM;
+   }
+   return 0;
 }
 
 struct mmap_params {
@@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, 
int idx,
return -1;
}
 
-   if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
-   >auxtrace_mp, evlist->mmap[idx].base, fd))
-   return -1;
-
return 0;
 }
 
@@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist 
*evlist, int idx,
*output = fd;
if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
return -1;
+
+   if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
+   >auxtrace_mp,
+   evlist->mmap[idx].base, fd))
+   return -1;
} else {
if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
return -1;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 414e383885f5..51574ce8ac69 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -30,7 +30,6 @@ struct perf_mmap {
int  mask;
atomic_t refcnt;
u64  prev;
-   struct auxtrace_mmap auxtrace_mmap;
char event_copy[PERF_SAMPLE_MAX_SIZE] 
__attribute__((aligned(8)));
 };
 
@@ -53,6 +52,7 @@ struct perf_evlist {
} workload;
struct fdarray   pollfd;
struct perf_mmap

[RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

2015-10-01 Thread Namhyung Kim 
Since it's gonna share struct mmap with dummy tracking evsel to track
meta events only, let's move auxtrace out of struct perf_mmap.

Cc: Adrian Hunter 
Signed-off-by: Namhyung Kim 
---
 tools/perf/builtin-record.c |  4 ++--
 tools/perf/util/evlist.c| 30 +-
 tools/perf/util/evlist.h|  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5e01c070dbf2..0accac6e0812 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record 
*rec)
 
for (i = 0; i < rec->evlist->nr_mmaps; i++) {
struct auxtrace_mmap *mm =
-   >evlist->mmap[i].auxtrace_mmap;
+   >evlist->auxtrace_mmap[i];
 
if (!mm->base)
continue;
@@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
int rc = 0;
 
for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-   struct auxtrace_mmap *mm = >evlist->mmap[i].auxtrace_mmap;
+   struct auxtrace_mmap *mm = >evlist->auxtrace_mmap[i];
 
if (rec->evlist->mmap[i].base) {
if (record__mmap_read(rec, i) != 0) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e46adcd5b408..042dffc67986 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct 
perf_evlist *evlist, int idx)
return event;
 }
 
-static bool perf_mmap__empty(struct perf_mmap *md)
+static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
 {
-   return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
+   struct perf_mmap *md = >mmap[idx];
+
+   return perf_mmap__read_head(md) == md->prev &&
+   evlist->auxtrace_mmap[idx].base == NULL;
 }
 
 static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, 
int idx)
perf_mmap__write_tail(md, old);
}
 
-   if (atomic_read(>refcnt) == 1 && perf_mmap__empty(md))
+   if (atomic_read(>refcnt) == 1 && perf_evlist__mmap_empty(evlist, 
idx))
perf_evlist__mmap_put(evlist, idx);
 }
 
@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist 
*evlist, int idx)
evlist->mmap[idx].base = NULL;
atomic_set(>mmap[idx].refcnt, 0);
}
-   auxtrace_mmap__munmap(>mmap[idx].auxtrace_mmap);
+   auxtrace_mmap__munmap(>auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)
@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist 
*evlist)
if (cpu_map__empty(evlist->cpus))
evlist->nr_mmaps = thread_map__nr(evlist->threads);
evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-   return evlist->mmap != NULL ? 0 : -ENOMEM;
+   if (evlist->mmap == NULL)
+   return -ENOMEM;
+   evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+  sizeof(struct auxtrace_mmap));
+   if (evlist->auxtrace_mmap == NULL) {
+   zfree(>mmap);
+   return -ENOMEM;
+   }
+   return 0;
 }
 
 struct mmap_params {
@@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, 
int idx,
return -1;
}
 
-   if (auxtrace_mmap__mmap(>mmap[idx].auxtrace_mmap,
-   >auxtrace_mp, evlist->mmap[idx].base, fd))
-   return -1;
-
return 0;
 }
 
@@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist 
*evlist, int idx,
*output = fd;
if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
return -1;
+
+   if (auxtrace_mmap__mmap(>auxtrace_mmap[idx],
+   >auxtrace_mp,
+   evlist->mmap[idx].base, fd))
+   return -1;
} else {
if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
return -1;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 414e383885f5..51574ce8ac69 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -30,7 +30,6 @@ struct perf_mmap {
int  mask;
atomic_t refcnt;
u64  prev;
-   struct auxtrace_mmap auxtrace_mmap;
char event_copy[PERF_SAMPLE_MAX_SIZE] 
__attribute__((aligned(8)));
 };
 
@@ -53,6 +52,7 @@ struct perf_evlist {
} workload;