diff mbox series

perf report: Fix regression when decoding intelPT traces

Message ID 1513213074-25173-1-git-send-email-mathieu.poirier@linaro.org
State New
Headers show
Series perf report: Fix regression when decoding intelPT traces | expand

Commit Message

Mathieu Poirier Dec. 14, 2017, 12:57 a.m. UTC
Commit (93d10af26bb7 perf tools: Optimize sample parsing for ordered
events) breaks intelPT trace decoding by invariably returning an error if
the event type isn't a PERF_SAMPLE_TIME.

With this patch the timestamp is initialised and processing is allowed to
continue even if an error is returned by function
perf_evlist__parse_sample_timestamp().

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

---
 tools/perf/util/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.7.4

Comments

Jiri Olsa Dec. 14, 2017, 3:28 p.m. UTC | #1
On Wed, Dec 13, 2017 at 05:57:54PM -0700, Mathieu Poirier wrote:
> Commit (93d10af26bb7 perf tools: Optimize sample parsing for ordered

> events) breaks intelPT trace decoding by invariably returning an error if

> the event type isn't a PERF_SAMPLE_TIME.


right, thanks for catchng that

> 

> With this patch the timestamp is initialised and processing is allowed to

> continue even if an error is returned by function

> perf_evlist__parse_sample_timestamp().

> 

> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> ---

>  tools/perf/util/session.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c

> index 54e30f1bcbd7..20cdcf14232b 100644

> --- a/tools/perf/util/session.c

> +++ b/tools/perf/util/session.c

> @@ -1512,7 +1512,7 @@ static s64 perf_session__process_event(struct perf_session *session,

>  

>  		ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);

>  		if (ret)

> -			return ret;

> +			timestamp = -1ULL;


we still have some -EFAULT error codes in there that we
want to catch.. should it be more like in below?

thanks,
jirka


---
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 54e30f1bcbd7..9498aa0efe39 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1508,10 +1508,10 @@ static s64 perf_session__process_event(struct perf_session *session,
 		return perf_session__process_user_event(session, event, file_offset);
 
 	if (tool->ordered_events) {
-		u64 timestamp;
+		u64 timestamp = -1ULL
 
 		ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);
-		if (ret)
+		if (ret != -1)
 			return ret;
 
 		ret = perf_session__queue_event(session, event, timestamp, file_offset);
Mathieu Poirier Dec. 14, 2017, 4:03 p.m. UTC | #2
On 14 December 2017 at 08:28, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, Dec 13, 2017 at 05:57:54PM -0700, Mathieu Poirier wrote:

>> Commit (93d10af26bb7 perf tools: Optimize sample parsing for ordered

>> events) breaks intelPT trace decoding by invariably returning an error if

>> the event type isn't a PERF_SAMPLE_TIME.

>

> right, thanks for catchng that

>

>>

>> With this patch the timestamp is initialised and processing is allowed to

>> continue even if an error is returned by function

>> perf_evlist__parse_sample_timestamp().

>>

>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>> ---

>>  tools/perf/util/session.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c

>> index 54e30f1bcbd7..20cdcf14232b 100644

>> --- a/tools/perf/util/session.c

>> +++ b/tools/perf/util/session.c

>> @@ -1512,7 +1512,7 @@ static s64 perf_session__process_event(struct perf_session *session,

>>

>>               ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);

>>               if (ret)

>> -                     return ret;

>> +                     timestamp = -1ULL;

>

> we still have some -EFAULT error codes in there that we

> want to catch.. should it be more like in below?

>

> thanks,

> jirka

>

>

> ---

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c

> index 54e30f1bcbd7..9498aa0efe39 100644

> --- a/tools/perf/util/session.c

> +++ b/tools/perf/util/session.c

> @@ -1508,10 +1508,10 @@ static s64 perf_session__process_event(struct perf_session *session,

>                 return perf_session__process_user_event(session, event, file_offset);

>

>         if (tool->ordered_events) {

> -               u64 timestamp;

> +               u64 timestamp = -1ULL

>

>                 ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);

> -               if (ret)

> +               if (ret != -1)

>                         return ret;


Yes, that's better  - I'll send another revision.

>

>                 ret = perf_session__queue_event(session, event, timestamp, file_offset);
diff mbox series

Patch

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 54e30f1bcbd7..20cdcf14232b 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1512,7 +1512,7 @@  static s64 perf_session__process_event(struct perf_session *session,
 
 		ret = perf_evlist__parse_sample_timestamp(evlist, event, &timestamp);
 		if (ret)
-			return ret;
+			timestamp = -1ULL;
 
 		ret = perf_session__queue_event(session, event, timestamp, file_offset);
 		if (ret != -ETIME)