Re: [PATCH 2/3] PERF(kernel): Cleanup power events

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/25/2010 2:41 AM, Thomas Renninger wrote:
On Monday 25 October 2010 08:54:34 Arjan van de Ven wrote:
On 10/19/2010 4:36 AM, Thomas Renninger wrote:
   static void poll_idle(void)
   {
-	trace_power_start(POWER_CSTATE, 0, smp_processor_id());
   	local_irq_enable();
   	while (!need_resched())
   		cpu_relax();
-	trace_power_end(0);
   }
why did you remove the idle tracepoints from this one ???
Because no idle/sleep state is entered here.
State 0 does not exist or say, it means the machine is not idle.
The new event uses idle state 0 spec conform as "exit sleep state".

If this should still be trackable some kind of dummy sleep state:
#define IDLE_BUSY_LOOP 0xFE
(or similar) must get defined and passed like this:
trace_processor_idle(IDLE_BUSY_LOOP, smp_processor_id());
     cpu_relax()
trace_processor_idle(0, smp_processor_id());

I could imagine this is somewhat worth it to compare idle results
to "no idle state at all" is used.
But nobody should ever use idle=poll, comparing deep sleep states
with C1 with (idle=halt) should be sufficient?

this is not idle=poll on the command line only.
this also gets used normally, in two cases
1) during real time operations, for some short periods of time
    (think wallstreet trading)
2) by the menu governor when the next event is less than a few microseconds away, so short that even C1 is too much

I know that your new API tries to use "0" as exit, but 0 is already taken (in all power terminology at least on x86 it is) for this.

why isn't your "exit" a special define?


also, if you look at many other similar perf events, they ever separate entry/exit points:

process/do_process.cpp: perf_events->add_event("irq:irq_handler_entry"); process/do_process.cpp: perf_events->add_event("irq:irq_handler_exit");
process/do_process.cpp:         perf_events->add_event("irq:softirq_entry");
process/do_process.cpp:         perf_events->add_event("irq:softirq_exit");
process/do_process.cpp: perf_events->add_event("timer:timer_expire_entry"); process/do_process.cpp: perf_events->add_event("timer:timer_expire_exit"); process/do_process.cpp: perf_events->add_event("timer:hrtimer_expire_entry"); process/do_process.cpp: perf_events->add_event("timer:hrtimer_expire_exit");
process/do_process.cpp:         perf_events->add_event("power:power_start");
process/do_process.cpp:         perf_events->add_event("power:power_end");
process/do_process.cpp: perf_events->add_event("workqueue:workqueue_execute_start"); process/do_process.cpp: perf_events->add_event("workqueue:workqueue_execute_end");

so there is already an API consistency precedent
(and frankly, trying to multiplex in "exit" via a magic value is asking for trouble API wise)

--
To unsubscribe from this list: send the line "unsubscribe linux-trace-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Development]     [Linux USB Development]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux