On 5 September 2012 18:14, Sergey Senozhatsky
<sergey.senozhatsky(a)gmail.com> wrote:
Hi,
On (09/05/12 15:52), Rajagopal Venkat wrote:
> Incorrect timer and work perf events timestamp tracing is one
> of the reason for reporting usage over 100%. This patch will
> resolve the issue by
> - rejecting the events for which entry timestamp is not recorded.
how is that possible?
do you mean erasing between measurements?
schematically:
measure0:
ev1.start
ev2.start
ev2.end
processing
clear
measure1:
ev3.start
ev1.end <<<<<
evX.end <<<<<
These events are causing numbers to go wrong.
delta = time - running_since[timer_struct];
accumulated_runtime += delta
Since running_since[timer_struct] returns zero, event timestamp
itself gets added to accumulated_runtime, causing usage to go
high something like 2693%.
ev3.end
processing
clear
if so, then we're loosing events, which is no good. reporting less than 100%
is ok, but reporting less than real is not.
I did thought of it. Yes, agree that, we are loosing events for which
start timestamp
is not recorded. I believe correct solution would be to consider these
events end
timestamp relative to first_stamp(src/process/do_process.cpp).
p.s.
I'll try to check emails, but most probably will be off-line most of the day.
-ss
> Currently these events exit timestamp itself is considered as
> usage period resulting in over 100% usage.
> - clearing event timestamps from global map at the end of each
> measurement to avoid collision with earlier recorded timestamps.
>
> Signed-off-by: Rajagopal Venkat <rajagopal.venkat(a)linaro.org>
> ---
> src/process/timer.cpp | 5 ++++-
> src/process/work.cpp | 5 ++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/process/timer.cpp b/src/process/timer.cpp
> index 8917490..db074c4 100644
> --- a/src/process/timer.cpp
> +++ b/src/process/timer.cpp
> @@ -79,7 +79,8 @@ uint64_t timer::done(uint64_t time, uint64_t timer_struct)
> {
> int64_t delta;
>
> - if (running_since[timer_struct] > time)
> + if (running_since.find(timer_struct) == running_since.end() ||
> + running_since[timer_struct] > time)
> return 0;
>
> delta = time - running_since[timer_struct];
> @@ -147,6 +148,8 @@ void clear_timers(void)
> all_timers.erase(it);
> it = all_timers.begin();
> }
> +
> + running_since.clear();
> }
>
> bool timer::is_deferred(void)
> diff --git a/src/process/work.cpp b/src/process/work.cpp
> index 82f13a2..e436643 100644
> --- a/src/process/work.cpp
> +++ b/src/process/work.cpp
> @@ -56,7 +56,8 @@ uint64_t work::done(uint64_t time, uint64_t work_struct)
> {
> int64_t delta;
>
> - if (running_since[work_struct] > time)
> + if (running_since.find(work_struct) == running_since.end() ||
> + running_since[work_struct] > time)
> return 0;
>
> delta = time - running_since[work_struct];
> @@ -102,6 +103,8 @@ void clear_work(void)
> all_work.erase(it);
> it = all_work.begin();
> }
> +
> + running_since.clear();
> }
>
>
> --
> 1.7.11.3
>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
>
https://lists.01.org/mailman/listinfo/powertop
>
--
Regards,
Rajagopal