Commit d10e0cc1 authored by Ross Lagerwall's avatar Ross Lagerwall Committed by Boris Ostrovsky

xenbus: Avoid deadlock during suspend due to open transactions

During a suspend/resume, the xenwatch thread waits for all outstanding
xenstore requests and transactions to complete. This does not work
correctly for transactions started by userspace because it waits for
them to complete after freezing userspace threads which means the
transactions have no way of completing, resulting in a deadlock. This is
trivial to reproduce by running this script and then suspending the VM:

    import pyxs, time
    c = pyxs.client.Client(xen_bus_path="/dev/xen/xenbus")
    c.connect()
    c.transaction()
    time.sleep(3600)

Even if this deadlock were resolved, misbehaving userspace should not
prevent a VM from being migrated. So, instead of waiting for these
transactions to complete before suspending, store the current generation
id for each transaction when it is started. The global generation id is
incremented during resume. If the caller commits the transaction and the
generation id does not match the current generation id, return EAGAIN so
that they try again. If the transaction was instead discarded, return OK
since no changes were made anyway.

This only affects users of the xenbus file interface. In-kernel users of
xenbus are assumed to be well-behaved and complete all transactions
before freezing.
Signed-off-by: default avatarRoss Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
Signed-off-by: default avatarBoris Ostrovsky <boris.ostrovsky@oracle.com>
parent 41349672
...@@ -83,6 +83,7 @@ struct xb_req_data { ...@@ -83,6 +83,7 @@ struct xb_req_data {
int num_vecs; int num_vecs;
int err; int err;
enum xb_req_state state; enum xb_req_state state;
bool user_req;
void (*cb)(struct xb_req_data *); void (*cb)(struct xb_req_data *);
void *par; void *par;
}; };
...@@ -133,4 +134,6 @@ void xenbus_ring_ops_init(void); ...@@ -133,4 +134,6 @@ void xenbus_ring_ops_init(void);
int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par); int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par);
void xenbus_dev_queue_reply(struct xb_req_data *req); void xenbus_dev_queue_reply(struct xb_req_data *req);
extern unsigned int xb_dev_generation_id;
#endif #endif
...@@ -62,6 +62,8 @@ ...@@ -62,6 +62,8 @@
#include "xenbus.h" #include "xenbus.h"
unsigned int xb_dev_generation_id;
/* /*
* An element of a list of outstanding transactions, for which we're * An element of a list of outstanding transactions, for which we're
* still waiting a reply. * still waiting a reply.
...@@ -69,6 +71,7 @@ ...@@ -69,6 +71,7 @@
struct xenbus_transaction_holder { struct xenbus_transaction_holder {
struct list_head list; struct list_head list;
struct xenbus_transaction handle; struct xenbus_transaction handle;
unsigned int generation_id;
}; };
/* /*
...@@ -441,6 +444,7 @@ static int xenbus_write_transaction(unsigned msg_type, ...@@ -441,6 +444,7 @@ static int xenbus_write_transaction(unsigned msg_type,
rc = -ENOMEM; rc = -ENOMEM;
goto out; goto out;
} }
trans->generation_id = xb_dev_generation_id;
list_add(&trans->list, &u->transactions); list_add(&trans->list, &u->transactions);
} else if (msg->hdr.tx_id != 0 && } else if (msg->hdr.tx_id != 0 &&
!xenbus_get_transaction(u, msg->hdr.tx_id)) !xenbus_get_transaction(u, msg->hdr.tx_id))
...@@ -449,6 +453,20 @@ static int xenbus_write_transaction(unsigned msg_type, ...@@ -449,6 +453,20 @@ static int xenbus_write_transaction(unsigned msg_type,
!(msg->hdr.len == 2 && !(msg->hdr.len == 2 &&
(!strcmp(msg->body, "T") || !strcmp(msg->body, "F")))) (!strcmp(msg->body, "T") || !strcmp(msg->body, "F"))))
return xenbus_command_reply(u, XS_ERROR, "EINVAL"); return xenbus_command_reply(u, XS_ERROR, "EINVAL");
else if (msg_type == XS_TRANSACTION_END) {
trans = xenbus_get_transaction(u, msg->hdr.tx_id);
if (trans && trans->generation_id != xb_dev_generation_id) {
list_del(&trans->list);
kfree(trans);
if (!strcmp(msg->body, "T"))
return xenbus_command_reply(u, XS_ERROR,
"EAGAIN");
else
return xenbus_command_reply(u,
XS_TRANSACTION_END,
"OK");
}
}
rc = xenbus_dev_request_and_reply(&msg->hdr, u); rc = xenbus_dev_request_and_reply(&msg->hdr, u);
if (rc && trans) { if (rc && trans) {
......
...@@ -105,6 +105,7 @@ static void xs_suspend_enter(void) ...@@ -105,6 +105,7 @@ static void xs_suspend_enter(void)
static void xs_suspend_exit(void) static void xs_suspend_exit(void)
{ {
xb_dev_generation_id++;
spin_lock(&xs_state_lock); spin_lock(&xs_state_lock);
xs_suspend_active--; xs_suspend_active--;
spin_unlock(&xs_state_lock); spin_unlock(&xs_state_lock);
...@@ -125,7 +126,7 @@ static uint32_t xs_request_enter(struct xb_req_data *req) ...@@ -125,7 +126,7 @@ static uint32_t xs_request_enter(struct xb_req_data *req)
spin_lock(&xs_state_lock); spin_lock(&xs_state_lock);
} }
if (req->type == XS_TRANSACTION_START) if (req->type == XS_TRANSACTION_START && !req->user_req)
xs_state_users++; xs_state_users++;
xs_state_users++; xs_state_users++;
rq_id = xs_request_id++; rq_id = xs_request_id++;
...@@ -140,7 +141,7 @@ void xs_request_exit(struct xb_req_data *req) ...@@ -140,7 +141,7 @@ void xs_request_exit(struct xb_req_data *req)
spin_lock(&xs_state_lock); spin_lock(&xs_state_lock);
xs_state_users--; xs_state_users--;
if ((req->type == XS_TRANSACTION_START && req->msg.type == XS_ERROR) || if ((req->type == XS_TRANSACTION_START && req->msg.type == XS_ERROR) ||
(req->type == XS_TRANSACTION_END && (req->type == XS_TRANSACTION_END && !req->user_req &&
!WARN_ON_ONCE(req->msg.type == XS_ERROR && !WARN_ON_ONCE(req->msg.type == XS_ERROR &&
!strcmp(req->body, "ENOENT")))) !strcmp(req->body, "ENOENT"))))
xs_state_users--; xs_state_users--;
...@@ -286,6 +287,7 @@ int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par) ...@@ -286,6 +287,7 @@ int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par)
req->num_vecs = 1; req->num_vecs = 1;
req->cb = xenbus_dev_queue_reply; req->cb = xenbus_dev_queue_reply;
req->par = par; req->par = par;
req->user_req = true;
xs_send(req, msg); xs_send(req, msg);
...@@ -313,6 +315,7 @@ static void *xs_talkv(struct xenbus_transaction t, ...@@ -313,6 +315,7 @@ static void *xs_talkv(struct xenbus_transaction t,
req->vec = iovec; req->vec = iovec;
req->num_vecs = num_vecs; req->num_vecs = num_vecs;
req->cb = xs_wake_up; req->cb = xs_wake_up;
req->user_req = false;
msg.req_id = 0; msg.req_id = 0;
msg.tx_id = t.id; msg.tx_id = t.id;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment