Re: [PATCH 4/5] perf tools: Support single perf.data file directory
Em Mon, Oct 21, 2019 at 03:42:39PM +0300, Adrian Hunter escreveu:
> On 7/10/19 3:06 PM, Adrian Hunter wrote:
> > On 7/10/19 2:20 PM, Jiri Olsa wrote:
> >> On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
> >>
> >> SNIP
> >>
> >>> u8 pad[8] = {0};
> >>>
> >>> - if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> >>> + if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
> >>> off_t file_offset;
> >>> int fd = perf_data__fd(data);
> >>> int err;
> >>> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> >>> index df173f0bf654..964ea101dba6 100644
> >>> --- a/tools/perf/util/data.c
> >>> +++ b/tools/perf/util/data.c
> >>> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
> >>> DIR *dir;
> >>> int nr = 0;
> >>>
> >>> + /*
> >>> + * Directory containing a single regular perf data file which is already
> >>> + * open, means there is nothing more to do here.
> >>> + */
> >>> + if (perf_data__is_single_file(data))
> >>> + return 0;
> >>> +
> >>
> >> cool, I like this approach much more than the previous flag
> >
> > Yes it is much nicer. Thanks for your direction on that.
> >
> >>
> >> any change you (if there's repost) or Arnaldo
> >> could squeeze in indent change below?
> >
> > Sent a patch, to be applied before these.
> >
>
> That is:
>
> "[PATCH] perf session: Fix indent in perf_session__new()"
Done it as a separate patch, etc.
> Any comments on this patch set Arnaldo?
--
- Arnaldo
Re: [PATCH 4/5] perf tools: Support single perf.data file directory
On Mon, Oct 21, 2019 at 10:46:11AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 21, 2019 at 03:42:39PM +0300, Adrian Hunter escreveu:
> > On 7/10/19 3:06 PM, Adrian Hunter wrote:
> > > On 7/10/19 2:20 PM, Jiri Olsa wrote:
> > >> On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
> > >>
> > >> SNIP
> > >>
> > >>> u8 pad[8] = {0};
> > >>>
> > >>> - if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> > >>> + if (!perf_data__is_pipe(data) &&
> > >>> perf_data__is_single_file(data)) {
> > >>> off_t file_offset;
> > >>> int fd = perf_data__fd(data);
> > >>> int err;
> > >>> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> > >>> index df173f0bf654..964ea101dba6 100644
> > >>> --- a/tools/perf/util/data.c
> > >>> +++ b/tools/perf/util/data.c
> > >>> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
> > >>> DIR *dir;
> > >>> int nr = 0;
> > >>>
> > >>> + /*
> > >>> +* Directory containing a single regular perf data file which
> > >>> is already
> > >>> +* open, means there is nothing more to do here.
> > >>> +*/
> > >>> + if (perf_data__is_single_file(data))
> > >>> + return 0;
> > >>> +
> > >>
> > >> cool, I like this approach much more than the previous flag
> > >
> > > Yes it is much nicer. Thanks for your direction on that.
> > >
> > >>
> > >> any change you (if there's repost) or Arnaldo
> > >> could squeeze in indent change below?
> > >
> > > Sent a patch, to be applied before these.
> > >
> >
> > That is:
> >
> > "[PATCH] perf session: Fix indent in perf_session__new()"
> >
> > Any comments on this patch set Arnaldo?
>
> I'll take a look at it later, I think Jiri is ok with it already?
yes, it's just indent fix
jirka
Re: [PATCH 4/5] perf tools: Support single perf.data file directory
Em Mon, Oct 21, 2019 at 03:42:39PM +0300, Adrian Hunter escreveu:
> On 7/10/19 3:06 PM, Adrian Hunter wrote:
> > On 7/10/19 2:20 PM, Jiri Olsa wrote:
> >> On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
> >>
> >> SNIP
> >>
> >>> u8 pad[8] = {0};
> >>>
> >>> - if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> >>> + if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
> >>> off_t file_offset;
> >>> int fd = perf_data__fd(data);
> >>> int err;
> >>> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> >>> index df173f0bf654..964ea101dba6 100644
> >>> --- a/tools/perf/util/data.c
> >>> +++ b/tools/perf/util/data.c
> >>> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
> >>> DIR *dir;
> >>> int nr = 0;
> >>>
> >>> + /*
> >>> + * Directory containing a single regular perf data file which is already
> >>> + * open, means there is nothing more to do here.
> >>> + */
> >>> + if (perf_data__is_single_file(data))
> >>> + return 0;
> >>> +
> >>
> >> cool, I like this approach much more than the previous flag
> >
> > Yes it is much nicer. Thanks for your direction on that.
> >
> >>
> >> any change you (if there's repost) or Arnaldo
> >> could squeeze in indent change below?
> >
> > Sent a patch, to be applied before these.
> >
>
> That is:
>
> "[PATCH] perf session: Fix indent in perf_session__new()"
>
> Any comments on this patch set Arnaldo?
I'll take a look at it later, I think Jiri is ok with it already?
- Arnaldo
Re: [PATCH 4/5] perf tools: Support single perf.data file directory
On 7/10/19 3:06 PM, Adrian Hunter wrote:
> On 7/10/19 2:20 PM, Jiri Olsa wrote:
>> On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
>>
>> SNIP
>>
>>> u8 pad[8] = {0};
>>>
>>> - if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
>>> + if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
>>> off_t file_offset;
>>> int fd = perf_data__fd(data);
>>> int err;
>>> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
>>> index df173f0bf654..964ea101dba6 100644
>>> --- a/tools/perf/util/data.c
>>> +++ b/tools/perf/util/data.c
>>> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
>>> DIR *dir;
>>> int nr = 0;
>>>
>>> + /*
>>> +* Directory containing a single regular perf data file which is already
>>> +* open, means there is nothing more to do here.
>>> +*/
>>> + if (perf_data__is_single_file(data))
>>> + return 0;
>>> +
>>
>> cool, I like this approach much more than the previous flag
>
> Yes it is much nicer. Thanks for your direction on that.
>
>>
>> any change you (if there's repost) or Arnaldo
>> could squeeze in indent change below?
>
> Sent a patch, to be applied before these.
>
That is:
"[PATCH] perf session: Fix indent in perf_session__new()"
Any comments on this patch set Arnaldo?
Re: [PATCH 4/5] perf tools: Support single perf.data file directory
On 7/10/19 2:20 PM, Jiri Olsa wrote:
> On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
>
> SNIP
>
>> u8 pad[8] = {0};
>>
>> -if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
>> +if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
>> off_t file_offset;
>> int fd = perf_data__fd(data);
>> int err;
>> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
>> index df173f0bf654..964ea101dba6 100644
>> --- a/tools/perf/util/data.c
>> +++ b/tools/perf/util/data.c
>> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
>> DIR *dir;
>> int nr = 0;
>>
>> +/*
>> + * Directory containing a single regular perf data file which is already
>> + * open, means there is nothing more to do here.
>> + */
>> +if (perf_data__is_single_file(data))
>> +return 0;
>> +
>
> cool, I like this approach much more than the previous flag
Yes it is much nicer. Thanks for your direction on that.
>
> any change you (if there's repost) or Arnaldo
> could squeeze in indent change below?
Sent a patch, to be applied before these.
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index bfa80fe8d369..7f567a521cea 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -227,8 +227,8 @@ struct perf_session *perf_session__new(struct perf_data
> *data,
> /* Open the directory data. */
> if (data->is_dir) {
> ret = perf_data__open_dir(data);
> - if (ret)
> - goto out_delete;
> + if (ret)
> + goto out_delete;
> }
>
> if (!symbol_conf.kallsyms_name &&
>
Re: [PATCH 4/5] perf tools: Support single perf.data file directory
On Fri, Oct 04, 2019 at 11:31:20AM +0300, Adrian Hunter wrote:
SNIP
> u8 pad[8] = {0};
>
> - if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> + if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
> off_t file_offset;
> int fd = perf_data__fd(data);
> int err;
> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index df173f0bf654..964ea101dba6 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
> DIR *dir;
> int nr = 0;
>
> + /*
> + * Directory containing a single regular perf data file which is already
> + * open, means there is nothing more to do here.
> + */
> + if (perf_data__is_single_file(data))
> + return 0;
> +
cool, I like this approach much more than the previous flag
any change you (if there's repost) or Arnaldo
could squeeze in indent change below?
thanks,
jirka
---
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index bfa80fe8d369..7f567a521cea 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -227,8 +227,8 @@ struct perf_session *perf_session__new(struct perf_data
*data,
/* Open the directory data. */
if (data->is_dir) {
ret = perf_data__open_dir(data);
- if (ret)
- goto out_delete;
+ if (ret)
+ goto out_delete;
}
if (!symbol_conf.kallsyms_name &&
[PATCH 4/5] perf tools: Support single perf.data file directory
Support directory output that contains a regular perf.data file, named
"data". By default the directory is named perf.data i.e.
perf.data
└── data
Most of the infrastucture to support a directory is already there. This
patch makes the changes needed to support the format above.
Presently there is no 'perf record' option to output a directory.
This is preparation for adding support for putting a copy of /proc/kcore in
the directory.
Signed-off-by: Adrian Hunter
---
.../perf.data-directory-format.txt| 28 +++
tools/perf/builtin-record.c | 2 +-
tools/perf/util/data.c| 9 +-
tools/perf/util/data.h| 6
4 files changed, 43 insertions(+), 2 deletions(-)
create mode 100644 tools/perf/Documentation/perf.data-directory-format.txt
diff --git a/tools/perf/Documentation/perf.data-directory-format.txt
b/tools/perf/Documentation/perf.data-directory-format.txt
new file mode 100644
index ..4bf08908178d
--- /dev/null
+++ b/tools/perf/Documentation/perf.data-directory-format.txt
@@ -0,0 +1,28 @@
+perf.data directory format
+
+DISCLAIMER This is not ABI yet and is subject to possible change
+ in following versions of perf. We will remove this
+ disclaimer once the directory format soaks in.
+
+
+This document describes the on-disk perf.data directory format.
+
+The layout is described by HEADER_DIR_FORMAT feature.
+Currently it holds only version number (0):
+
+ HEADER_DIR_FORMAT = 24
+
+ struct {
+ uint64_t version;
+ }
+
+The current only version value 0 means that:
+ - there is a single perf.data file named 'data' within the directory.
+ e.g.
+
+$ tree -ps perf.data
+perf.data
+└── [-rw--- 25912] data
+
+Future versions are expected to describe different data files
+layout according to special needs.
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 23332861de6e..fceac9d42b4e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -537,7 +537,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
size_t padding;
u8 pad[8] = {0};
- if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
+ if (!perf_data__is_pipe(data) && perf_data__is_single_file(data)) {
off_t file_offset;
int fd = perf_data__fd(data);
int err;
diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index df173f0bf654..964ea101dba6 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -76,6 +76,13 @@ int perf_data__open_dir(struct perf_data *data)
DIR *dir;
int nr = 0;
+ /*
+* Directory containing a single regular perf data file which is already
+* open, means there is nothing more to do here.
+*/
+ if (perf_data__is_single_file(data))
+ return 0;
+
if (WARN_ON(!data->is_dir))
return -EINVAL;
@@ -406,7 +413,7 @@ unsigned long perf_data__size(struct perf_data *data)
u64 size = data->file.size;
int i;
- if (!data->is_dir)
+ if (perf_data__is_single_file(data))
return size;
for (i = 0; i < data->dir.nr; i++) {
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index 218fe9a16801..f68815f7e428 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -10,6 +10,7 @@ enum perf_data_mode {
};
enum perf_dir_version {
+ PERF_DIR_SINGLE_FILE= 0,
PERF_DIR_VERSION= 1,
};
@@ -54,6 +55,11 @@ static inline bool perf_data__is_dir(struct perf_data *data)
return data->is_dir;
}
+static inline bool perf_data__is_single_file(struct perf_data *data)
+{
+ return data->dir.version == PERF_DIR_SINGLE_FILE;
+}
+
static inline int perf_data__fd(struct perf_data *data)
{
return data->file.fd;
--
2.17.1
