On Tue, Sep 24, 2019 at 12:12:25PM +0300, Adrian Hunter wrote:
> 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.
don't know ;-)
but I'd like to have one way of finding out the directory layout
the code for threaded record uses DIR_FORMAT feature value
to ensure the directory contains the expected files, which
is data file with 'data.<cpu>' name for every cpu
>
> >
> > 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.
ok, that might work, but please make that somehow explicit/visible
what files/directories are possible in the directory, so we could
easily see them and add new ones
jirka
