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

[Adrian Hunter <adrian.hunter@intel.com>]

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 <adrian.hunter@intel.com>
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  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 =
>> -				&rec->evlist->mmap[i].auxtrace_mmap;
>> +				&rec->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 = &rec->evlist->mmap[i].auxtrace_mmap;
>> +		struct auxtrace_mmap *mm = &rec->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 = &evlist->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(&md->refcnt) == 1 && perf_mmap__empty(md))
>> +	if (atomic_read(&md->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(&evlist->mmap[idx].refcnt, 0);
>>  	}
>> -	auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
>> +	auxtrace_mmap__munmap(&evlist->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(&evlist->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(&evlist->mmap[idx].auxtrace_mmap,
>> -				&mp->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(&evlist->auxtrace_mmap[idx],
>> +						&mp->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 *mmap;
>> +	struct auxtrace_mmap *auxtrace_mmap;
>>  	struct thread_map *threads;
>>  	struct cpu_map	  *cpus;
>>  	struct perf_evsel *selected;
>> -- 
>> 2.6.0