Commit b1766b62890e3bba1a778a20ef8bf9348d6096c2

Authored by Konrad Rzeszutek Wilk
1 parent 402c5e15b4

xen/pciback: Use mutexes when working with Xenbus state transitions.

The caller that orchestrates the state changes is xenwatch_thread
and it takes a mutex. In our processing of Xenbus states we can take
the luxery of going to sleep on a mutex, so lets do that and
also fix this bug:

BUG: sleeping function called from invalid context at /linux/kernel/mutex.c:271
in_atomic(): 1, irqs_disabled(): 0, pid: 32, name: xenwatch
2 locks held by xenwatch/32:
 #0:  (xenwatch_mutex){......}, at: [<ffffffff813856ab>] xenwatch_thread+0x4b/0x180
 #1:  (&(&pdev->dev_lock)->rlock){......}, at: [<ffffffff8138f05b>] xen_pcibk_disconnect+0x1b/0x80
Pid: 32, comm: xenwatch Not tainted 3.1.0-rc6-00015-g3ce340d #2
Call Trace:
 [<ffffffff810892b2>] __might_sleep+0x102/0x130
 [<ffffffff8163b90f>] mutex_lock_nested+0x2f/0x50
 [<ffffffff81382c1c>] unbind_from_irq+0x2c/0x1b0
 [<ffffffff8110da66>] ? free_irq+0x56/0xb0
 [<ffffffff81382dbc>] unbind_from_irqhandler+0x1c/0x30
 [<ffffffff8138f06b>] xen_pcibk_disconnect+0x2b/0x80
 [<ffffffff81390348>] xen_pcibk_frontend_changed+0xe8/0x140
 [<ffffffff81387ac2>] xenbus_otherend_changed+0xd2/0x150
 [<ffffffff810895c1>] ? get_parent_ip+0x11/0x50
 [<ffffffff81387de0>] frontend_changed+0x10/0x20
 [<ffffffff81385712>] xenwatch_thread+0xb2/0x180

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Showing 2 changed files with 10 additions and 14 deletions Side-by-side Diff

drivers/xen/xen-pciback/pciback.h
... ... @@ -29,7 +29,7 @@
29 29  
30 30 struct xen_pcibk_device {
31 31 void *pci_dev_data;
32   - spinlock_t dev_lock;
  32 + struct mutex dev_lock;
33 33 struct xenbus_device *xdev;
34 34 struct xenbus_watch be_watch;
35 35 u8 be_watching;
drivers/xen/xen-pciback/xenbus.c
... ... @@ -43,7 +43,7 @@
43 43 pdev->xdev = xdev;
44 44 dev_set_drvdata(&xdev->dev, pdev);
45 45  
46   - spin_lock_init(&pdev->dev_lock);
  46 + mutex_init(&pdev->dev_lock);
47 47  
48 48 pdev->sh_info = NULL;
49 49 pdev->evtchn_irq = INVALID_EVTCHN_IRQ;
50 50  
... ... @@ -61,14 +61,12 @@
61 61  
62 62 static void xen_pcibk_disconnect(struct xen_pcibk_device *pdev)
63 63 {
64   - spin_lock(&pdev->dev_lock);
65   -
  64 + mutex_lock(&pdev->dev_lock);
66 65 /* Ensure the guest can't trigger our handler before removing devices */
67 66 if (pdev->evtchn_irq != INVALID_EVTCHN_IRQ) {
68 67 unbind_from_irqhandler(pdev->evtchn_irq, pdev);
69 68 pdev->evtchn_irq = INVALID_EVTCHN_IRQ;
70 69 }
71   - spin_unlock(&pdev->dev_lock);
72 70  
73 71 /* If the driver domain started an op, make sure we complete it
74 72 * before releasing the shared memory */
75 73  
... ... @@ -76,13 +74,11 @@
76 74 /* Note, the workqueue does not use spinlocks at all.*/
77 75 flush_workqueue(xen_pcibk_wq);
78 76  
79   - spin_lock(&pdev->dev_lock);
80 77 if (pdev->sh_info != NULL) {
81 78 xenbus_unmap_ring_vfree(pdev->xdev, pdev->sh_info);
82 79 pdev->sh_info = NULL;
83 80 }
84   - spin_unlock(&pdev->dev_lock);
85   -
  81 + mutex_unlock(&pdev->dev_lock);
86 82 }
87 83  
88 84 static void free_pdev(struct xen_pcibk_device *pdev)
89 85  
... ... @@ -119,9 +115,7 @@
119 115 goto out;
120 116 }
121 117  
122   - spin_lock(&pdev->dev_lock);
123 118 pdev->sh_info = vaddr;
124   - spin_unlock(&pdev->dev_lock);
125 119  
126 120 err = bind_interdomain_evtchn_to_irqhandler(
127 121 pdev->xdev->otherend_id, remote_evtchn, xen_pcibk_handle_event,
128 122  
... ... @@ -131,10 +125,7 @@
131 125 "Error binding event channel to IRQ");
132 126 goto out;
133 127 }
134   -
135   - spin_lock(&pdev->dev_lock);
136 128 pdev->evtchn_irq = err;
137   - spin_unlock(&pdev->dev_lock);
138 129 err = 0;
139 130  
140 131 dev_dbg(&pdev->xdev->dev, "Attached!\n");
... ... @@ -149,6 +140,7 @@
149 140 char *magic = NULL;
150 141  
151 142  
  143 + mutex_lock(&pdev->dev_lock);
152 144 /* Make sure we only do this setup once */
153 145 if (xenbus_read_driver_state(pdev->xdev->nodename) !=
154 146 XenbusStateInitialised)
... ... @@ -193,6 +185,7 @@
193 185  
194 186 dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err);
195 187 out:
  188 + mutex_unlock(&pdev->dev_lock);
196 189  
197 190 kfree(magic);
198 191  
... ... @@ -368,6 +361,7 @@
368 361  
369 362 dev_dbg(&pdev->xdev->dev, "Reconfiguring device ...\n");
370 363  
  364 + mutex_lock(&pdev->dev_lock);
371 365 /* Make sure we only reconfigure once */
372 366 if (xenbus_read_driver_state(pdev->xdev->nodename) !=
373 367 XenbusStateReconfiguring)
... ... @@ -505,6 +499,7 @@
505 499 }
506 500  
507 501 out:
  502 + mutex_unlock(&pdev->dev_lock);
508 503 return 0;
509 504 }
510 505  
... ... @@ -561,6 +556,7 @@
561 556 char dev_str[64];
562 557 char state_str[64];
563 558  
  559 + mutex_lock(&pdev->dev_lock);
564 560 /* It's possible we could get the call to setup twice, so make sure
565 561 * we're not already connected.
566 562 */
567 563  
... ... @@ -641,10 +637,10 @@
641 637 "Error switching to initialised state!");
642 638  
643 639 out:
  640 + mutex_unlock(&pdev->dev_lock);
644 641 if (!err)
645 642 /* see if pcifront is already configured (if not, we'll wait) */
646 643 xen_pcibk_attach(pdev);
647   -
648 644 return err;
649 645 }
650 646