Linux-perf-users Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] perf inject: correct event attribute sizes
@ 2020-11-24 19:58 Al Grant
  2020-12-16  6:58 ` Denis Nikitin
  0 siblings, 1 reply; 7+ messages in thread
From: Al Grant @ 2020-11-24 19:58 UTC (permalink / raw)
  To: linux-perf-users, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim
  Cc: Al Grant, Al Grant, linux-kernel

When perf inject reads a perf.data file from an older version of perf,
it writes event attributes into the output with the original size field,
but lays them out as if they had the size currently used. Readers see
a corrupt file. Update the size field to match the layout.

Signed-off-by: Al Grant <al.grant@foss.arm.com>
---
 tools/perf/util/header.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index be850e9f8852..0d95981df8dd 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3331,6 +3331,14 @@ int perf_session__write_header(struct perf_session *session,
 	attr_offset = lseek(ff.fd, 0, SEEK_CUR);
 
 	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
+			/*
+			 * We are likely in "perf inject" and have read
+			 * from an older file. Update attr size so that
+			 * reader gets the right offset to the ids.
+			 */
+			evsel->core.attr.size = sizeof(evsel->core.attr);
+		}
 		f_attr = (struct perf_file_attr){
 			.attr = evsel->core.attr,
 			.ids  = {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] perf inject: correct event attribute sizes
  2020-11-24 19:58 [PATCH] perf inject: correct event attribute sizes Al Grant
@ 2020-12-16  6:58 ` Denis Nikitin
  2020-12-23 22:11   ` Jiri Olsa
  0 siblings, 1 reply; 7+ messages in thread
From: Denis Nikitin @ 2020-12-16  6:58 UTC (permalink / raw)
  To: linux-perf-users, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung
  Cc: al.grant, al.grant, linux-kernel, denik

When perf inject reads a perf.data file from an older version of perf,
it writes event attributes into the output with the original size field,
but lays them out as if they had the size currently used. Readers see
a corrupt file. Update the size field to match the layout.

Signed-off-by: Al Grant <al.grant@foss.arm.com>
Signed-off-by: Denis Nikitin <denik@chromium.org>
---
 tools/perf/util/header.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index be850e9f8852..0d95981df8dd 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3331,6 +3331,14 @@ int perf_session__write_header(struct perf_session *session,
 	attr_offset = lseek(ff.fd, 0, SEEK_CUR);
 
 	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
+			/*
+			 * We are likely in "perf inject" and have read
+			 * from an older file. Update attr size so that
+			 * reader gets the right offset to the ids.
+			 */
+			evsel->core.attr.size = sizeof(evsel->core.attr);
+		}
 		f_attr = (struct perf_file_attr){
 			.attr = evsel->core.attr,
 			.ids  = {
-- 
2.29.2.684.gfbc64c5ab5-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] perf inject: correct event attribute sizes
  2020-12-16  6:58 ` Denis Nikitin
@ 2020-12-23 22:11   ` Jiri Olsa
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2020-12-23 22:11 UTC (permalink / raw)
  To: Denis Nikitin
  Cc: linux-perf-users, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, namhyung, al.grant, al.grant, linux-kernel

On Tue, Dec 15, 2020 at 10:58:16PM -0800, Denis Nikitin wrote:
> When perf inject reads a perf.data file from an older version of perf,
> it writes event attributes into the output with the original size field,
> but lays them out as if they had the size currently used. Readers see
> a corrupt file. Update the size field to match the layout.
> 
> Signed-off-by: Al Grant <al.grant@foss.arm.com>
> Signed-off-by: Denis Nikitin <denik@chromium.org>
> ---
>  tools/perf/util/header.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index be850e9f8852..0d95981df8dd 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3331,6 +3331,14 @@ int perf_session__write_header(struct perf_session *session,
>  	attr_offset = lseek(ff.fd, 0, SEEK_CUR);
>  
>  	evlist__for_each_entry(evlist, evsel) {
> +		if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
> +			/*
> +			 * We are likely in "perf inject" and have read
> +			 * from an older file. Update attr size so that
> +			 * reader gets the right offset to the ids.
> +			 */
> +			evsel->core.attr.size = sizeof(evsel->core.attr);
> +		}

seems ok, just wondering if it would be better
to fix it in perf_event__process_attr instead

where we know where the data is coming from,
here we could cover for some unexpected case
of having different attr.size?

anyway both would be probably equal, I'm ok
with either way

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka


>  		f_attr = (struct perf_file_attr){
>  			.attr = evsel->core.attr,
>  			.ids  = {
> -- 
> 2.29.2.684.gfbc64c5ab5-goog
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] perf inject: correct event attribute sizes
  2020-08-26  9:42 Al Grant
  2020-08-26 11:27 ` Mark Rutland
@ 2020-09-01 17:26 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-09-01 17:26 UTC (permalink / raw)
  To: Al Grant; +Cc: linux-perf-users, linux-kernel, acme

Em Wed, Aug 26, 2020 at 10:42:04AM +0100, Al Grant escreveu:
> When perf inject reads a perf.data file from an older version of perf,
> it writes event attributes into the output with the original size field,
> but lays them out as if they had the size currently used. Readers see
> a corrupt file. Update the size field to match the layout.
> 
> From: Denis Nikitin <denik@google.com>
> Signed-off-by: Al Grant <al.grant@foss.arm.com>

Ok, so the author is Denis, has he provided his Signed-off-by?

- Arnaldo
 
>  tools/perf/util/header.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 9cf4efdcbbbd..762eb94bd532 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3326,6 +3326,14 @@ int perf_session__write_header(struct
> perf_session *session,
>         attr_offset = lseek(ff.fd, 0, SEEK_CUR);
> 
>         evlist__for_each_entry(evlist, evsel) {
> +               if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
> +                       /*
> +                        * We are likely in "perf inject" and have read +
> * from an older file. Update attr size so that
> +                        * reader gets the right offset to the ids.
> +                        */
> +                       evsel->core.attr.size = sizeof(evsel->core.attr);
> +               }
>                 f_attr = (struct perf_file_attr){
>                         .attr = evsel->core.attr,
>                         .ids  = {

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] perf inject: correct event attribute sizes
  2020-08-26 11:27 ` Mark Rutland
@ 2020-08-26 13:23   ` Al Grant
  0 siblings, 0 replies; 7+ messages in thread
From: Al Grant @ 2020-08-26 13:23 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-perf-users, linux-kernel, acme, Denis Nikitin

On 26/08/2020 12:27, Mark Rutland wrote:
> On Wed, Aug 26, 2020 at 10:42:04AM +0100, Al Grant wrote:
>> When perf inject reads a perf.data file from an older version of perf,
>> it writes event attributes into the output with the original size field,
>> but lays them out as if they had the size currently used. Readers see
>> a corrupt file. Update the size field to match the layout.
>>
>> From: Denis Nikitin <denik@google.com>
>> Signed-off-by: Al Grant <al.grant@foss.arm.com>
> 
> Did Denis write this patch?

Joint work - I fixed the sizes in the event attributes section,
but Denis noticed that the copies in HEADER_EVENT_DESC also need
fixing, so the final text is his.

> If so, we need an S-o-B line from them.

I have his approval to upstream this, so:

Signed-off-by: Denis Nikitin <denik@google.com>

Al


> 
> Mark.
> 
>>   tools/perf/util/header.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index 9cf4efdcbbbd..762eb94bd532 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -3326,6 +3326,14 @@ int perf_session__write_header(struct
>> perf_session *session,
>>          attr_offset = lseek(ff.fd, 0, SEEK_CUR);
>>
>>          evlist__for_each_entry(evlist, evsel) {
>> +               if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
>> +                       /*
>> +                        * We are likely in "perf inject" and have read +
>> * from an older file. Update attr size so that
>> +                        * reader gets the right offset to the ids.
>> +                        */
>> +                       evsel->core.attr.size = sizeof(evsel->core.attr);
>> +               }
>>                  f_attr = (struct perf_file_attr){
>>                          .attr = evsel->core.attr,
>>                          .ids  = {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] perf inject: correct event attribute sizes
  2020-08-26  9:42 Al Grant
@ 2020-08-26 11:27 ` Mark Rutland
  2020-08-26 13:23   ` Al Grant
  2020-09-01 17:26 ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 7+ messages in thread
From: Mark Rutland @ 2020-08-26 11:27 UTC (permalink / raw)
  To: Al Grant; +Cc: linux-perf-users, linux-kernel, acme, Denis Nikitin

On Wed, Aug 26, 2020 at 10:42:04AM +0100, Al Grant wrote:
> When perf inject reads a perf.data file from an older version of perf,
> it writes event attributes into the output with the original size field,
> but lays them out as if they had the size currently used. Readers see
> a corrupt file. Update the size field to match the layout.
> 
> From: Denis Nikitin <denik@google.com>
> Signed-off-by: Al Grant <al.grant@foss.arm.com>

Did Denis write this patch?

If so, we need an S-o-B line from them.

Mark.

>  tools/perf/util/header.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 9cf4efdcbbbd..762eb94bd532 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3326,6 +3326,14 @@ int perf_session__write_header(struct
> perf_session *session,
>         attr_offset = lseek(ff.fd, 0, SEEK_CUR);
> 
>         evlist__for_each_entry(evlist, evsel) {
> +               if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
> +                       /*
> +                        * We are likely in "perf inject" and have read +
> * from an older file. Update attr size so that
> +                        * reader gets the right offset to the ids.
> +                        */
> +                       evsel->core.attr.size = sizeof(evsel->core.attr);
> +               }
>                 f_attr = (struct perf_file_attr){
>                         .attr = evsel->core.attr,
>                         .ids  = {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] perf inject: correct event attribute sizes
@ 2020-08-26  9:42 Al Grant
  2020-08-26 11:27 ` Mark Rutland
  2020-09-01 17:26 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 7+ messages in thread
From: Al Grant @ 2020-08-26  9:42 UTC (permalink / raw)
  To: linux-perf-users, linux-kernel, acme

When perf inject reads a perf.data file from an older version of perf,
it writes event attributes into the output with the original size field,
but lays them out as if they had the size currently used. Readers see
a corrupt file. Update the size field to match the layout.

From: Denis Nikitin <denik@google.com>
Signed-off-by: Al Grant <al.grant@foss.arm.com>

  tools/perf/util/header.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 9cf4efdcbbbd..762eb94bd532 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3326,6 +3326,14 @@ int perf_session__write_header(struct
perf_session *session,
         attr_offset = lseek(ff.fd, 0, SEEK_CUR);

         evlist__for_each_entry(evlist, evsel) {
+               if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
+                       /*
+                        * We are likely in "perf inject" and have read 
+                        * from an older file. Update attr size so that
+                        * reader gets the right offset to the ids.
+                        */
+                       evsel->core.attr.size = sizeof(evsel->core.attr);
+               }
                 f_attr = (struct perf_file_attr){
                         .attr = evsel->core.attr,
                         .ids  = {

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-24 19:58 [PATCH] perf inject: correct event attribute sizes Al Grant
2020-12-16  6:58 ` Denis Nikitin
2020-12-23 22:11   ` Jiri Olsa
  -- strict thread matches above, loose matches on Subject: below --
2020-08-26  9:42 Al Grant
2020-08-26 11:27 ` Mark Rutland
2020-08-26 13:23   ` Al Grant
2020-09-01 17:26 ` Arnaldo Carvalho de Melo

Linux-perf-users Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-perf-users/0 linux-perf-users/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-perf-users linux-perf-users/ https://lore.kernel.org/linux-perf-users \
		linux-perf-users@vger.kernel.org
	public-inbox-index linux-perf-users

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-perf-users


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git