From 402c5e15b44070461dcc2f41536c16d0cfbca9c3 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Wed, 21 Sep 2011 16:22:11 -0400 Subject: xen/pciback: miscellaneous adjustments This is a minor bugfix and a set of small cleanups; as it is not clear whether this needs splitting into pieces (and if so, at what granularity), it is a single combined patch. - add a missing return statement to an error path in kill_domain_by_device() - use pci_is_enabled() rather than raw atomic_read() - remove a bogus attempt to zero-terminate an already zero-terminated string - #define DRV_NAME once uniformly in the shared local header - make DRIVER_ATTR() variables static - eliminate a pointless use of list_for_each_entry_safe() - add MODULE_ALIAS() - a little bit of constification - adjust a few messages - remove stray semicolons from inline function definitions Signed-off-by: Jan Beulich [v1: Dropped the resource_size fix, altered the description] [v2: Fixed cleanpatch.pl comments] Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/xen-pciback/pciback.h | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'drivers/xen/xen-pciback/pciback.h') diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h index a0e131a8150..d095acdc0e6 100644 --- a/drivers/xen/xen-pciback/pciback.h +++ b/drivers/xen/xen-pciback/pciback.h @@ -15,6 +15,8 @@ #include #include +#define DRV_NAME "xen-pciback" + struct pci_dev_entry { struct list_head list; struct pci_dev *dev; @@ -89,7 +91,7 @@ typedef int (*publish_pci_root_cb) (struct xen_pcibk_device *pdev, * passthrough - BDFs are exactly like in the host. */ struct xen_pcibk_backend { - char *name; + const char *name; int (*init)(struct xen_pcibk_device *pdev); void (*free)(struct xen_pcibk_device *pdev); int (*find)(struct pci_dev *pcidev, struct xen_pcibk_device *pdev, @@ -104,9 +106,9 @@ struct xen_pcibk_backend { unsigned int devfn); }; -extern struct xen_pcibk_backend xen_pcibk_vpci_backend; -extern struct xen_pcibk_backend xen_pcibk_passthrough_backend; -extern struct xen_pcibk_backend *xen_pcibk_backend; +extern const struct xen_pcibk_backend xen_pcibk_vpci_backend; +extern const struct xen_pcibk_backend xen_pcibk_passthrough_backend; +extern const struct xen_pcibk_backend *xen_pcibk_backend; static inline int xen_pcibk_add_pci_dev(struct xen_pcibk_device *pdev, struct pci_dev *dev, @@ -116,13 +118,14 @@ static inline int xen_pcibk_add_pci_dev(struct xen_pcibk_device *pdev, if (xen_pcibk_backend && xen_pcibk_backend->add) return xen_pcibk_backend->add(pdev, dev, devid, publish_cb); return -1; -}; +} + static inline void xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev, struct pci_dev *dev) { if (xen_pcibk_backend && xen_pcibk_backend->free) return xen_pcibk_backend->release(pdev, dev); -}; +} static inline struct pci_dev * xen_pcibk_get_pci_dev(struct xen_pcibk_device *pdev, unsigned int domain, @@ -131,7 +134,8 @@ xen_pcibk_get_pci_dev(struct xen_pcibk_device *pdev, unsigned int domain, if (xen_pcibk_backend && xen_pcibk_backend->get) return xen_pcibk_backend->get(pdev, domain, bus, devfn); return NULL; -}; +} + /** * Add for domain0 PCIE-AER handling. Get guest domain/bus/devfn in xen_pcibk * before sending aer request to pcifront, so that guest could identify @@ -148,25 +152,29 @@ static inline int xen_pcibk_get_pcifront_dev(struct pci_dev *pcidev, return xen_pcibk_backend->find(pcidev, pdev, domain, bus, devfn); return -1; -}; +} + static inline int xen_pcibk_init_devices(struct xen_pcibk_device *pdev) { if (xen_pcibk_backend && xen_pcibk_backend->init) return xen_pcibk_backend->init(pdev); return -1; -}; +} + static inline int xen_pcibk_publish_pci_roots(struct xen_pcibk_device *pdev, publish_pci_root_cb cb) { if (xen_pcibk_backend && xen_pcibk_backend->publish) return xen_pcibk_backend->publish(pdev, cb); return -1; -}; +} + static inline void xen_pcibk_release_devices(struct xen_pcibk_device *pdev) { if (xen_pcibk_backend && xen_pcibk_backend->free) return xen_pcibk_backend->free(pdev); -}; +} + /* Handles events from front-end */ irqreturn_t xen_pcibk_handle_event(int irq, void *dev_id); void xen_pcibk_do_op(struct work_struct *data); -- cgit v1.2.3-70-g09d2 From b1766b62890e3bba1a778a20ef8bf9348d6096c2 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Fri, 16 Sep 2011 14:43:14 -0400 Subject: 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: [] xenwatch_thread+0x4b/0x180 #1: (&(&pdev->dev_lock)->rlock){......}, at: [] xen_pcibk_disconnect+0x1b/0x80 Pid: 32, comm: xenwatch Not tainted 3.1.0-rc6-00015-g3ce340d #2 Call Trace: [] __might_sleep+0x102/0x130 [] mutex_lock_nested+0x2f/0x50 [] unbind_from_irq+0x2c/0x1b0 [] ? free_irq+0x56/0xb0 [] unbind_from_irqhandler+0x1c/0x30 [] xen_pcibk_disconnect+0x2b/0x80 [] xen_pcibk_frontend_changed+0xe8/0x140 [] xenbus_otherend_changed+0xd2/0x150 [] ? get_parent_ip+0x11/0x50 [] frontend_changed+0x10/0x20 [] xenwatch_thread+0xb2/0x180 Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/xen-pciback/pciback.h | 2 +- drivers/xen/xen-pciback/xenbus.c | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) (limited to 'drivers/xen/xen-pciback/pciback.h') diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h index d095acdc0e6..e9b4011c5f9 100644 --- a/drivers/xen/xen-pciback/pciback.h +++ b/drivers/xen/xen-pciback/pciback.h @@ -29,7 +29,7 @@ struct pci_dev_entry { struct xen_pcibk_device { void *pci_dev_data; - spinlock_t dev_lock; + struct mutex dev_lock; struct xenbus_device *xdev; struct xenbus_watch be_watch; u8 be_watching; diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c index 522f2daa96c..474d52ec337 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) pdev->xdev = xdev; dev_set_drvdata(&xdev->dev, pdev); - spin_lock_init(&pdev->dev_lock); + mutex_init(&pdev->dev_lock); pdev->sh_info = NULL; pdev->evtchn_irq = INVALID_EVTCHN_IRQ; @@ -61,14 +61,12 @@ out: static void xen_pcibk_disconnect(struct xen_pcibk_device *pdev) { - spin_lock(&pdev->dev_lock); - + mutex_lock(&pdev->dev_lock); /* Ensure the guest can't trigger our handler before removing devices */ if (pdev->evtchn_irq != INVALID_EVTCHN_IRQ) { unbind_from_irqhandler(pdev->evtchn_irq, pdev); pdev->evtchn_irq = INVALID_EVTCHN_IRQ; } - spin_unlock(&pdev->dev_lock); /* If the driver domain started an op, make sure we complete it * before releasing the shared memory */ @@ -76,13 +74,11 @@ static void xen_pcibk_disconnect(struct xen_pcibk_device *pdev) /* Note, the workqueue does not use spinlocks at all.*/ flush_workqueue(xen_pcibk_wq); - spin_lock(&pdev->dev_lock); if (pdev->sh_info != NULL) { xenbus_unmap_ring_vfree(pdev->xdev, pdev->sh_info); pdev->sh_info = NULL; } - spin_unlock(&pdev->dev_lock); - + mutex_unlock(&pdev->dev_lock); } 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, goto out; } - spin_lock(&pdev->dev_lock); pdev->sh_info = vaddr; - spin_unlock(&pdev->dev_lock); err = bind_interdomain_evtchn_to_irqhandler( 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, "Error binding event channel to IRQ"); goto out; } - - spin_lock(&pdev->dev_lock); pdev->evtchn_irq = err; - spin_unlock(&pdev->dev_lock); err = 0; dev_dbg(&pdev->xdev->dev, "Attached!\n"); @@ -149,6 +140,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev) char *magic = NULL; + mutex_lock(&pdev->dev_lock); /* Make sure we only do this setup once */ if (xenbus_read_driver_state(pdev->xdev->nodename) != XenbusStateInitialised) @@ -193,6 +185,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev) dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err); out: + mutex_unlock(&pdev->dev_lock); kfree(magic); @@ -368,6 +361,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev) dev_dbg(&pdev->xdev->dev, "Reconfiguring device ...\n"); + mutex_lock(&pdev->dev_lock); /* Make sure we only reconfigure once */ if (xenbus_read_driver_state(pdev->xdev->nodename) != XenbusStateReconfiguring) @@ -505,6 +499,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev) } out: + mutex_unlock(&pdev->dev_lock); return 0; } @@ -561,6 +556,7 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev) char dev_str[64]; char state_str[64]; + mutex_lock(&pdev->dev_lock); /* It's possible we could get the call to setup twice, so make sure * we're not already connected. */ @@ -641,10 +637,10 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev) "Error switching to initialised state!"); out: + mutex_unlock(&pdev->dev_lock); if (!err) /* see if pcifront is already configured (if not, we'll wait) */ xen_pcibk_attach(pdev); - return err; } -- cgit v1.2.3-70-g09d2