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