Re: [Xen-devel] [PATCH 2 of 2] Avoid vcpu migration of paused vcpus

[Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>]

On 03/22/2012 11:26 AM, Jan Beulich wrote:
> > > > On 22.03.12 at 09:22, Juergen Gross<juergen.gross@xxxxxxxxxxxxxx>  wrote:
> > --- a/xen/common/schedule.c     Thu Mar 22 09:21:44 2012 +0100
> > +++ b/xen/common/schedule.c     Thu Mar 22 09:21:47 2012 +0100
> > @@ -525,6 +525,47 @@ void vcpu_force_reschedule(struct vcpu *
> >      }
> > }
> >
> > +static int vcpu_chk_affinity(struct vcpu *v, unsigned int cpu)
> > +{
> > +    cpumask_t online_affinity;
> > +    struct cpupool *c;
> > +
> > +    c = v->domain->cpupool;
> > +    cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
> > +    if ( cpumask_empty(&online_affinity)&&
> There's no need for a local cpumask_t variable here - please use
> !cpumask_intersects(v->cpu_affinity, c->cpu_valid) instead.
Okay.

> > +         cpumask_test_cpu(cpu, v->cpu_affinity) )
> > +    {
> > +        printk("Breaking vcpu affinity for domain %d vcpu %d\n",
> > +                v->domain->domain_id, v->vcpu_id);
> > +        cpumask_setall(v->cpu_affinity);
> > +    }
> > +
> > +    return ( !cpumask_test_cpu(cpu, c->cpu_valid)&&  (v->processor == cpu) );
> Please drop the extra outermost parentheses.
Okay.

> > +}
> > +
> > +void vcpu_arouse(struct vcpu *v)
> > +{
> > +    unsigned long flags;
> > +
> > +    if ( atomic_read(&v->pause_count) ||
> > +         atomic_read(&v->domain->pause_count) )
> Is it not possible (or even more correct) to use vcpu_runnable() here?
No. There might be flags set in v->pause_flags which I don't want to test here.
I'm only interested in "real" paused state. An old "migrating" case should be
reconsidered here, e.g.

> > +        return;
> > +
> > +    vcpu_schedule_lock_irqsave(v, flags);
> > +
> > +    if ( unlikely(vcpu_chk_affinity(v, v->processor)&&  (v != current)) )
> unlikely() is generally useful only around individual conditions (i.e.
> not around&&  or || expressions).
Ah, I didn't know that. Will change it.

> > +    {
> > +        set_bit(_VPF_migrating,&v->pause_flags);
> > +        vcpu_schedule_unlock_irqrestore(v, flags);
> > +        vcpu_migrate(v);
> > +        return;
> > +    }
> > +
> > +    vcpu_schedule_unlock_irqrestore(v, flags);
> > +
> > +    vcpu_wake(v);
> > +}
> > +
> > /*
> >   * This function is used by cpu_hotplug code from stop_machine context
> >   * and from cpupools to switch schedulers on a cpu.
> > @@ -534,7 +575,6 @@ int cpu_disable_scheduler(unsigned int c
> >      struct domain *d;
> >      struct vcpu *v;
> >      struct cpupool *c;
> > -    cpumask_t online_affinity;
> >      int    ret = 0;
> >
> >      c = per_cpu(cpupool, cpu);
> > @@ -547,34 +587,33 @@ int cpu_disable_scheduler(unsigned int c
> >          {
> >              vcpu_schedule_lock_irq(v);
> >
> > -            cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
> > -            if ( cpumask_empty(&online_affinity)&&
> > -                 cpumask_test_cpu(cpu, v->cpu_affinity) )
> > +            if ( likely(!atomic_read(&v->pause_count)&&
> > +                        !atomic_read(&d->pause_count)) )
> Same question as above regarding vcpu_runnable().
Same answer :-)

> >              {
> > -                printk("Breaking vcpu affinity for domain %d vcpu %d\n",
> > -                        v->domain->domain_id, v->vcpu_id);
> > -                cpumask_setall(v->cpu_affinity);
> > -            }
> > +                if ( vcpu_chk_affinity(v, cpu) )
> > +                {
> > +                    set_bit(_VPF_migrating,&v->pause_flags);
> > +                    vcpu_schedule_unlock_irq(v);
> > +                    vcpu_sleep_nosync(v);
> > +                    vcpu_migrate(v);
> > +                }
> > +                else
> > +                {
> > +                    vcpu_schedule_unlock_irq(v);
> > +                }
> Please drop the unnecessary braces here, as per the recently posted
> coding style draft.
Okay.


Juergen

--
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@xxxxxxxxxxxxxx
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel