Re: [PATCH v2 04/15] perf session: move reader object definition to header file

[Alexey Budankov <alexey.budankov@linux.intel.com>]

On 24.10.2020 18:44, Jiri Olsa wrote:
> On Wed, Oct 21, 2020 at 06:59:48PM +0300, Alexey Budankov wrote:
>>
>> Move definition of reader to session header file to be shared
>> among different source files. Introduce reference to active
>> reader object from session object.
>>
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>>  tools/perf/util/session.c | 27 ---------------------------
>>  tools/perf/util/session.h | 25 +++++++++++++++++++++++++
>>  2 files changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 6f09d506b2f6..911b2dbcd0ac 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -2110,33 +2110,6 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
>>  	return 0;
>>  }
>>  
>> -/*
>> - * On 64bit we can mmap the data file in one go. No need for tiny mmap
>> - * slices. On 32bit we use 32MB.
>> - */
>> -#if BITS_PER_LONG == 64
>> -#define MMAP_SIZE ULLONG_MAX
>> -#define NUM_MMAPS 1
>> -#else
>> -#define MMAP_SIZE (32 * 1024 * 1024ULL)
>> -#define NUM_MMAPS 128
>> -#endif
>> -
>> -struct reader;
>> -
>> -typedef s64 (*reader_cb_t)(struct perf_session *session,
>> -			   union perf_event *event,
>> -			   u64 file_offset,
>> -			   const char *file_path);
>> -
>> -struct reader {
>> -	int		 fd;
>> -	const char	 *path;
>> -	u64		 data_size;
>> -	u64		 data_offset;
>> -	reader_cb_t	 process;
>> -};
>> -
>>  static int
>>  reader__process_events(struct reader *rd, struct perf_session *session,
>>  		       struct ui_progress *prog)
>> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
>> index 378ffc3e2809..abdb8518a81f 100644
>> --- a/tools/perf/util/session.h
>> +++ b/tools/perf/util/session.h
>> @@ -20,6 +20,30 @@ struct thread;
>>  struct auxtrace;
>>  struct itrace_synth_opts;
>>  
>> +/*
>> + * On 64bit we can mmap the data file in one go. No need for tiny mmap
>> + * slices. On 32bit we use 32MB.
>> + */
>> +#if BITS_PER_LONG == 64
>> +#define MMAP_SIZE ULLONG_MAX
>> +#define NUM_MMAPS 1
>> +#else
>> +#define MMAP_SIZE (32 * 1024 * 1024ULL)
>> +#define NUM_MMAPS 128
>> +#endif
>> +
>> +typedef s64 (*reader_cb_t)(struct perf_session *session,
>> +			   union perf_event *event,
>> +			   u64 file_offset, const char *file_path);
>> +
>> +struct reader {
>> +	int		 fd;
>> +	const char	 *path;
>> +	u64		 data_size;
>> +	u64		 data_offset;
>> +	reader_cb_t	 process;
>> +};
> 
> I wasn't able to find where is this used ouf of session.c ?

I will double check and adjust appropriately.

> 
> 
>> +
>>  struct perf_session {
>>  	struct perf_header	header;
>>  	struct machines		machines;
>> @@ -41,6 +65,7 @@ struct perf_session {
>>  	struct zstd_data	zstd_data;
>>  	struct decomp		*decomp;
>>  	struct decomp		*decomp_last;
>> +	struct reader		*reader;
> 
> please define it in the patch where it's actualy used

Corrected in v3.

Alexei

> 
> thanks,
> jirka
>