Re: [Xen-ia64-devel] [PATCH][3/3] Steal time accounting for PVdomain/IA64 TAKE2

[Aron Griffis <aron@xxxxxx>]

Hello Atsushi and Isaku,

I'm considering filing a Red Hat bugzilla to request
http://xenbits.xensource.com/xen-unstable.hg?rev/204046d99562 in
RHEL5.x.  However I would like to understand the implication
better so I can describe it in the bz.

Without 204046d99562, I think the steal time accounting will have
stale values after a suspend/resume.  But could you explain what
effect this ultimately has on the domain?  Is it only a problem
in the top reporting or is there a deeper concern?

Also I noticed what appears to be a small error in the original
consider_steal_time patch, see below:

Atsushi SAKAI wrote:  [Fri Mar 09 2007, 02:59:13AM EST]
> +#ifdef CONFIG_XEN
> +static unsigned long 
> +consider_steal_time(unsigned long new_itm, struct pt_regs *regs)
> +{
> +     unsigned long delta_itm = 0;
> +     unsigned long stolen, blocked;
> +     unsigned long sched_time;
> +     unsigned long stolentick = 0;
> +     int cpu = smp_processor_id();
> +     int i;
> +     struct vcpu_runstate_info *runstate = &per_cpu(runstate, 
> smp_processor_id());
> +     struct task_struct *p = current;

Above delta_itm is initialized to 0

> +     do {
> +             sched_time = runstate->state_entry_time;
> +             mb();
> +             stolen =runstate->time[RUNSTATE_runnable] + 
> +                     runstate->time[RUNSTATE_offline] -
> +                     per_cpu(processed_stolen_time,cpu);
> +             blocked =runstate->time[RUNSTATE_blocked] -
> +                     per_cpu(processed_blocked_time,cpu);
> +             mb();
> +     } while (sched_time != runstate->state_entry_time);
> +
> +     /* check for vcpu migration effect
> +      * In this case, itc value is reversed.
> +      * This causes huge stolen value.  
> +      * This function just checks and reject this effect.
> +      */
> +     if(!time_after_eq(runstate->time[RUNSTATE_blocked],
> +             per_cpu(processed_blocked_time,cpu)))
> +                     blocked = 0;
> +
> +     if(!time_after_eq(runstate->time[RUNSTATE_runnable]+
> +             runstate->time[RUNSTATE_offline],
> +             per_cpu(processed_stolen_time,cpu)))
> +                     stolen = 0;
> +
> +     if(!time_after(delta_itm + new_itm, ia64_get_itc()))
> +             stolentick = ia64_get_itc() - delta_itm - new_itm;

Here delta_itm is used (twice), but I think it will always be
zero here.  Is this a mistake?

Thanks,
Aron

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