Re: [BUG] perf_event: when events are grouped, the time enabled / running values are incorrect

[stephane eranian]

Hi,

I believe there is another bug related to timing and I am not
sure if this patch fixes it too.

If you add TIME_ENABLED|TIME_RUNNING in samples, they
will get zero all the time (ENA, RUN):

$ libpfm/perf_examples/task_smpl noploop 4
period=240000000 freq=0
58  PERF_COUNT_HW_CPU_CYCLES
59  PERF_COUNT_HW_INSTRUCTIONS
noploop for 4 seconds

IIP:0x0000000000400651 PID:10613 TID:10613 TIME:27480525003765
STREAM_ID:58 PERIOD:240000000 ENA=0 RUN=0 NR=2
	240000257 PERF_COUNT_HW_CPU_CYCLES (58)
	239740143 PERF_COUNT_HW_INSTRUCTIONS (59)

IP:0x0000000000400651 PID:10613 TID:10613 TIME:27480626252349
STREAM_ID:58 PERIOD:240000000 ENA=0 RUN=0 NR=2
	480000257 PERF_COUNT_HW_CPU_CYCLES (58)
	479688363 PERF_COUNT_HW_INSTRUCTIONS (59)

Events are also grouped in this example. Both this same issue exists
also when only
one event is used. I suspect an update_event_times() or
update_group_times() is also
missing on the sampling path in perf_overflow_handler().


On Tue, May 11, 2010 at 4:42 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Fri, 2010-05-07 at 18:56 -0700, Corey Ashford wrote:
>> Hi,
>>
>> There appears to be a bug in the kernel related to reading up the time
>> running and enabled values for events that are in a group. ÂThe group
>> leader's time running and time enable values look correct, but all of
>> the other group members have a zero value for their time running and
>> time enabled fields.
>>
>> This happens only when remote monitoring a process (perhaps only after
>> it has terminated)... when self monitoring, the time running/enabled
>> values come out non-zero, and the values are the same for all of the
>> counters (as one would expect since they can be enabled/disabled
>> simultaneously).
>>
>> I've attached a test case which you can place in the tools/perf
>> subdirectory and compile with just:
>>
>> gcc -o show_re_bug show_re_bug.c
>
> The below seems to fix this for me.
>
> ---
> Subject: perf: Fix exit() vs event-groups
> From: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Date: Tue May 11 16:19:10 CEST 2010
>
> Corey reported that the value scale times of group siblings are not
> updated when the monitored task dies.
>
> The problem appears to be that we only update the group leader's
> time values, fix it by updating the whole group.
>
> Reported-by: Corey Ashford <cjashfor@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Cc: Paul Mackerras <paulus@xxxxxxxxx>
> Cc: stable@xxxxxxxxxx
> ---
> Âkernel/perf_event.c | Â 44 +++++++++++++++++++++++---------------------
> Â1 file changed, 23 insertions(+), 21 deletions(-)
>
> Index: linux-2.6/kernel/perf_event.c
> ===================================================================
> --- linux-2.6.orig/kernel/perf_event.c
> +++ linux-2.6/kernel/perf_event.c
> @@ -255,6 +255,18 @@ static void update_event_times(struct pe
> Â Â Â Âevent->total_time_running = run_end - event->tstamp_running;
> Â}
>
> +/*
> + * Update total_time_enabled and total_time_running for all events in a group.
> + */
> +static void update_group_times(struct perf_event *leader)
> +{
> + Â Â Â struct perf_event *event;
> +
> + Â Â Â update_event_times(leader);
> + Â Â Â list_for_each_entry(event, &leader->sibling_list, group_entry)
> + Â Â Â Â Â Â Â update_event_times(event);
> +}
> +
> Âstatic struct list_head *
> Âctx_group_list(struct perf_event *event, struct perf_event_context *ctx)
> Â{
> @@ -320,7 +332,7 @@ list_del_event(struct perf_event *event,
> Â Â Â Âif (event->group_leader != event)
> Â Â Â Â Â Â Â Âevent->group_leader->nr_siblings--;
>
> - Â Â Â update_event_times(event);
> + Â Â Â update_group_times(event);
>
> Â Â Â Â/*
> Â Â Â Â * If event was in error state, then keep it
> @@ -502,18 +514,6 @@ retry:
> Â}
>
> Â/*
> - * Update total_time_enabled and total_time_running for all events in a group.
> - */
> -static void update_group_times(struct perf_event *leader)
> -{
> - Â Â Â struct perf_event *event;
> -
> - Â Â Â update_event_times(leader);
> - Â Â Â list_for_each_entry(event, &leader->sibling_list, group_entry)
> - Â Â Â Â Â Â Â update_event_times(event);
> -}
> -
> -/*
> Â* Cross CPU call to disable a performance event
> Â*/
> Âstatic void __perf_event_disable(void *info)
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/