[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] Remove the transaction parameter from xenbus_switch_state and move the state



# HG changeset patch
# User emellor@xxxxxxxxxxxxxxxxxxxxxx
# Node ID 24aa3bd826ff277b0960186cb8ae4ab55c42502a
# Parent  94971fe9c62a82886ff2dec11b17418b5f25e73f
Remove the transaction parameter from xenbus_switch_state and move the state
switch out of a transaction, in the few cases where it is inside one.

In order to behave properly, it is necessary for a driver to know its own
xenbus state (see changeset 9469:b3cb19d2b07f, for example).  This
value is stored as xenbus_device.state and updated by xenbus_switch_state.

If xenbus_switch_state occurs within a transaction, then there is a possibility
that the transaction would be aborted, leaving the state field dangerously out
of sync with the value currently in the store.

This fixes recent problems seen whereby bringing multiple devices up at the
same time results in some devices not coming up (often all of the even-numbered
ones, because of the pattern of transaction conflict).

Signed-off-by: Ewan Mellor <ewan@xxxxxxxxxxxxx>

diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c Thu Mar 30 23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c Thu Mar 30 23:24:54 2006
@@ -142,7 +142,7 @@
        if (err)
                goto fail;
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err)
                goto fail;
 
@@ -270,7 +270,7 @@
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -343,15 +343,17 @@
                goto abort;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateConnected);
-       if (err)
-               goto abort;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err == -EAGAIN)
                goto again;
        if (err)
                xenbus_dev_fatal(dev, err, "ending transaction");
+
+       err = xenbus_switch_state(dev, XenbusStateConnected);
+       if (err)
+               xenbus_dev_fatal(dev, err, "switching to Connected state",
+                                dev->nodename);
+
        return;
  abort:
        xenbus_transaction_end(xbt, 1);
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c      Thu Mar 30 
23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c      Thu Mar 30 
23:24:54 2006
@@ -176,10 +176,6 @@
                goto abort_transaction;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateInitialised);
-       if (err)
-               goto abort_transaction;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err) {
                if (err == -EAGAIN)
@@ -187,6 +183,8 @@
                xenbus_dev_fatal(dev, err, "completing transaction");
                goto destroy_blkring;
        }
+
+       xenbus_switch_state(dev, XenbusStateInitialised);
 
        return 0;
 
@@ -324,7 +322,7 @@
                return;
        }
 
-       (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected);
+       (void)xenbus_switch_state(info->xbdev, XenbusStateConnected);
 
        /* Kick pending requests. */
        spin_lock_irq(&blkif_io_lock);
@@ -349,7 +347,7 @@
 
        xlvbd_del(info);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
 }
 
 
@@ -755,7 +753,7 @@
 
        kfree(copy);
 
-       (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected);
+       (void)xenbus_switch_state(info->xbdev, XenbusStateConnected);
 
        /* Now safe for us to use the shared ring */
        spin_lock_irq(&blkif_io_lock);
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c Thu Mar 30 23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c Thu Mar 30 23:24:54 2006
@@ -92,7 +92,7 @@
        if (err)
                goto fail;
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err) {
                goto fail;
        }
@@ -209,7 +209,7 @@
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -254,7 +254,7 @@
                return;
        }
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateConnected);
+       xenbus_switch_state(dev, XenbusStateConnected);
 }
 
 
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Thu Mar 30 
23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Thu Mar 30 
23:24:54 2006
@@ -1216,7 +1216,7 @@
 
        close_netdev(info);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
 }
 
 
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c Thu Mar 30 23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c Thu Mar 30 23:24:54 2006
@@ -137,7 +137,7 @@
 
        dev_dbg(&pdev->xdev->dev, "Connecting...\n");
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
        if (err)
                xenbus_dev_fatal(pdev->xdev, err,
                                 "Error switching to connected state!");
@@ -165,7 +165,7 @@
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(xdev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(xdev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -341,7 +341,7 @@
                goto out;
        }
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateInitialised);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
        if (err)
                xenbus_dev_fatal(pdev->xdev, err,
                                 "Error switching to initialised state!");
@@ -386,7 +386,7 @@
        }
 
        /* wait for xend to configure us */
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err)
                goto out;
 
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c        Thu Mar 30 
23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c        Thu Mar 30 
23:24:54 2006
@@ -96,10 +96,6 @@
        if (!err)
                err = xenbus_printf(trans, pdev->xdev->nodename,
                                    "magic", XEN_PCI_MAGIC);
-       if (!err)
-               err =
-                   xenbus_switch_state(pdev->xdev, trans,
-                                       XenbusStateInitialised);
 
        if (err) {
                xenbus_transaction_end(trans, 1);
@@ -117,6 +113,8 @@
                        goto out;
                }
        }
+
+       xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
 
        dev_dbg(&pdev->xdev->dev, "publishing successful!\n");
 
@@ -186,7 +184,7 @@
                }
        }
 
-       err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected);
+       err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
        if (err)
                goto out;
 
@@ -205,8 +203,7 @@
        prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
 
        if (prev_state < XenbusStateClosing)
-               err = xenbus_switch_state(pdev->xdev, XBT_NULL,
-                                       XenbusStateClosing);
+               err = xenbus_switch_state(pdev->xdev, XenbusStateClosing);
 
        if (!err && prev_state == XenbusStateConnected)
                pcifront_disconnect(pdev);
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c Thu Mar 30 23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c Thu Mar 30 23:24:54 2006
@@ -87,7 +87,7 @@
                goto fail;
        }
 
-       err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait);
+       err = xenbus_switch_state(dev, XenbusStateInitWait);
        if (err) {
                goto fail;
        }
@@ -175,7 +175,7 @@
                break;
 
        case XenbusStateClosing:
-               xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+               xenbus_switch_state(dev, XenbusStateClosing);
                break;
 
        case XenbusStateClosed:
@@ -247,18 +247,15 @@
                goto abort;
        }
 
-       err = xenbus_switch_state(dev, xbt, XenbusStateConnected);
-       if (err)
-               goto abort;
-
-       be->tpmif->status = CONNECTED;
-
        err = xenbus_transaction_end(xbt, 0);
        if (err == -EAGAIN)
                goto again;
-       if (err) {
+       if (err)
                xenbus_dev_fatal(be->dev, err, "end of transaction");
-       }
+
+       err = xenbus_switch_state(dev, XenbusStateConnected);
+       if (!err)
+               be->tpmif->status = CONNECTED;
        return;
 abort:
        xenbus_transaction_end(xbt, 1);
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c   Thu Mar 30 
23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c   Thu Mar 30 
23:24:54 2006
@@ -84,9 +84,7 @@
 EXPORT_SYMBOL_GPL(xenbus_watch_path2);
 
 
-int xenbus_switch_state(struct xenbus_device *dev,
-                       xenbus_transaction_t xbt,
-                       XenbusState state)
+int xenbus_switch_state(struct xenbus_device *dev, XenbusState state)
 {
        /* We check whether the state is currently set to the given value, and
           if not, then the state is set.  We don't want to unconditionally
@@ -94,6 +92,12 @@
           unnecessarily.  Furthermore, if the node has gone, we don't write
           to it, as the device will be tearing down, and we don't want to
           resurrect that directory.
+
+          Note that, because of this cached value of our state, this function
+          will not work inside a Xenstore transaction (something it was
+          trying to in the past) because dev->state would not get reset if
+          the transaction was aborted.
+
         */
 
        int current_state;
@@ -102,12 +106,12 @@
        if (state == dev->state)
                return 0;
 
-       err = xenbus_scanf(xbt, dev->nodename, "state", "%d",
-                              &current_state);
+       err = xenbus_scanf(XBT_NULL, dev->nodename, "state", "%d",
+                          &current_state);
        if (err != 1)
                return 0;
 
-       err = xenbus_printf(xbt, dev->nodename, "state", "%d", state);
+       err = xenbus_printf(XBT_NULL, dev->nodename, "state", "%d", state);
        if (err) {
                if (state != XenbusStateClosing) /* Avoid looping */
                        xenbus_dev_fatal(dev, err, "writing new state");
@@ -193,7 +197,7 @@
        _dev_error(dev, err, fmt, ap);
        va_end(ap);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing);
+       xenbus_switch_state(dev, XenbusStateClosing);
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_fatal);
 
diff -r 94971fe9c62a -r 24aa3bd826ff 
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c    Thu Mar 30 
23:15:12 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c    Thu Mar 30 
23:24:54 2006
@@ -364,7 +364,7 @@
        return 0;
 fail:
        xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename);
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
        return -ENODEV;
 }
 
@@ -381,7 +381,7 @@
        if (drv->remove)
                drv->remove(dev);
 
-       xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed);
+       xenbus_switch_state(dev, XenbusStateClosed);
        return 0;
 }
 
diff -r 94971fe9c62a -r 24aa3bd826ff linux-2.6-xen-sparse/include/xen/xenbus.h
--- a/linux-2.6-xen-sparse/include/xen/xenbus.h Thu Mar 30 23:15:12 2006
+++ b/linux-2.6-xen-sparse/include/xen/xenbus.h Thu Mar 30 23:24:54 2006
@@ -204,14 +204,10 @@
 
 /**
  * Advertise in the store a change of the given driver to the given new_state.
- * Perform the change inside the given transaction xbt.  xbt may be NULL, in
- * which case this is performed inside its own transaction.  Return 0 on
- * success, or -errno on error.  On error, the device will switch to
- * XenbusStateClosing, and the error will be saved in the store.
- */
-int xenbus_switch_state(struct xenbus_device *dev,
-                       xenbus_transaction_t xbt,
-                       XenbusState new_state);
+ * Return 0 on success, or -errno on error.  On error, the device will switch
+ * to XenbusStateClosing, and the error will be saved in the store.
+ */
+int xenbus_switch_state(struct xenbus_device *dev, XenbusState new_state);
 
 
 /**

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.