[PATCH 0/5] perf session: Add reader object

[PATCH 0/5] perf session: Add reader object

[Jiri Olsa]

hi,
this patchset adds reader object to interface event
processing for any data. It's defined as:

  struct reader {
        int fd;
        u64 data_size;
        u64 data_offset;
  };

Now we can simply define reader object for arbitrary file
data portion and pass it to reader__process_events function
to process its data.

It's preparation for multiple file storage under perf.data
directory.

Available also in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/fixes

thanks,
jirka


---
Jiri Olsa (5):
      perf session: Get rid of file_size variable
      perf session: Add reader object
      perf session: Add data_size to reader object
      perf session: Add data_offset to reader object
      perf session: Add reader__process_events function

 tools/perf/util/session.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 47 insertions(+), 26 deletions(-)

[PATCH 1/5] perf session: Get rid of file_size variable

[Jiri Olsa]

It's not needed and removing it makes the code little
more simple for upcoming changes.

It's safe to replace file_size with data_size, because
perf_data__size value is never smaller than data_offset
+ data_size.

Link: http://lkml.kernel.org/n/tip-ocz7zwwkkx11v0mkxrtcddih@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/session.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 3972f081a8f4..d6c1d3853f27 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1818,7 +1818,6 @@ static int __perf_session__process_events(struct perf_session *session)
 	struct ordered_events *oe = &session->ordered_events;
 	struct perf_tool *tool = session->tool;
 	int fd = perf_data__fd(session->data);
-	u64 file_size = perf_data__size(session->data);
 	u64 data_offset = session->header.data_offset;
 	u64 data_size = session->header.data_size;
 	u64 head, page_offset, file_offset, file_pos, size;
@@ -1838,14 +1837,13 @@ static int __perf_session__process_events(struct perf_session *session)
 	if (data_size == 0)
 		goto out;
 
-	if (data_offset + data_size < file_size)
-		file_size = data_offset + data_size;
+	ui_progress__init_size(&prog, data_size, "Processing events...");
 
-	ui_progress__init_size(&prog, file_size, "Processing events...");
+	data_size += data_offset;
 
 	mmap_size = MMAP_SIZE;
-	if (mmap_size > file_size) {
-		mmap_size = file_size;
+	if (mmap_size > data_size) {
+		mmap_size = data_size;
 		session->one_mmap = true;
 	}
 
@@ -1910,7 +1908,7 @@ static int __perf_session__process_events(struct perf_session *session)
 	if (session_done())
 		goto out;
 
-	if (file_pos < file_size)
+	if (file_pos < data_size)
 		goto more;
 
 out:
-- 
2.17.2

[PATCH 2/5] perf session: Add reader object

[Jiri Olsa]

Adding session private reader object to encapsulate reading
of the event data block. Starting with fd field inside.

Link: http://lkml.kernel.org/n/tip-glbs0xvtdcstckscvahueigf@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/session.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d6c1d3853f27..f5eecae982ba 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1813,11 +1813,17 @@ fetch_mmaped_event(struct perf_session *session,
 #define NUM_MMAPS 128
 #endif
 
+struct reader {
+	int	fd;
+};
+
 static int __perf_session__process_events(struct perf_session *session)
 {
+	struct reader rd = {
+		.fd		= perf_data__fd(session->data),
+	};
 	struct ordered_events *oe = &session->ordered_events;
 	struct perf_tool *tool = session->tool;
-	int fd = perf_data__fd(session->data);
 	u64 data_offset = session->header.data_offset;
 	u64 data_size = session->header.data_size;
 	u64 head, page_offset, file_offset, file_pos, size;
@@ -1857,7 +1863,7 @@ static int __perf_session__process_events(struct perf_session *session)
 		mmap_flags = MAP_PRIVATE;
 	}
 remap:
-	buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, fd,
+	buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd.fd,
 		   file_offset);
 	if (buf == MAP_FAILED) {
 		pr_err("failed to mmap file\n");
-- 
2.17.2

[PATCH 3/5] perf session: Add data_size to reader object

[Jiri Olsa]

Adding data_size to reader object. Keeping the data_size
variable instead of replacing it with rd.data_size as it
will be used in following patch.

Link: http://lkml.kernel.org/n/tip-5xcux9957yqsljh1cm5ngyqy@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/session.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f5eecae982ba..40692875f196 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1815,17 +1815,19 @@ fetch_mmaped_event(struct perf_session *session,
 
 struct reader {
 	int	fd;
+	u64	data_size;
 };
 
 static int __perf_session__process_events(struct perf_session *session)
 {
 	struct reader rd = {
 		.fd		= perf_data__fd(session->data),
+		.data_size	= session->header.data_size,
 	};
 	struct ordered_events *oe = &session->ordered_events;
 	struct perf_tool *tool = session->tool;
 	u64 data_offset = session->header.data_offset;
-	u64 data_size = session->header.data_size;
+	u64 data_size = rd.data_size;
 	u64 head, page_offset, file_offset, file_pos, size;
 	int err, mmap_prot, mmap_flags, map_idx = 0;
 	size_t	mmap_size;
-- 
2.17.2

[PATCH 4/5] perf session: Add data_offset to reader object

[Jiri Olsa]

Adding data_offset to reader object.

Link: http://lkml.kernel.org/n/tip-k531enq5bmack3mjccjblvi6@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/session.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 40692875f196..bc2a4f80535c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1816,6 +1816,7 @@ fetch_mmaped_event(struct perf_session *session,
 struct reader {
 	int	fd;
 	u64	data_size;
+	u64	data_offset;
 };
 
 static int __perf_session__process_events(struct perf_session *session)
@@ -1823,10 +1824,10 @@ static int __perf_session__process_events(struct perf_session *session)
 	struct reader rd = {
 		.fd		= perf_data__fd(session->data),
 		.data_size	= session->header.data_size,
+		.data_offset	= session->header.data_offset,
 	};
 	struct ordered_events *oe = &session->ordered_events;
 	struct perf_tool *tool = session->tool;
-	u64 data_offset = session->header.data_offset;
 	u64 data_size = rd.data_size;
 	u64 head, page_offset, file_offset, file_pos, size;
 	int err, mmap_prot, mmap_flags, map_idx = 0;
@@ -1838,16 +1839,16 @@ static int __perf_session__process_events(struct perf_session *session)
 
 	perf_tool__fill_defaults(tool);
 
-	page_offset = page_size * (data_offset / page_size);
+	page_offset = page_size * (rd.data_offset / page_size);
 	file_offset = page_offset;
-	head = data_offset - page_offset;
+	head = rd.data_offset - page_offset;
 
 	if (data_size == 0)
 		goto out;
 
 	ui_progress__init_size(&prog, data_size, "Processing events...");
 
-	data_size += data_offset;
+	data_size += rd.data_offset;
 
 	mmap_size = MMAP_SIZE;
 	if (mmap_size > data_size) {
-- 
2.17.2

[PATCH 5/5] perf session: Add reader__process_events function

[Jiri Olsa]

Adding reader__process_events function to readers data.

The reader object is defined by file's fd, data offset
and data size.

Now we can simply define reader object for arbitrary file
data portion and pass it to reader__process_events function
to process its data.

Link: http://lkml.kernel.org/n/tip-sh3yfoavlzr2f45rf8g6145j@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/session.c | 62 ++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index bc2a4f80535c..6a9dc5f0675c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1819,36 +1819,25 @@ struct reader {
 	u64	data_offset;
 };
 
-static int __perf_session__process_events(struct perf_session *session)
+static int
+reader__process_events(struct reader *rd, struct perf_session *session,
+		       struct ui_progress *prog)
 {
-	struct reader rd = {
-		.fd		= perf_data__fd(session->data),
-		.data_size	= session->header.data_size,
-		.data_offset	= session->header.data_offset,
-	};
-	struct ordered_events *oe = &session->ordered_events;
-	struct perf_tool *tool = session->tool;
-	u64 data_size = rd.data_size;
+	u64 data_size = rd->data_size;
 	u64 head, page_offset, file_offset, file_pos, size;
-	int err, mmap_prot, mmap_flags, map_idx = 0;
+	int err = 0, mmap_prot, mmap_flags, map_idx = 0;
 	size_t	mmap_size;
 	char *buf, *mmaps[NUM_MMAPS];
 	union perf_event *event;
-	struct ui_progress prog;
 	s64 skip;
 
-	perf_tool__fill_defaults(tool);
-
-	page_offset = page_size * (rd.data_offset / page_size);
+	page_offset = page_size * (rd->data_offset / page_size);
 	file_offset = page_offset;
-	head = rd.data_offset - page_offset;
+	head = rd->data_offset - page_offset;
 
-	if (data_size == 0)
-		goto out;
+	ui_progress__init_size(prog, data_size, "Processing events...");
 
-	ui_progress__init_size(&prog, data_size, "Processing events...");
-
-	data_size += rd.data_offset;
+	data_size += rd->data_offset;
 
 	mmap_size = MMAP_SIZE;
 	if (mmap_size > data_size) {
@@ -1866,12 +1855,12 @@ static int __perf_session__process_events(struct perf_session *session)
 		mmap_flags = MAP_PRIVATE;
 	}
 remap:
-	buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd.fd,
+	buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, rd->fd,
 		   file_offset);
 	if (buf == MAP_FAILED) {
 		pr_err("failed to mmap file\n");
 		err = -errno;
-		goto out_err;
+		goto out;
 	}
 	mmaps[map_idx] = buf;
 	map_idx = (map_idx + 1) & (ARRAY_SIZE(mmaps) - 1);
@@ -1903,7 +1892,7 @@ static int __perf_session__process_events(struct perf_session *session)
 		       file_offset + head, event->header.size,
 		       event->header.type);
 		err = -EINVAL;
-		goto out_err;
+		goto out;
 	}
 
 	if (skip)
@@ -1912,7 +1901,7 @@ static int __perf_session__process_events(struct perf_session *session)
 	head += size;
 	file_pos += size;
 
-	ui_progress__update(&prog, size);
+	ui_progress__update(prog, size);
 
 	if (session_done())
 		goto out;
@@ -1921,6 +1910,31 @@ static int __perf_session__process_events(struct perf_session *session)
 		goto more;
 
 out:
+	return err;
+}
+
+static int __perf_session__process_events(struct perf_session *session)
+{
+	struct reader rd = {
+		.fd		= perf_data__fd(session->data),
+		.data_size	= session->header.data_size,
+		.data_offset	= session->header.data_offset,
+	};
+	struct ordered_events *oe = &session->ordered_events;
+	struct perf_tool *tool = session->tool;
+	struct ui_progress prog;
+	int err;
+
+	perf_tool__fill_defaults(tool);
+
+	if (rd.data_size == 0)
+		return -1;
+
+	ui_progress__init_size(&prog, rd.data_size, "Processing events...");
+
+	err = reader__process_events(&rd, session, &prog);
+	if (err)
+		goto out_err;
 	/* do the final flush for ordered samples */
 	err = ordered_events__flush(oe, OE_FLUSH__FINAL);
 	if (err)
-- 
2.17.2

Re: [PATCH 1/5] perf session: Get rid of file_size variable

[Arnaldo Carvalho de Melo]

Em Wed, Dec 26, 2018 at 02:51:28PM +0100, Jiri Olsa escreveu:
> It's not needed and removing it makes the code little
> more simple for upcoming changes.
> 
> It's safe to replace file_size with data_size, because
> perf_data__size value is never smaller than data_offset
> + data_size.

This is not applying to my perf/core branch, can you check?

- Arnaldo
 
> Link: http://lkml.kernel.org/n/tip-ocz7zwwkkx11v0mkxrtcddih@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/session.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 3972f081a8f4..d6c1d3853f27 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1818,7 +1818,6 @@ static int __perf_session__process_events(struct perf_session *session)
>  	struct ordered_events *oe = &session->ordered_events;
>  	struct perf_tool *tool = session->tool;
>  	int fd = perf_data__fd(session->data);
> -	u64 file_size = perf_data__size(session->data);
>  	u64 data_offset = session->header.data_offset;
>  	u64 data_size = session->header.data_size;
>  	u64 head, page_offset, file_offset, file_pos, size;
> @@ -1838,14 +1837,13 @@ static int __perf_session__process_events(struct perf_session *session)
>  	if (data_size == 0)
>  		goto out;
>  
> -	if (data_offset + data_size < file_size)
> -		file_size = data_offset + data_size;
> +	ui_progress__init_size(&prog, data_size, "Processing events...");
>  
> -	ui_progress__init_size(&prog, file_size, "Processing events...");
> +	data_size += data_offset;
>  
>  	mmap_size = MMAP_SIZE;
> -	if (mmap_size > file_size) {
> -		mmap_size = file_size;
> +	if (mmap_size > data_size) {
> +		mmap_size = data_size;
>  		session->one_mmap = true;
>  	}
>  
> @@ -1910,7 +1908,7 @@ static int __perf_session__process_events(struct perf_session *session)
>  	if (session_done())
>  		goto out;
>  
> -	if (file_pos < file_size)
> +	if (file_pos < data_size)
>  		goto more;
>  
>  out:
> -- 
> 2.17.2

-- 

- Arnaldo

Re: [PATCH 1/5] perf session: Get rid of file_size variable

[Jiri Olsa]

On Wed, Dec 26, 2018 at 04:43:42PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 26, 2018 at 02:51:28PM +0100, Jiri Olsa escreveu:
> > It's not needed and removing it makes the code little
> > more simple for upcoming changes.
> > 
> > It's safe to replace file_size with data_size, because
> > perf_data__size value is never smaller than data_offset
> > + data_size.
> 
> This is not applying to my perf/core branch, can you check?

hum, I've got clean rebase, pushed in perf/fixes

jirka

> 
> - Arnaldo
>  
> > Link: http://lkml.kernel.org/n/tip-ocz7zwwkkx11v0mkxrtcddih@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/util/session.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index 3972f081a8f4..d6c1d3853f27 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -1818,7 +1818,6 @@ static int __perf_session__process_events(struct perf_session *session)
> >  	struct ordered_events *oe = &session->ordered_events;
> >  	struct perf_tool *tool = session->tool;
> >  	int fd = perf_data__fd(session->data);
> > -	u64 file_size = perf_data__size(session->data);
> >  	u64 data_offset = session->header.data_offset;
> >  	u64 data_size = session->header.data_size;
> >  	u64 head, page_offset, file_offset, file_pos, size;
> > @@ -1838,14 +1837,13 @@ static int __perf_session__process_events(struct perf_session *session)
> >  	if (data_size == 0)
> >  		goto out;
> >  
> > -	if (data_offset + data_size < file_size)
> > -		file_size = data_offset + data_size;
> > +	ui_progress__init_size(&prog, data_size, "Processing events...");
> >  
> > -	ui_progress__init_size(&prog, file_size, "Processing events...");
> > +	data_size += data_offset;
> >  
> >  	mmap_size = MMAP_SIZE;
> > -	if (mmap_size > file_size) {
> > -		mmap_size = file_size;
> > +	if (mmap_size > data_size) {
> > +		mmap_size = data_size;
> >  		session->one_mmap = true;
> >  	}
> >  
> > @@ -1910,7 +1908,7 @@ static int __perf_session__process_events(struct perf_session *session)
> >  	if (session_done())
> >  		goto out;
> >  
> > -	if (file_pos < file_size)
> > +	if (file_pos < data_size)
> >  		goto more;
> >  
> >  out:
> > -- 
> > 2.17.2
> 
> -- 
> 
> - Arnaldo