]>
Commit | Line | Data |
---|---|---|
a15a8890 SL |
1 | From 0fb28e46a86e6d5ece83ad652bf05e20a719d6c4 Mon Sep 17 00:00:00 2001 |
2 | From: Ross Lagerwall <ross.lagerwall@citrix.com> | |
3 | Date: Mon, 13 May 2019 14:56:35 +0100 | |
4 | Subject: xenbus: Avoid deadlock during suspend due to open transactions | |
5 | ||
6 | [ Upstream commit d10e0cc113c9e1b64b5c6e3db37b5c839794f3df ] | |
7 | ||
8 | During a suspend/resume, the xenwatch thread waits for all outstanding | |
9 | xenstore requests and transactions to complete. This does not work | |
10 | correctly for transactions started by userspace because it waits for | |
11 | them to complete after freezing userspace threads which means the | |
12 | transactions have no way of completing, resulting in a deadlock. This is | |
13 | trivial to reproduce by running this script and then suspending the VM: | |
14 | ||
15 | import pyxs, time | |
16 | c = pyxs.client.Client(xen_bus_path="/dev/xen/xenbus") | |
17 | c.connect() | |
18 | c.transaction() | |
19 | time.sleep(3600) | |
20 | ||
21 | Even if this deadlock were resolved, misbehaving userspace should not | |
22 | prevent a VM from being migrated. So, instead of waiting for these | |
23 | transactions to complete before suspending, store the current generation | |
24 | id for each transaction when it is started. The global generation id is | |
25 | incremented during resume. If the caller commits the transaction and the | |
26 | generation id does not match the current generation id, return EAGAIN so | |
27 | that they try again. If the transaction was instead discarded, return OK | |
28 | since no changes were made anyway. | |
29 | ||
30 | This only affects users of the xenbus file interface. In-kernel users of | |
31 | xenbus are assumed to be well-behaved and complete all transactions | |
32 | before freezing. | |
33 | ||
34 | Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> | |
35 | Reviewed-by: Juergen Gross <jgross@suse.com> | |
36 | Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> | |
37 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
38 | --- | |
39 | drivers/xen/xenbus/xenbus.h | 3 +++ | |
40 | drivers/xen/xenbus/xenbus_dev_frontend.c | 18 ++++++++++++++++++ | |
41 | drivers/xen/xenbus/xenbus_xs.c | 7 +++++-- | |
42 | 3 files changed, 26 insertions(+), 2 deletions(-) | |
43 | ||
44 | diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h | |
45 | index 092981171df1..d75a2385b37c 100644 | |
46 | --- a/drivers/xen/xenbus/xenbus.h | |
47 | +++ b/drivers/xen/xenbus/xenbus.h | |
48 | @@ -83,6 +83,7 @@ struct xb_req_data { | |
49 | int num_vecs; | |
50 | int err; | |
51 | enum xb_req_state state; | |
52 | + bool user_req; | |
53 | void (*cb)(struct xb_req_data *); | |
54 | void *par; | |
55 | }; | |
56 | @@ -133,4 +134,6 @@ void xenbus_ring_ops_init(void); | |
57 | int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par); | |
58 | void xenbus_dev_queue_reply(struct xb_req_data *req); | |
59 | ||
60 | +extern unsigned int xb_dev_generation_id; | |
61 | + | |
62 | #endif | |
63 | diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c | |
64 | index 0782ff3c2273..39c63152a358 100644 | |
65 | --- a/drivers/xen/xenbus/xenbus_dev_frontend.c | |
66 | +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c | |
67 | @@ -62,6 +62,8 @@ | |
68 | ||
69 | #include "xenbus.h" | |
70 | ||
71 | +unsigned int xb_dev_generation_id; | |
72 | + | |
73 | /* | |
74 | * An element of a list of outstanding transactions, for which we're | |
75 | * still waiting a reply. | |
76 | @@ -69,6 +71,7 @@ | |
77 | struct xenbus_transaction_holder { | |
78 | struct list_head list; | |
79 | struct xenbus_transaction handle; | |
80 | + unsigned int generation_id; | |
81 | }; | |
82 | ||
83 | /* | |
84 | @@ -441,6 +444,7 @@ static int xenbus_write_transaction(unsigned msg_type, | |
85 | rc = -ENOMEM; | |
86 | goto out; | |
87 | } | |
88 | + trans->generation_id = xb_dev_generation_id; | |
89 | list_add(&trans->list, &u->transactions); | |
90 | } else if (msg->hdr.tx_id != 0 && | |
91 | !xenbus_get_transaction(u, msg->hdr.tx_id)) | |
92 | @@ -449,6 +453,20 @@ static int xenbus_write_transaction(unsigned msg_type, | |
93 | !(msg->hdr.len == 2 && | |
94 | (!strcmp(msg->body, "T") || !strcmp(msg->body, "F")))) | |
95 | return xenbus_command_reply(u, XS_ERROR, "EINVAL"); | |
96 | + else if (msg_type == XS_TRANSACTION_END) { | |
97 | + trans = xenbus_get_transaction(u, msg->hdr.tx_id); | |
98 | + if (trans && trans->generation_id != xb_dev_generation_id) { | |
99 | + list_del(&trans->list); | |
100 | + kfree(trans); | |
101 | + if (!strcmp(msg->body, "T")) | |
102 | + return xenbus_command_reply(u, XS_ERROR, | |
103 | + "EAGAIN"); | |
104 | + else | |
105 | + return xenbus_command_reply(u, | |
106 | + XS_TRANSACTION_END, | |
107 | + "OK"); | |
108 | + } | |
109 | + } | |
110 | ||
111 | rc = xenbus_dev_request_and_reply(&msg->hdr, u); | |
112 | if (rc && trans) { | |
113 | diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c | |
114 | index 49a3874ae6bb..ddc18da61834 100644 | |
115 | --- a/drivers/xen/xenbus/xenbus_xs.c | |
116 | +++ b/drivers/xen/xenbus/xenbus_xs.c | |
117 | @@ -105,6 +105,7 @@ static void xs_suspend_enter(void) | |
118 | ||
119 | static void xs_suspend_exit(void) | |
120 | { | |
121 | + xb_dev_generation_id++; | |
122 | spin_lock(&xs_state_lock); | |
123 | xs_suspend_active--; | |
124 | spin_unlock(&xs_state_lock); | |
125 | @@ -125,7 +126,7 @@ static uint32_t xs_request_enter(struct xb_req_data *req) | |
126 | spin_lock(&xs_state_lock); | |
127 | } | |
128 | ||
129 | - if (req->type == XS_TRANSACTION_START) | |
130 | + if (req->type == XS_TRANSACTION_START && !req->user_req) | |
131 | xs_state_users++; | |
132 | xs_state_users++; | |
133 | rq_id = xs_request_id++; | |
134 | @@ -140,7 +141,7 @@ void xs_request_exit(struct xb_req_data *req) | |
135 | spin_lock(&xs_state_lock); | |
136 | xs_state_users--; | |
137 | if ((req->type == XS_TRANSACTION_START && req->msg.type == XS_ERROR) || | |
138 | - (req->type == XS_TRANSACTION_END && | |
139 | + (req->type == XS_TRANSACTION_END && !req->user_req && | |
140 | !WARN_ON_ONCE(req->msg.type == XS_ERROR && | |
141 | !strcmp(req->body, "ENOENT")))) | |
142 | xs_state_users--; | |
143 | @@ -286,6 +287,7 @@ int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par) | |
144 | req->num_vecs = 1; | |
145 | req->cb = xenbus_dev_queue_reply; | |
146 | req->par = par; | |
147 | + req->user_req = true; | |
148 | ||
149 | xs_send(req, msg); | |
150 | ||
151 | @@ -313,6 +315,7 @@ static void *xs_talkv(struct xenbus_transaction t, | |
152 | req->vec = iovec; | |
153 | req->num_vecs = num_vecs; | |
154 | req->cb = xs_wake_up; | |
155 | + req->user_req = false; | |
156 | ||
157 | msg.req_id = 0; | |
158 | msg.tx_id = t.id; | |
159 | -- | |
160 | 2.20.1 | |
161 |