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

[Laszlo Ersek <lersek@redhat.com>]

On 10/19/11 09:51, Jan Beulich wrote:
>>>> On 18.10.11 at 22:42, Laszlo Ersek<lersek@redhat.com>  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