Re: [PATCH v1 1/3] perf record: allocate affinity masks

[Alexey Budankov]

Hi,
On 12.12.2018 15:15, Jiri Olsa wrote:
> On Wed, Dec 12, 2018 at 10:38:23AM +0300, Alexey Budankov wrote:
>>
>> Allocate affinity option and masks for mmap data buffers and
>> record thread as well as initialize allocated objects.
>>
>> Signed-off-by: Alexey Budankov <alexey.budankov@xxxxxxxxxxxxxxx>
>> ---
>>  tools/perf/builtin-record.c | 11 ++++++++++-
>>  tools/perf/perf.h           |  5 +++++
>>  tools/perf/util/evlist.c    |  6 +++---
>>  tools/perf/util/evlist.h    |  2 +-
>>  tools/perf/util/mmap.c      |  2 ++
>>  tools/perf/util/mmap.h      |  3 ++-
>>  6 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 882285fb9f64..4979719e54ae 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -81,6 +81,7 @@ struct record {
>>  	bool			timestamp_boundary;
>>  	struct switch_output	switch_output;
>>  	unsigned long long	samples;
>> +	cpu_set_t		affinity_mask;
>>  };
>>  
>>  static volatile int auxtrace_record__snapshot_started;
>> @@ -533,7 +534,8 @@ static int record__mmap_evlist(struct record *rec,
>>  
>>  	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
>>  				 opts->auxtrace_mmap_pages,
>> -				 opts->auxtrace_snapshot_mode, opts->nr_cblocks) < 0) {
>> +				 opts->auxtrace_snapshot_mode,
>> +				 opts->nr_cblocks, opts->affinity) < 0) {
>>  		if (errno == EPERM) {
>>  			pr_err("Permission error mapping pages.\n"
>>  			       "Consider increasing "
>> @@ -1980,6 +1982,9 @@ int cmd_record(int argc, const char **argv)
>>  # undef REASON
>>  #endif
>>  
>> +	CPU_ZERO(&rec->affinity_mask);
>> +	rec->opts.affinity = PERF_AFFINITY_SYS;
>> +
>>  	rec->evlist = perf_evlist__new();
>>  	if (rec->evlist == NULL)
>>  		return -ENOMEM;
>> @@ -2143,6 +2148,10 @@ int cmd_record(int argc, const char **argv)
>>  	if (verbose > 0)
>>  		pr_info("nr_cblocks: %d\n", rec->opts.nr_cblocks);
>>  
>> +	pr_debug("affinity (UNSET:%d, NODE:%d, CPU:%d) = %d\n",
>> +		 PERF_AFFINITY_SYS, PERF_AFFINITY_NODE,
>> +		 PERF_AFFINITY_CPU, rec->opts.affinity);
> 
> please make this user friendly and display the actual string like "UNSET/NODE/CPU"

Implemented in v2.

> 
>> +
>>  	err = __cmd_record(&record, argc, argv);
>>  out:
>>  	perf_evlist__delete(rec->evlist);
>> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
>> index 388c6dd128b8..08e75815de2f 100644
>> --- a/tools/perf/perf.h
>> +++ b/tools/perf/perf.h
>> @@ -83,8 +83,13 @@ struct record_opts {
>>  	clockid_t    clockid;
>>  	u64          clockid_res_ns;
>>  	int	     nr_cblocks;
>> +	int	     affinity;
>>  };
>>  
>> +#define PERF_AFFINITY_SYS  0
>> +#define PERF_AFFINITY_NODE 1
>> +#define PERF_AFFINITY_CPU  2
> 
> please put those to enum

Implemented in v2.

Thanks,
Alexey

> 
> thanks,
> jirka
>