[Xen-devel] xenbus_dev_request_and_reply()

[John Levon <levon@xxxxxxxxxxxxxxxxx>]

I just came across this little charmer:

169 void *xenbus_dev_request_and_reply(struct xsd_sockmsg *msg)
170 {
171         void *ret;
172         struct xsd_sockmsg req_msg = *msg;
173         int err;
174
175         if (req_msg.type == XS_TRANSACTION_START)
176                 down_read(&xs_state.suspend_mutex);
177
178         mutex_lock(&xs_state.request_mutex);
179
180         err = xb_write(msg, sizeof(*msg) + msg->len);
181         if (err) {
182                 msg->type = XS_ERROR;
183                 ret = ERR_PTR(err);
184         } else
185                 ret = read_reply(&msg->type, &msg->len);
186
187         mutex_unlock(&xs_state.request_mutex);
188
189         if ((req_msg.type == XS_TRANSACTION_END) ||
190             ((req_msg.type == XS_TRANSACTION_START) &&
191              (msg->type == XS_ERROR)))
192                 up_read(&xs_state.suspend_mutex);
193
194         return ret;
195 }

So if userspace gets killed during a transaction, we'll never be able to
suspend. And of course an invalid transaction end will panic the domain.

Unfortunately blktap is actually using transactions.

Given that transaction ID's are local to a connection, can't we avoid
taking the mutex altogether? Any further requests as part of the
transaction will not be found in the list and we'll return an error
reply as expected. Am I missing something?

regards
john

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