Re: [Xen-devel] [PATCH 1/2] xen/x86: Move irq allocation from Xen smp_op.cpu_up()

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 03/25/2016 11:10 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 17, 2016 at 09:33:32AM -0400, Boris Ostrovsky wrote:
> > Commit ce0d3c0a6fb1 ("genirq: Revert sparse irq locking around
> > __cpu_up() and move it to x86 for now") reverted irq locking
> > introduced by commit a89941816726 ("hotplug: Prevent alloc/free
> > of irq descriptors during cpu up/down") because of Xen allocating
> > irqs in both of its cpu_up ops.
> >
> > We can move those allocations into CPU notifiers so that original
> > patch can be reinstated.
> Original being "hotplug: Prevent alloc/free..." ?
Yes.

> -static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long 
> action,
> -                             void *hcpu)
> +static int xen_cpu_notify(struct notifier_block *self, unsigned long action,
> +                         void *hcpu)
>   {
>         int cpu = (long)hcpu;
> +       int rc;
> +
>         switch (action) {
>         case CPU_UP_PREPARE:
> -               xen_vcpu_setup(cpu);
> -               if (xen_have_vector_callback) {
> -                       if (xen_feature(XENFEAT_hvm_safe_pvclock))
> -                               xen_setup_timer(cpu);
> +               if (xen_hvm_domain()) {
> +                       /*
> +                        * This can happen if CPU was offlined earlier and
> +                        * offlining timed out in common_cpu_die().
> +                        */
> +                       if (cpu_report_state(cpu) == CPU_DEAD_FROZEN) {
> +                               xen_smp_intr_free(cpu);
> +                               xen_uninit_lock_cpu(cpu);
> +                       }
> +
> +                       xen_vcpu_setup(cpu);
>                 }
> +
> +               if (xen_pv_domain() ||
> +                   (xen_have_vector_callback &&
> +                    xen_feature(XENFEAT_hvm_safe_pvclock)))
> +                       xen_setup_timer(cpu);
> +
> +               rc = xen_smp_intr_init(cpu);
> +               if (rc) {
> +                       WARN(1, "xen_smp_intr_init() for CPU %d failed: %d\n",
> +                            cpu, rc);
> +                       return NOTIFY_BAD;
> +               }
> +
> +               break;
> +       case CPU_ONLINE:
> +               xen_init_lock_cpu(cpu);
> +               break;
> +       case CPU_UP_CANCELED:
> +               xen_smp_intr_free(cpu);
> xen_uninit_lock_cpu ?
I don't think this is needed: we initialize lock in CPU_ONLINE notifier 
which can only be called after CPU_UP_CANCELED would have run (in which 
case we'll never do CPU_ONLINE)
-boris

> > +               if (xen_pv_domain() ||
> > +                   (xen_have_vector_callback &&
> > +                    xen_feature(XENFEAT_hvm_safe_pvclock)))
> > +                       xen_teardown_timer(cpu);
> >                 break;
> >         default:
> >                 break;

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