2019-02-16 05:56:57

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] perf: update perf.data file format documentation

I found that the documentation of the flags section is some how
different from the actual format used and expected by the perf
tools. In this patch the according section of the file format
documentation is updated to conform to the expectations of the
perf tool suite.

Signed-off-by: Jonas Rabenstein <[email protected]>
---
.../perf/Documentation/perf.data-file-format.txt | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index dfb218feaad9..6ea199f28330 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,13 +43,10 @@ struct perf_file_section {

Flags section:

-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
-
-Some headers consist of strings, which are defined like this:
+The Flags section is placed directly after the data section and consists of a
+variable amount of information described by the flags-bitset in the perf_header.
+A lot of the headers in the Flags section are simple strings and are represented
+like this:

struct perf_header_string {
uint32_t len;
@@ -82,7 +79,7 @@ assigned by the linker to an executable.
struct build_id_event {
struct perf_event_header header;
pid_t pid;
- uint8_t build_id[24];
+ uint8_t build_id[PERF_ALIGN(24, sizeof(u64))];
char filename[header.size - offsetof(struct build_id_event, filename)];
};

@@ -131,7 +128,7 @@ An uint64_t with the total memory in bytes.

HEADER_CMDLINE = 11,

-A perf_header_string with the perf command line used to collect the data.
+A perf_header_string_list with the perf arg-vector used to collect the data.

HEADER_EVENT_DESC = 12,

--
2.17.1



2019-02-17 23:24:39

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: update perf.data file format documentation

On Fri, Feb 15, 2019 at 07:28:23PM +0100, Jonas Rabenstein wrote:
> I found that the documentation of the flags section is some how
> different from the actual format used and expected by the perf
> tools. In this patch the according section of the file format
> documentation is updated to conform to the expectations of the
> perf tool suite.
>
> Signed-off-by: Jonas Rabenstein <[email protected]>
> ---
> .../perf/Documentation/perf.data-file-format.txt | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> index dfb218feaad9..6ea199f28330 100644
> --- a/tools/perf/Documentation/perf.data-file-format.txt
> +++ b/tools/perf/Documentation/perf.data-file-format.txt
> @@ -43,13 +43,10 @@ struct perf_file_section {
>
> Flags section:
>
> -The header is followed by different optional headers, described by the bits set
> -in flags. Only headers for which the bit is set are included. Each header
> -consists of a perf_file_section located after the initial header.
> -The respective perf_file_section points to the data of the additional
> -header and defines its size.
> -
> -Some headers consist of strings, which are defined like this:
> +The Flags section is placed directly after the data section and consists of a
> +variable amount of information described by the flags-bitset in the perf_header.
> +A lot of the headers in the Flags section are simple strings and are represented
> +like this:

some how I find this more confusing.. please describe
what's actualy wrong with the current wording

>
> struct perf_header_string {
> uint32_t len;
> @@ -82,7 +79,7 @@ assigned by the linker to an executable.
> struct build_id_event {
> struct perf_event_header header;
> pid_t pid;
> - uint8_t build_id[24];
> + uint8_t build_id[PERF_ALIGN(24, sizeof(u64))];

isn't that always 24? I guess u meant:

build_id[PERF_ALIGN(20, sizeof(u64))];


> char filename[header.size - offsetof(struct build_id_event, filename)];
> };
>
> @@ -131,7 +128,7 @@ An uint64_t with the total memory in bytes.
>
> HEADER_CMDLINE = 11,
>
> -A perf_header_string with the perf command line used to collect the data.
> +A perf_header_string_list with the perf arg-vector used to collect the data.

nice catch

thanks,
jirka

>
> HEADER_EVENT_DESC = 12,
>
> --
> 2.17.1
>

2019-02-18 12:47:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: update perf.data file format documentation

Em Mon, Feb 18, 2019 at 12:22:46AM +0100, Jiri Olsa escreveu:
> On Fri, Feb 15, 2019 at 07:28:23PM +0100, Jonas Rabenstein wrote:
> > I found that the documentation of the flags section is some how
> > different from the actual format used and expected by the perf
> > tools. In this patch the according section of the file format
> > documentation is updated to conform to the expectations of the
> > perf tool suite.
> >
> > Signed-off-by: Jonas Rabenstein <[email protected]>
> > ---
> > .../perf/Documentation/perf.data-file-format.txt | 15 ++++++---------
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> > index dfb218feaad9..6ea199f28330 100644
> > --- a/tools/perf/Documentation/perf.data-file-format.txt
> > +++ b/tools/perf/Documentation/perf.data-file-format.txt
> > @@ -43,13 +43,10 @@ struct perf_file_section {
> >
> > Flags section:
> >
> > -The header is followed by different optional headers, described by the bits set
> > -in flags. Only headers for which the bit is set are included. Each header
> > -consists of a perf_file_section located after the initial header.
> > -The respective perf_file_section points to the data of the additional
> > -header and defines its size.
> > -
> > -Some headers consist of strings, which are defined like this:
> > +The Flags section is placed directly after the data section and consists of a
> > +variable amount of information described by the flags-bitset in the perf_header.
> > +A lot of the headers in the Flags section are simple strings and are represented
> > +like this:
>
> some how I find this more confusing.. please describe
> what's actualy wrong with the current wording
>
> >
> > struct perf_header_string {
> > uint32_t len;
> > @@ -82,7 +79,7 @@ assigned by the linker to an executable.
> > struct build_id_event {
> > struct perf_event_header header;
> > pid_t pid;
> > - uint8_t build_id[24];
> > + uint8_t build_id[PERF_ALIGN(24, sizeof(u64))];
>
> isn't that always 24? I guess u meant:
>
> build_id[PERF_ALIGN(20, sizeof(u64))];
>
>
> > char filename[header.size - offsetof(struct build_id_event, filename)];
> > };
> >
> > @@ -131,7 +128,7 @@ An uint64_t with the total memory in bytes.
> >
> > HEADER_CMDLINE = 11,
> >
> > -A perf_header_string with the perf command line used to collect the data.
> > +A perf_header_string_list with the perf arg-vector used to collect the data.
>
> nice catch

This is why patch granularity is important, this part is unrelated to
the other fixes, so should be in a separate patch, had it been like
that I'd already process this part that got Jiri's Acked-by (in the form
of a positive comment).

- Arnaldo

2019-02-18 14:04:01

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation

The content of this feature header is a perf_header_string_list of
the argument vector and not a perf_header_string of the commandline.
---
tools/perf/Documentation/perf.data-file-format.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index dfb218feaad9..5f9a3924830b 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes.

HEADER_CMDLINE = 11,

-A perf_header_string with the perf command line used to collect the data.
+A perf_header_string_list with the perf arg-vector used to collect the data.

HEADER_EVENT_DESC = 12,

--
2.19.2


2019-02-18 14:21:07

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] perf: fix documentation of the Flags section in perf.data

According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.
---
tools/perf/Documentation/perf.data-file-format.txt | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {

Flags section:

-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.

Some headers consist of strings, which are defined like this:

--
2.19.2


2019-02-19 14:05:50

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation

On Mon, Feb 18, 2019 at 03:02:03PM +0100, Jonas Rabenstein wrote:
> The content of this feature header is a perf_header_string_list of
> the argument vector and not a perf_header_string of the commandline.

missing your Signed-off-by

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

> ---
> tools/perf/Documentation/perf.data-file-format.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> index dfb218feaad9..5f9a3924830b 100644
> --- a/tools/perf/Documentation/perf.data-file-format.txt
> +++ b/tools/perf/Documentation/perf.data-file-format.txt
> @@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes.
>
> HEADER_CMDLINE = 11,
>
> -A perf_header_string with the perf command line used to collect the data.
> +A perf_header_string_list with the perf arg-vector used to collect the data.
>
> HEADER_EVENT_DESC = 12,
>
> --
> 2.19.2
>

2019-02-19 14:06:05

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: fix documentation of the Flags section in perf.data

On Mon, Feb 18, 2019 at 03:18:46PM +0100, Jonas Rabenstein wrote:
> According to the current documentation the flags section is placed after
> the file header itself but the code assumes to find the flags section
> after the data section. This change updates the documentation to that
> assumption.

missing your Signed-off-by

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

> ---
> tools/perf/Documentation/perf.data-file-format.txt | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> index 5f9a3924830b..593ef49b273c 100644
> --- a/tools/perf/Documentation/perf.data-file-format.txt
> +++ b/tools/perf/Documentation/perf.data-file-format.txt
> @@ -43,11 +43,10 @@ struct perf_file_section {
>
> Flags section:
>
> -The header is followed by different optional headers, described by the bits set
> -in flags. Only headers for which the bit is set are included. Each header
> -consists of a perf_file_section located after the initial header.
> -The respective perf_file_section points to the data of the additional
> -header and defines its size.
> +For each of the optional features a perf_file_section it placed after the data
> +section if the feature bit is set in the perf_header flags bitset. The
> +respective perf_file_section points to the data of the additional header and
> +defines its size.
>
> Some headers consist of strings, which are defined like this:
>
> --
> 2.19.2
>

2019-02-19 15:25:24

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation

Em Tue, Feb 19, 2019 at 03:04:09PM +0100, Jiri Olsa escreveu:
> On Mon, Feb 18, 2019 at 03:02:03PM +0100, Jonas Rabenstein wrote:
> > The content of this feature header is a perf_header_string_list of
> > the argument vector and not a perf_header_string of the commandline.
>
> missing your Signed-off-by

Please provide for both and I'll apply.

In such a case you could repost with what is missing (your
Signed-off-by: your-email) and collect the Acked-by from whoever
provides them, Jiri in this case, so just add these two lines to the
changeset log:

Acked-by: Jiri Olsa <[email protected]>
Signed-off-by: Jonas Rabenstein <[email protected]>

Thanks,

- Arnaldo

> Acked-by: Jiri Olsa <[email protected]>
>
> thanks,
> jirka
>
> > ---
> > tools/perf/Documentation/perf.data-file-format.txt | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
> > index dfb218feaad9..5f9a3924830b 100644
> > --- a/tools/perf/Documentation/perf.data-file-format.txt
> > +++ b/tools/perf/Documentation/perf.data-file-format.txt
> > @@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes.
> >
> > HEADER_CMDLINE = 11,
> >
> > -A perf_header_string with the perf command line used to collect the data.
> > +A perf_header_string_list with the perf arg-vector used to collect the data.
> >
> > HEADER_EVENT_DESC = 12,
> >
> > --
> > 2.19.2
> >

--

- Arnaldo

2019-02-19 15:46:03

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] perf: fix HEADER_CMDLINE description in perf.data documentation

The content of this feature header is a perf_header_string_list of
the argument vector and not a perf_header_string of the commandline.

Acked-by: Jiri Olsa <[email protected]>
Signed-off-by: Jonas Rabenstein <[email protected]>
---
tools/perf/Documentation/perf.data-file-format.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index dfb218feaad9..5f9a3924830b 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes.

HEADER_CMDLINE = 11,

-A perf_header_string with the perf command line used to collect the data.
+A perf_header_string_list with the perf arg-vector used to collect the data.

HEADER_EVENT_DESC = 12,

--
2.19.2


2019-02-19 15:46:12

by Jonas Rabenstein

[permalink] [raw]
Subject: [PATCH] perf: fix documentation of the Flags section in perf.data

According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.

Acked-by: Jiri Olsa <[email protected]>
Signed-off-by: Jonas Rabenstein <[email protected]>
---
tools/perf/Documentation/perf.data-file-format.txt | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {

Flags section:

-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.

Some headers consist of strings, which are defined like this:

--
2.19.2


Subject: [tip:perf/core] perf doc: Fix HEADER_CMDLINE description in perf.data documentation

Commit-ID: 7a663c0ff330a068c088ad46bb0130e651a9fec3
Gitweb: https://git.kernel.org/tip/7a663c0ff330a068c088ad46bb0130e651a9fec3
Author: Jonas Rabenstein <[email protected]>
AuthorDate: Tue, 19 Feb 2019 16:45:14 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 19 Feb 2019 13:39:08 -0300

perf doc: Fix HEADER_CMDLINE description in perf.data documentation

The content of the HEADER_CMDLINE feature header is a perf_header_string_list
of the argument vector and not a perf_header_string of the commandline.

Signed-off-by: Jonas Rabenstein <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Richter <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf.data-file-format.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index dfb218feaad9..5f9a3924830b 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -131,7 +131,7 @@ An uint64_t with the total memory in bytes.

HEADER_CMDLINE = 11,

-A perf_header_string with the perf command line used to collect the data.
+A perf_header_string_list with the perf arg-vector used to collect the data.

HEADER_EVENT_DESC = 12,


Subject: [tip:perf/core] perf doc: Fix documentation of the Flags section in perf.data

Commit-ID: 8c23a522388b34cd7bc8473987eda0c75eb37c0e
Gitweb: https://git.kernel.org/tip/8c23a522388b34cd7bc8473987eda0c75eb37c0e
Author: Jonas Rabenstein <[email protected]>
AuthorDate: Tue, 19 Feb 2019 16:45:15 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 19 Feb 2019 13:39:12 -0300

perf doc: Fix documentation of the Flags section in perf.data

According to the current documentation the flags section is placed after
the file header itself but the code assumes to find the flags section
after the data section. This change updates the documentation to that
assumption.

Signed-off-by: Jonas Rabenstein <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Richter <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf.data-file-format.txt | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index 5f9a3924830b..593ef49b273c 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -43,11 +43,10 @@ struct perf_file_section {

Flags section:

-The header is followed by different optional headers, described by the bits set
-in flags. Only headers for which the bit is set are included. Each header
-consists of a perf_file_section located after the initial header.
-The respective perf_file_section points to the data of the additional
-header and defines its size.
+For each of the optional features a perf_file_section it placed after the data
+section if the feature bit is set in the perf_header flags bitset. The
+respective perf_file_section points to the data of the additional header and
+defines its size.

Some headers consist of strings, which are defined like this: