aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/xen
diff options
context:
space:
mode:
authorDavid Vrabel <david.vrabel@citrix.com>2013-07-19 10:51:58 -0400
committerKonrad Rzeszutek Wilk <konrad.wilk@oracle.com>2013-07-30 09:21:14 -0400
commit179fbd5a45f0d4034cc6fd37b8d367a3b79663c4 (patch)
treec29af1d11cea9ac499fa4ad2e5deab8e2d7a487d /drivers/xen
parent9e7fd145b691127cfd48ab8c05cc1aa6d35b57ad (diff)
xen/evtchn: avoid a deadlock when unbinding an event channel
Unbinding an event channel (either with the ioctl or when the evtchn device is closed) may deadlock because disable_irq() is called with port_user_lock held which is also locked by the interrupt handler. Think of the IOCTL_EVTCHN_UNBIND is being serviced, the routine has just taken the lock, and an interrupt happens. The evtchn_interrupt is invoked, tries to take the lock and spins forever. A quick glance at the code shows that the spinlock is a local IRQ variant. Unfortunately that does not help as "disable_irq() waits for the interrupt handler on all CPUs to stop running. If the irq occurs on another VCPU, it tries to take port_user_lock and can't because the unbind ioctl is holding it." (from David). Hence we cannot depend on the said spinlock to protect us. We could make it a system wide IRQ disable spinlock but there is a better way. We can piggyback on the fact that the existence of the spinlock is to make get_port_user() checks be up-to-date. And we can alter those checks to not depend on the spin lock (as it's protected by u->bind_mutex in the ioctl) and can remove the unnecessary locking (this is IOCTL_EVTCHN_UNBIND) path. In the interrupt handler we cannot use the mutex, but we do not need it. "The unbind disables the irq before making the port user stale, so when you clear it you are guaranteed that the interrupt handler that might use that port cannot be running." (from David). Hence this patch removes the spinlock usage on the teardown path and piggybacks on disable_irq happening before we muck with the get_port_user() data. This ensures that the interrupt handler will never run on stale data. Signed-off-by: David Vrabel <david.vrabel@citrix.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> [v1: Expanded the commit description a bit]
Diffstat (limited to 'drivers/xen')
-rw-r--r--drivers/xen/evtchn.c21
1 files changed, 2 insertions, 19 deletions
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index 8feecf01d55c..b6165e047f48 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -379,18 +379,12 @@ static long evtchn_ioctl(struct file *file,
379 if (unbind.port >= NR_EVENT_CHANNELS) 379 if (unbind.port >= NR_EVENT_CHANNELS)
380 break; 380 break;
381 381
382 spin_lock_irq(&port_user_lock);
383
384 rc = -ENOTCONN; 382 rc = -ENOTCONN;
385 if (get_port_user(unbind.port) != u) { 383 if (get_port_user(unbind.port) != u)
386 spin_unlock_irq(&port_user_lock);
387 break; 384 break;
388 }
389 385
390 disable_irq(irq_from_evtchn(unbind.port)); 386 disable_irq(irq_from_evtchn(unbind.port));
391 387
392 spin_unlock_irq(&port_user_lock);
393
394 evtchn_unbind_from_user(u, unbind.port); 388 evtchn_unbind_from_user(u, unbind.port);
395 389
396 rc = 0; 390 rc = 0;
@@ -490,26 +484,15 @@ static int evtchn_release(struct inode *inode, struct file *filp)
490 int i; 484 int i;
491 struct per_user_data *u = filp->private_data; 485 struct per_user_data *u = filp->private_data;
492 486
493 spin_lock_irq(&port_user_lock);
494
495 free_page((unsigned long)u->ring);
496
497 for (i = 0; i < NR_EVENT_CHANNELS; i++) { 487 for (i = 0; i < NR_EVENT_CHANNELS; i++) {
498 if (get_port_user(i) != u) 488 if (get_port_user(i) != u)
499 continue; 489 continue;
500 490
501 disable_irq(irq_from_evtchn(i)); 491 disable_irq(irq_from_evtchn(i));
502 }
503
504 spin_unlock_irq(&port_user_lock);
505
506 for (i = 0; i < NR_EVENT_CHANNELS; i++) {
507 if (get_port_user(i) != u)
508 continue;
509
510 evtchn_unbind_from_user(get_port_user(i), i); 492 evtchn_unbind_from_user(get_port_user(i), i);
511 } 493 }
512 494
495 free_page((unsigned long)u->ring);
513 kfree(u->name); 496 kfree(u->name);
514 kfree(u); 497 kfree(u);
515 498