talexander
/
freebsd_amp_hwpstate
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

xenbus: improve device tracking 

xenbus needs to keep track of the devices exposed on xenstore, so that
it can trigger frontend and backend device creation.

Removal of backend devices is currently detected by checking the
existence of the device (backend) xenstore directory, but that's prone
to races as the device driver would usually add entries to such
directory itself, so under certain circumstances it's possible for a
driver to add node to the directory after the toolstack has removed
it.  This leads to devices not removed, which can eventually exhaust
the memory of FreeBSD.

Fix this by checking for the existence of the 'state' node instead of
the directory, as such node will always be present when a device is
active, and will be removed by the toolstack when the device is shut
down.  In order to avoid any races with the updating of the 'state'
node by FreeBSD and the toolstack removing it use a transaction in
xenbusb_write_ivar() for that purpose.

Reported by: Ze Dupsys <zedupsys@gmail.com>
Sponsored by: Citrix Systems R&D
This commit is contained in:
Roger Pau Monné 2022-03-21 12:47:20 +01:00
parent 91d6afe6e2
commit f3d54ded28
1 changed files with 43 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
sys/xen/xenbus/xenbusb.c
View File

@ -254,7 +254,7 @@ xenbusb_delete_child(device_t dev, device_t child)
static void
xenbusb_verify_device(device_t dev, device_t child)
{
if (xs_exists(XST_NIL, xenbus_get_node(child), "") == 0) {
if (xs_exists(XST_NIL, xenbus_get_node(child), "state") == 0) {
/*
* Device tree has been removed from Xenbus.
* Tear down the device.
@ -907,6 +907,7 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value)
case XENBUS_IVAR_STATE:
{
int error;
struct xs_transaction xst;
newstate = (enum xenbus_state)value;
sx_xlock(&ivars->xd_lock);
@ -915,31 +916,39 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value)
goto out;
}
error = xs_scanf(XST_NIL, ivars->xd_node, "state",
NULL, "%d", &currstate);
if (error)
goto out;
do {
error = xs_printf(XST_NIL, ivars->xd_node, "state",
"%d", newstate);
} while (error == EAGAIN);
if (error) {
/*
* Avoid looping through xenbus_dev_fatal()
* which calls xenbus_write_ivar to set the
* state to closing.
*/
if (newstate != XenbusStateClosing)
xenbus_dev_fatal(dev, error,
"writing new state");
goto out;
}
error = xs_transaction_start(&xst);
if (error != 0)
goto out;
do {
error = xs_scanf(xst, ivars->xd_node, "state",
NULL, "%d", &currstate);
} while (error == EAGAIN);
if (error)
goto out;
do {
error = xs_printf(xst, ivars->xd_node, "state",
"%d", newstate);
} while (error == EAGAIN);
if (error) {
/*
* Avoid looping through xenbus_dev_fatal()
* which calls xenbus_write_ivar to set the
* state to closing.
*/
if (newstate != XenbusStateClosing)
xenbus_dev_fatal(dev, error,
"writing new state");
goto out;
}
} while (xs_transaction_end(xst, 0));
ivars->xd_state = newstate;
if ((ivars->xd_flags & XDF_CONNECTING) != 0
&& (newstate == XenbusStateClosed
|| newstate == XenbusStateConnected)) {
if ((ivars->xd_flags & XDF_CONNECTING) != 0 &&
(newstate == XenbusStateClosed ||
newstate == XenbusStateConnected)) {
struct xenbusb_softc *xbs;
ivars->xd_flags &= ~XDF_CONNECTING;
@ -949,8 +958,18 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value)
wakeup(&ivars->xd_state);
out:
if (error != 0)
xs_transaction_end(xst, 1);
sx_xunlock(&ivars->xd_lock);
return (error);
/*
* Shallow ENOENT errors, as returning an error here will
* trigger a panic. ENOENT is fine to ignore, because it means
* the toolstack has removed the state node as part of
* destroying the device, and so we have to shut down the
* device without recreating it or else the node would be
* leaked.
*/
return (error == ENOENT ? 0 : error);
}
case XENBUS_IVAR_NODE: