[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCHv1] xen/events: don't bind non-percpu VIRQs with percpu chip



On 05/19/2015 01:50 PM, David Vrabel wrote:
A non-percpu VIRQ (e.g., VIRQ_CONSOLE) may be freed on a different
VCPU than it is bound to.  This can result in a race between
handle_percpu_irq() and removing the action in __free_irq() because
handle_percpu_irq() does not take desc->lock.  The interrupt handler
sees a NULL action and oopses.

Only use the percpu chip/handler for per-CPU VIRQs (like VIRQ_TIMER).

   # cat /proc/interrupts | grep virq
    40:      87246          0  xen-percpu-virq      timer0
    44:          0          0  xen-percpu-virq      debug0
    47:          0      20995  xen-percpu-virq      timer1
    51:          0          0  xen-percpu-virq      debug1
    69:          0          0   xen-dyn-virq      xen-pcpu
    74:          0          0   xen-dyn-virq      mce
    75:         29          0   xen-dyn-virq      hvc_console

Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>

Since we also use percpu handler when binding for IPIs should we also set explicitly set IRQF_PERCPU in bind_ipi_to_irqhandler() (and drop it from call sites)?

Regardless of that,

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>


---
  drivers/tty/hvc/hvc_xen.c        |    2 +-
  drivers/xen/events/events_base.c |   12 ++++++++----
  include/xen/events.h             |    2 +-
  3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 5bab1c6..7a3d146 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -289,7 +289,7 @@ static int xen_initial_domain_console_init(void)
                        return -ENOMEM;
        }
- info->irq = bind_virq_to_irq(VIRQ_CONSOLE, 0);
+       info->irq = bind_virq_to_irq(VIRQ_CONSOLE, 0, false);
        info->vtermno = HVC_COOKIE;
spin_lock(&xencons_lock);
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 2b8553b..3838795 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -957,7 +957,7 @@ unsigned xen_evtchn_nr_channels(void)
  }
  EXPORT_SYMBOL_GPL(xen_evtchn_nr_channels);
-int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
+int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu)
  {
        struct evtchn_bind_virq bind_virq;
        int evtchn, irq, ret;
@@ -971,8 +971,12 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
                if (irq < 0)
                        goto out;
- irq_set_chip_and_handler_name(irq, &xen_percpu_chip,
-                                             handle_percpu_irq, "virq");
+               if (percpu)
+                       irq_set_chip_and_handler_name(irq, &xen_percpu_chip,
+                                                     handle_percpu_irq, 
"virq");
+               else
+                       irq_set_chip_and_handler_name(irq, &xen_dynamic_chip,
+                                                     handle_edge_irq, "virq");
bind_virq.virq = virq;
                bind_virq.vcpu = cpu;
@@ -1062,7 +1066,7 @@ int bind_virq_to_irqhandler(unsigned int virq, unsigned 
int cpu,
  {
        int irq, retval;
- irq = bind_virq_to_irq(virq, cpu);
+       irq = bind_virq_to_irq(virq, cpu, irqflags & IRQF_PERCPU);
        if (irq < 0)
                return irq;
        retval = request_irq(irq, handler, irqflags, devname, dev_id);
diff --git a/include/xen/events.h b/include/xen/events.h
index 5321cd9..7d95fdf 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -17,7 +17,7 @@ int bind_evtchn_to_irqhandler(unsigned int evtchn,
                              irq_handler_t handler,
                              unsigned long irqflags, const char *devname,
                              void *dev_id);
-int bind_virq_to_irq(unsigned int virq, unsigned int cpu);
+int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu);
  int bind_virq_to_irqhandler(unsigned int virq, unsigned int cpu,
                            irq_handler_t handler,
                            unsigned long irqflags, const char *devname,


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.