lkml.org 
[lkml]   [2011]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] xen/pciback: Use mutexes when working with Xenbus state transitions.
    >>> On 16.09.11 at 21:06, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

    > 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

    This is only the direct conversion of existing spinlock accesses in
    xenbus.c. However, in the course of converting from the legacy
    implementation you stripped a couple more (in xen_pcibk_attach(),
    xen_pcibk_reconfigure(), and xen_pcibk_setup_backend()), and
    those should now get re-added imo.

    Jan

    > 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>
    > ---
    > drivers/xen/xen-pciback/pciback.h | 2 +-
    > drivers/xen/xen-pciback/xenbus.c | 16 +++++-----------
    > 2 files changed, 6 insertions(+), 12 deletions(-)
    >
    > diff --git a/drivers/xen/xen-pciback/pciback.h
    > b/drivers/xen/xen-pciback/pciback.h
    > index a0e131a..c3af628 100644
    > --- a/drivers/xen/xen-pciback/pciback.h
    > +++ b/drivers/xen/xen-pciback/pciback.h
    > @@ -27,7 +27,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 978d2c6..c057d67 100644
    > --- a/drivers/xen/xen-pciback/xenbus.c
    > +++ b/drivers/xen/xen-pciback/xenbus.c
    > @@ -44,7 +44,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;
    > @@ -62,14 +62,13 @@ 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 */
    > @@ -77,13 +76,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)
    > @@ -120,9 +117,8 @@ static int xen_pcibk_do_attach(struct xen_pcibk_device
    > *pdev, int gnt_ref,
    > goto out;
    > }
    >
    > - spin_lock(&pdev->dev_lock);
    > + mutex_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,
    > @@ -132,14 +128,12 @@ 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");
    > out:
    > + mutex_unlock(&pdev->dev_lock);
    > return err;
    > }
    >





    \
     
     \ /
      Last update: 2011-09-19 12:45    [W:0.051 / U:0.248 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site