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

Re: [Xen-devel] [PATCH] remove blocked time accounting from xen "clockchip"



On 10/19/11 09:51, Jan Beulich wrote:
On 18.10.11 at 22:42, Laszlo Ersek<lersek@xxxxxxxxxx>  wrote:
... because the "clock_event_device framework" already accounts for idle
time through the "event_handler" function pointer in
xen_timer_interrupt().

As event_handler is being checked to be non-zero, shouldn't the
code you remove simply become conditional (upon event_handler
being zero)?

After experimenting further and reading more code, I think that the event_handler == NULL case is spurious and impossible in the long run. (I tested faking it periodically and the VM stops progressing during boot, after udev is started).

Furthermore / independently, these are the callers of account_idle_ticks() -- a complete tree:

account_idle_ticks() [kernel/sched.c]
  <- tick_nohz_restart_sched_tick() [kernel/time/tick-sched.c]
    <- cpu_idle() [various arches]

  <- do_stolen_accounting() [arch/x86/xen/time.c]
    <- xen_timer_interrupt()

  <- consider_steal_time() [arch/ia64/xen/time.c]
    <- xen_do_steal_accounting()
       = xen_time_ops.do_steal_accounting

(The ia64/xen time code seems to be modeled after the x86/xen code.)

Jeremy, could you please educate me why the original version of do_stolen_accounting() (commit f91a8b44) had added the

    account_steal_time(idle_task(smp_processor_id()), ticks);

part? I think it was wrong from the start.

In linux-2.6.18-xen, cpu_idle() [arch/x86_64/kernel/process-xen.c] doesn't seem to bump the idle time counter. So the interrupt handler routine timer_interrupt() [arch/i386/kernel/time-xen.c] has to, after doing the stolen accounting. I suspect this logic was transferred to the pvops kernel superfluously, where cpu_idle() was already handling the idle time accounting.

I believe my claim is consistent with the fact that only the xen-specific timer interrupt handlers care directly about idle time in the pvops kernel.

I'm convinced the patch is correct, and only the commit message might need a small fix (mentioning cpu_idle()). If the above reasoning is insufficient, whom should I mail directly to confirm/refute/complete it? I tried mingo and tglx in private, but got no answer yet.

If you can accept the reasoning, I'll resend the patch with an updated commit message.

Thank you very much,
Laszlo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.