[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", - ¤t_state); + err = xenbus_scanf(XBT_NULL, dev->nodename, "state", "%d", + ¤t_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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |