Re: [PATCH RFC 1/2] perf tools: Support single perf.data file directory

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

On 24/09/19 12:34 AM, Jiri Olsa wrote:
> On Mon, Sep 16, 2019 at 11:56:45AM +0300, Adrian Hunter wrote:
>> Support directory output that contains a regular perf.data file. This is
>> preparation for adding support for putting a copy of /proc/kcore in that
>> directory.
>>
>> Distinguish the multiple file case from the regular (single) perf.data file
>> case by adding data->is_multi_file.
> 
> SNIP
> 
>>  static int open_file_read(struct perf_data *data)
>>  {
>>  	struct stat st;
>> @@ -302,12 +312,17 @@ static int open_dir(struct perf_data *data)
>>  {
>>  	int ret;
>>  
>> -	/*
>> -	 * So far we open only the header, so we can read the data version and
>> -	 * layout.
>> -	 */
>> -	if (asprintf(&data->file.path, "%s/header", data->path) < 0)
>> -		return -1;
>> +	if (perf_data__is_multi_file(data)) {
>> +		/*
>> +		 * So far we open only the header, so we can read the data version and
>> +		 * layout.
>> +		 */
>> +		if (asprintf(&data->file.path, "%s/header", data->path) < 0)
>> +			return -1;
>> +	} else {
>> +		if (asprintf(&data->file.path, "%s/perf.data", data->path) < 0)
>> +			return -1;
>> +	}
> 

Thanks for replying :-)

> first, please note that there's support for perf.data directory code,
> but it's not been enabled yet, so we can do any changes there without
> breaking existing users
> 
> currently the logic is prepared to have perf.data DIR_FORMAT feature
> to define the layout of the directory
> 
> it'd be great to have just single point where we get directory layout,
> not checking on files names first and checking on DIR_FORMAT later

Ok, but what are you suggesting?  Naming the data file "header" seems a bit
counter-intuitive in this case.

> 
> also the kcore will be beneficial for other layouts,
> so would be great to make it somehow optional/switchable

In these patches it is, because it is not related to the DIR_FORMAT.

> one of the options could be to have DIR_FORMAT feature as the source
> of directory layout and it'd have bitmask of files/dirs (like kcore_dir)
> available in the directory

Is there an advantage to making optional files/dirs part of the format?
i.e. if they are there, use them otherwise don't.