[PATCH] trace-cmd: fix length calculation for padding events

[PATCH] trace-cmd: fix length calculation for padding events

[David Sharp]

Padding events store size in bytes, not words. Usually this ends up pushing
the parser off the current page, but occasionally not, and when not,
it ends up desynchronizing the parser from the event stream. This would
manifest as a lot of "ug!" messages from trace-cmd, and kernelshark
crashing before displaying the UI.

Signed-off-by: David Sharp <dhsharp@google.com>
---
 trace-input.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/trace-input.c b/trace-input.c
index e3157a7..b6af1e6 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -1458,7 +1458,6 @@ translate_data(struct pevent *pevent,
 	switch (type_len) {
 	case RINGBUF_TYPE_PADDING:
 		*length = data2host4(pevent, *ptr);
-		*length *= 4;
 		*ptr += *length;
 		break;
 
-- 
1.7.7.3

Re: [PATCH] trace-cmd: fix length calculation for padding events

[Steven Rostedt]

On Thu, 2012-01-26 at 19:00 -0800, David Sharp wrote:
> Padding events store size in bytes, not words. Usually this ends up pushing
> the parser off the current page, but occasionally not, and when not,
> it ends up desynchronizing the parser from the event stream. This would
> manifest as a lot of "ug!" messages from trace-cmd, and kernelshark
> crashing before displaying the UI.
> 

Nice! Funny, Gregory (Cc'd) sent me a trace.dat file that exhibited this
behavior. I was going to look into it tomorrow. I tried it with this
patch and it makes the bug go away.

Nice catch!  I wonder how long this has been around without notice?
Probably from day one.

-- Steve

> Signed-off-by: David Sharp <dhsharp@google.com>
> ---
>  trace-input.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/trace-input.c b/trace-input.c
> index e3157a7..b6af1e6 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -1458,7 +1458,6 @@ translate_data(struct pevent *pevent,
>  	switch (type_len) {
>  	case RINGBUF_TYPE_PADDING:
>  		*length = data2host4(pevent, *ptr);
> -		*length *= 4;
>  		*ptr += *length;
>  		break;
>  

Re: [PATCH] trace-cmd: fix length calculation for padding events

[David Sharp]

On Thu, Jan 26, 2012 at 7:44 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 2012-01-26 at 19:00 -0800, David Sharp wrote:
>> Padding events store size in bytes, not words. Usually this ends up pushing
>> the parser off the current page, but occasionally not, and when not,
>> it ends up desynchronizing the parser from the event stream. This would
>> manifest as a lot of "ug!" messages from trace-cmd, and kernelshark
>> crashing before displaying the UI.
>>
>
> Nice! Funny, Gregory (Cc'd) sent me a trace.dat file that exhibited this
> behavior. I was going to look into it tomorrow. I tried it with this
> patch and it makes the bug go away.
>
> Nice catch!  I wonder how long this has been around without notice?
> Probably from day one.

Yeah, I think so. It's an easy mistake to make, because it's
incongruent with the rest of the event types.

>
> -- Steve
>
>> Signed-off-by: David Sharp <dhsharp@google.com>
>> ---
>>  trace-input.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/trace-input.c b/trace-input.c
>> index e3157a7..b6af1e6 100644
>> --- a/trace-input.c
>> +++ b/trace-input.c
>> @@ -1458,7 +1458,6 @@ translate_data(struct pevent *pevent,
>>       switch (type_len) {
>>       case RINGBUF_TYPE_PADDING:
>>               *length = data2host4(pevent, *ptr);
>> -             *length *= 4;
>>               *ptr += *length;
>>               break;
>>
>
>

Re: [PATCH] trace-cmd: fix length calculation for padding events

[Steven Rostedt]

On Fri, 2012-01-27 at 11:21 -0800, David Sharp wrote:

> > Nice catch!  I wonder how long this has been around without notice?
> > Probably from day one.
> 
> Yeah, I think so. It's an easy mistake to make, because it's
> incongruent with the rest of the event types.

Yeah, I admit, that was a flaw in the buffer design. :-/

-- Steve