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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Jan 20, 2012 at 09:57:04AM +0000, Jan Beulich wrote:
> >>> On 19.01.12 at 20:42, Konrad Rzeszutek Wilk <konrad@darnok.org> wrote:
> > I finally got some time to look at them and I think they are these ones:
> > 
> > git log --oneline 
> > e03b644fe68b1c6401465b02724d261538dba10f..3c404b578fab699c4708279938078d9404b
> > 255a4 
> > 3c404b5 KVM guest: Add a pv_ops stub for steal time
> > c9aaa89 KVM: Steal time implementation
> > 9ddabbe KVM: KVM Steal time guest/host interface
> > 4b6b35f KVM: Add constant to represent KVM MSRs enabled bit in guest/host 
> > interface
> > 
> > What is interesting is that they end up inserting a bunch of:
> > 
> >  
> > +       if (steal_account_process_tick())
> > +               return;
> > +
> > 
> > in irqtime_account_process_tick and in account_process_tick.
> 
> And this (particularly the "return" part of it) is what I have a hard
> time to understand: How can it be correct to not do any of the
> other accounting? After all, the function calls only
> account_steal_time(), but its certainly going to be common that
> part of the time was stolen, and part was spent executing.
> 
> Further, it's being called only from the process tick accounting
> functions, but clearly part of idle or interrupt time can also be
> stolen.

I took a stab at trying to implement this using this API and basing
it on top Laszlo patch.. But I've doubts about it too and I haven't
run any benchmarks past booting a guest.


commit b1acd2adad821fd87da6941c38f0dbaddd37dc6b
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date:   Mon Jan 23 14:21:20 2012 -0500

    xen/time: Use the pv_ops steal_time.
    
    In the past we were using do_stolen_account() to update the
    stolen time and this was done when by calling account_steal_ticks().
    
    The do_stolen_account() was called from xen_timer_interrupt(). The
    xen_timer_interrupt() would be called periodically (or frequently)
    depending on what the timer decided. We would piggy-back on the
    timer IRQ to update the steal time and idle time (fixed by
    'remove blocked time accounting from xen "clockchip"' patch).
    
    Instead of piggy-backing on the timer interrupt lets do it
    via the provided API pv-ops time calls - the steal_clock.
    
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 5c4a7f8..59cdd1b 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -71,14 +71,14 @@ static u64 get64(const u64 *p)
 /*
  * Runstate accounting
  */
-static void get_runstate_snapshot(struct vcpu_runstate_info *res)
+static void get_runstate_snapshot(struct vcpu_runstate_info *res, int cpu)
 {
 	u64 state_time;
 	struct vcpu_runstate_info *state;
 
 	BUG_ON(preemptible());
 
-	state = &__get_cpu_var(xen_runstate);
+	state = &per_cpu(xen_runstate, cpu);
 
 	/*
 	 * The runstate info is always updated by the hypervisor on
@@ -110,18 +110,18 @@ void xen_setup_runstate_info(int cpu)
 		BUG();
 }
 
-static void do_stolen_accounting(void)
+static u64 xen_steal_clock(int cpu)
 {
 	struct vcpu_runstate_info state;
 	struct vcpu_runstate_info *snap;
 	s64 runnable, offline, stolen;
-	cputime_t ticks;
+	u64 ticks;
 
-	get_runstate_snapshot(&state);
+	get_runstate_snapshot(&state, cpu);
 
 	WARN_ON(state.state != RUNSTATE_running);
 
-	snap = &__get_cpu_var(xen_runstate_snapshot);
+	snap = &per_cpu(xen_runstate_snapshot, cpu);
 
 	/* work out how much time the VCPU has not been runn*ing*  */
 	runnable = state.time[RUNSTATE_runnable] - snap->time[RUNSTATE_runnable];
@@ -138,7 +138,8 @@ static void do_stolen_accounting(void)
 
 	ticks = iter_div_u64_rem(stolen, NS_PER_TICK, &stolen);
 	__this_cpu_write(xen_residual_stolen, stolen);
-	account_steal_ticks(ticks);
+
+	return ticks;
 }
 
 /* Get the TSC speed from Xen */
@@ -377,8 +378,6 @@ static irqreturn_t xen_timer_interrupt(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 	}
 
-	do_stolen_accounting();
-
 	return ret;
 }
 
@@ -439,6 +438,7 @@ void xen_timer_resume(void)
 
 static const struct pv_time_ops xen_time_ops __initconst = {
 	.sched_clock = xen_clocksource_read,
+	.steal_clock = xen_steal_clock,
 };
 
 static void __init xen_time_init(void)
@@ -464,6 +464,9 @@ static void __init xen_time_init(void)
 	xen_setup_runstate_info(cpu);
 	xen_setup_timer(cpu);
 	xen_setup_cpu_clockevents();
+
+	jump_label_inc(&paravirt_steal_enabled);
+	jump_label_inc(&paravirt_steal_rq_enabled);
 }
 
 void __init xen_init_time_ops(void)
> 
> Jan