Re: [PATCH 5.0 05/93] perf data: Dont store auxtrace index for directory data file

[Jiri Olsa]

On Fri, Apr 19, 2019 at 09:25:27AM -0400, Sasha Levin wrote:
> On Thu, Apr 18, 2019 at 10:28:47PM +0300, Thomas Backlund wrote:
> > Den 18-04-2019 kl. 20:56, skrev Greg Kroah-Hartman:
> > > [ Upstream commit cd3dd8dd8ff62374d90cb3f2e54b8c94106c7810 ]
> > > 
> > > We can't store the auxtrace index when we store into multiple files,
> > > because we keep only offset for it, not the file.
> > > 
> > > The auxtrace data will be processed correctly in the 'pipe' mode.
> > > 
> > > Signed-off-by: Jiri Olsa 
> > > Cc: Adrian Hunter 
> > > Cc: Alexander Shishkin 
> > > Cc: Alexey Budankov 
> > > Cc: Andi Kleen 
> > > Cc: Namhyung Kim 
> > > Cc: Peter Zijlstra 
> > > Cc: Stephane Eranian 
> > > Link: http://lkml.kernel.org/r/20190308134745.5057-3-jo...@kernel.org
> > > Signed-off-by: Arnaldo Carvalho de Melo 
> > > Signed-off-by: Sasha Levin 
> > > ---
> > >  tools/perf/builtin-record.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > > index 882285fb9f64..3fd154f1701b 100644
> > > --- a/tools/perf/builtin-record.c
> > > +++ b/tools/perf/builtin-record.c
> > > @@ -386,7 +386,7 @@ static int record__process_auxtrace(struct perf_tool 
> > > *tool,
> > >   size_t padding;
> > >   u8 pad[8] = {0};
> > > - if (!perf_data__is_pipe(data)) {
> > > + if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> > >   off_t file_offset;
> > >   int fd = perf_data__fd(data);
> > >   int err;
> > > 
> > 
> > 
> > This breaks the build with:
> > 
> > builtin-record.c: In function 'record__process_auxtrace':
> > builtin-record.c:389:36: warning: implicit declaration of function
> > 'perf_data__is_dir'; did you mean 'perf_data__is_pipe'?
> > [-Wimplicit-function-declaration]
> >  if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
> >^
> > 
> > Looks like it depends atleast on:
> > 
> > commit ec65def1045e4c7817b7f741a86dadae82877a93
> > Author: Jiri Olsa 
> > Date:   Fri Mar 8 14:47:35 2019 +0100
> > 
> >perf data: Support having perf.data stored as a directory
> > 
> > 
> > Maybe better to drop it.
> 
> I've dropped it, thanks!

yes, no harm to drop this one if the perf_data__is_dir is not backported

jirka

Re: [PATCH 5.0 05/93] perf data: Dont store auxtrace index for directory data file

[Sasha Levin]

On Thu, Apr 18, 2019 at 10:28:47PM +0300, Thomas Backlund wrote:

Den 18-04-2019 kl. 20:56, skrev Greg Kroah-Hartman:

[ Upstream commit cd3dd8dd8ff62374d90cb3f2e54b8c94106c7810 ]

We can't store the auxtrace index when we store into multiple files,
because we keep only offset for it, not the file.

The auxtrace data will be processed correctly in the 'pipe' mode.

Signed-off-by: Jiri Olsa 
Cc: Adrian Hunter 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: Andi Kleen 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Link: http://lkml.kernel.org/r/20190308134745.5057-3-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
Signed-off-by: Sasha Levin 
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 882285fb9f64..3fd154f1701b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -386,7 +386,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
size_t padding;
u8 pad[8] = {0};
-   if (!perf_data__is_pipe(data)) {
+   if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
off_t file_offset;
int fd = perf_data__fd(data);
int err;




This breaks the build with:

builtin-record.c: In function 'record__process_auxtrace':
builtin-record.c:389:36: warning: implicit declaration of function 
'perf_data__is_dir'; did you mean 'perf_data__is_pipe'? 
[-Wimplicit-function-declaration]

 if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
   ^

Looks like it depends atleast on:

commit ec65def1045e4c7817b7f741a86dadae82877a93
Author: Jiri Olsa 
Date:   Fri Mar 8 14:47:35 2019 +0100

   perf data: Support having perf.data stored as a directory


Maybe better to drop it.


I've dropped it, thanks!

--
Thanks,
Sasha

Re: [PATCH 5.0 05/93] perf data: Dont store auxtrace index for directory data file

[Thomas Backlund]

Den 18-04-2019 kl. 20:56, skrev Greg Kroah-Hartman:

[ Upstream commit cd3dd8dd8ff62374d90cb3f2e54b8c94106c7810 ]

We can't store the auxtrace index when we store into multiple files,
because we keep only offset for it, not the file.

The auxtrace data will be processed correctly in the 'pipe' mode.

Signed-off-by: Jiri Olsa 
Cc: Adrian Hunter 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: Andi Kleen 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Link: http://lkml.kernel.org/r/20190308134745.5057-3-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
Signed-off-by: Sasha Levin 
---
  tools/perf/builtin-record.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 882285fb9f64..3fd154f1701b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -386,7 +386,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
size_t padding;
u8 pad[8] = {0};
  
-	if (!perf_data__is_pipe(data)) {

+   if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
off_t file_offset;
int fd = perf_data__fd(data);
int err;




This breaks the build with:

builtin-record.c: In function 'record__process_auxtrace':
builtin-record.c:389:36: warning: implicit declaration of function 
'perf_data__is_dir'; did you mean 'perf_data__is_pipe'? 
[-Wimplicit-function-declaration]

  if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
^

Looks like it depends atleast on:

commit ec65def1045e4c7817b7f741a86dadae82877a93
Author: Jiri Olsa 
Date:   Fri Mar 8 14:47:35 2019 +0100

perf data: Support having perf.data stored as a directory


Maybe better to drop it.

--
Thomas

Re: [PATCH 5.0 05/93] perf data: Dont store auxtrace index for directory data file

[Dan Rue]

On Thu, Apr 18, 2019 at 07:56:43PM +0200, Greg Kroah-Hartman wrote:
> [ Upstream commit cd3dd8dd8ff62374d90cb3f2e54b8c94106c7810 ]
> 
> We can't store the auxtrace index when we store into multiple files,
> because we keep only offset for it, not the file.
> 
> The auxtrace data will be processed correctly in the 'pipe' mode.
> 
> Signed-off-by: Jiri Olsa 
> Cc: Adrian Hunter 
> Cc: Alexander Shishkin 
> Cc: Alexey Budankov 
> Cc: Andi Kleen 
> Cc: Namhyung Kim 
> Cc: Peter Zijlstra 
> Cc: Stephane Eranian 
> Link: http://lkml.kernel.org/r/20190308134745.5057-3-jo...@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo 
> Signed-off-by: Sasha Levin 
> ---
>  tools/perf/builtin-record.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 882285fb9f64..3fd154f1701b 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -386,7 +386,7 @@ static int record__process_auxtrace(struct perf_tool 
> *tool,
>   size_t padding;
>   u8 pad[8] = {0};
>  
> - if (!perf_data__is_pipe(data)) {
> + if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {

This causes the following build error on 5.0:

builtin-top.c: In function ‘__cmd_top’:
builtin-top.c:1241:3: error: label ‘out_delete’ used but not defined
   goto out_delete;
   ^~~~
builtin-record.c: In function ‘record__process_auxtrace’:
builtin-record.c:389:36: error: implicit declaration of function 
‘perf_data__is_dir’; did you mean ‘perf_data__is_pipe’? 
[-Werror=implicit-function-declaration]
  if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
^
perf_data__is_pipe
builtin-record.c:389:36: error: nested extern declaration of 
‘perf_data__is_dir’ [-Werror=nested-externs]

Dropping the patch solves this error, though there is a second perf
related build error in this RC caused by 2c0bd03b5d20 ("perf top: Delete
the evlist before perf_session, fixing heap-use-after-free issue").


>   off_t file_offset;
>   int fd = perf_data__fd(data);
>   int err;
> -- 
> 2.19.1
> 
> 
> 

-- 
Linaro - Kernel Validation

[PATCH 5.0 05/93] perf data: Dont store auxtrace index for directory data file

[Greg Kroah-Hartman]

[ Upstream commit cd3dd8dd8ff62374d90cb3f2e54b8c94106c7810 ]

We can't store the auxtrace index when we store into multiple files,
because we keep only offset for it, not the file.

The auxtrace data will be processed correctly in the 'pipe' mode.

Signed-off-by: Jiri Olsa 
Cc: Adrian Hunter 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: Andi Kleen 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Link: http://lkml.kernel.org/r/20190308134745.5057-3-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
Signed-off-by: Sasha Levin 
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 882285fb9f64..3fd154f1701b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -386,7 +386,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
size_t padding;
u8 pad[8] = {0};
 
-   if (!perf_data__is_pipe(data)) {
+   if (!perf_data__is_pipe(data) && !perf_data__is_dir(data)) {
off_t file_offset;
int fd = perf_data__fd(data);
int err;
-- 
2.19.1