WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <JBeulich@xxxxxxxx>, Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] remove blocked time accounting from xen "clockchip"
From: Laszlo Ersek <lersek@xxxxxxxxxx>
Date: Thu, 20 Oct 2011 16:35:28 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Joe Jin <joe.jin@xxxxxxxxxx>, Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Thu, 20 Oct 2011 07:34:42 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E9E9D97020000780005C1DE@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1318970579-6282-1-git-send-email-lersek@xxxxxxxxxx> <4E9E9D97020000780005C1DE@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110928 Fedora/3.1.15-1.fc14 Lightning/1.0b3pre Mnenhy/0.8.4 Thunderbird/3.1.15
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