# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1257358412 0
# Node ID 9348c45356553229eac2641206660eedafea7bfe
# Parent 6301ebc85480dd54e520bd0e48c75c2b5640e2c6
xenbus: do not hold transaction_mutex when returning to userspace
================================================
[ BUG: lock held when returning to user space! ]
------------------------------------------------
xenstore-list/3522 is leaving the kernel with locks still held!
1 lock held by xenstore-list/3522:
#0: (&xs_state.transaction_mutex){......}, at: [<c026dc6f>]
xenbus_dev_request_and_reply+0x8f/0xa0
The canonical fix for this type of issue appears to be to maintain a
count manually rather than using an rwsem so do that here.
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
drivers/xen/xenbus/xenbus_xs.c | 57 +++++++++++++++++++++++++++++++++--------
1 files changed, 47 insertions(+), 10 deletions(-)
diff -r 6301ebc85480 -r 9348c4535655 drivers/xen/xenbus/xenbus_xs.c
--- a/drivers/xen/xenbus/xenbus_xs.c Fri Oct 23 10:07:22 2009 +0100
+++ b/drivers/xen/xenbus/xenbus_xs.c Wed Nov 04 18:13:32 2009 +0000
@@ -84,6 +84,14 @@ struct xs_handle {
/*
* Mutex ordering: transaction_mutex -> watch_mutex -> request_mutex.
* response_mutex is never taken simultaneously with the other three.
+ *
+ * transaction_mutex must be held before incrementing
+ * transaction_count. The mutex is held when a suspend is in
+ * progress to prevent new transactions starting.
+ *
+ * When decrementing transaction_count to zero the wait queue
+ * should be woken up, the suspend code waits for count to
+ * reach zero.
*/
/* One request at a time. */
@@ -93,7 +101,9 @@ struct xs_handle {
struct mutex response_mutex;
/* Protect transactions against save/restore. */
- struct rw_semaphore transaction_mutex;
+ struct mutex transaction_mutex;
+ atomic_t transaction_count;
+ wait_queue_head_t transaction_wq;
/* Protect watch (de)register against save/restore. */
struct rw_semaphore watch_mutex;
@@ -165,6 +175,31 @@ static void *read_reply(enum xsd_sockmsg
return body;
}
+static void transaction_start(void)
+{
+ mutex_lock(&xs_state.transaction_mutex);
+ atomic_inc(&xs_state.transaction_count);
+ mutex_unlock(&xs_state.transaction_mutex);
+}
+
+static void transaction_end(void)
+{
+ if (atomic_dec_and_test(&xs_state.transaction_count))
+ wake_up(&xs_state.transaction_wq);
+}
+
+static void transaction_suspend(void)
+{
+ mutex_lock(&xs_state.transaction_mutex);
+ wait_event(xs_state.transaction_wq,
+ atomic_read(&xs_state.transaction_count) == 0);
+}
+
+static void transaction_resume(void)
+{
+ mutex_unlock(&xs_state.transaction_mutex);
+}
+
void *xenbus_dev_request_and_reply(struct xsd_sockmsg *msg)
{
void *ret;
@@ -172,7 +207,7 @@ void *xenbus_dev_request_and_reply(struc
int err;
if (req_msg.type == XS_TRANSACTION_START)
- down_read(&xs_state.transaction_mutex);
+ transaction_start();
mutex_lock(&xs_state.request_mutex);
@@ -188,7 +223,7 @@ void *xenbus_dev_request_and_reply(struc
if ((req_msg.type == XS_TRANSACTION_END) ||
((req_msg.type == XS_TRANSACTION_START) &&
(msg->type == XS_ERROR)))
- up_read(&xs_state.transaction_mutex);
+ transaction_end();
return ret;
}
@@ -440,11 +475,11 @@ int xenbus_transaction_start(struct xenb
{
char *id_str;
- down_read(&xs_state.transaction_mutex);
+ transaction_start();
id_str = xs_single(XBT_NIL, XS_TRANSACTION_START, "", NULL);
if (IS_ERR(id_str)) {
- up_read(&xs_state.transaction_mutex);
+ transaction_end();
return PTR_ERR(id_str);
}
@@ -469,7 +504,7 @@ int xenbus_transaction_end(struct xenbus
err = xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
- up_read(&xs_state.transaction_mutex);
+ transaction_end();
return err;
}
@@ -670,7 +705,7 @@ EXPORT_SYMBOL_GPL(unregister_xenbus_watc
void xs_suspend(void)
{
- down_write(&xs_state.transaction_mutex);
+ transaction_suspend();
down_write(&xs_state.watch_mutex);
mutex_lock(&xs_state.request_mutex);
mutex_lock(&xs_state.response_mutex);
@@ -683,7 +718,7 @@ void xs_resume(void)
mutex_unlock(&xs_state.response_mutex);
mutex_unlock(&xs_state.request_mutex);
- up_write(&xs_state.transaction_mutex);
+ transaction_resume();
/* No need for watches_lock: the watch_mutex is sufficient. */
list_for_each_entry(watch, &watches, list) {
@@ -699,7 +734,7 @@ void xs_suspend_cancel(void)
mutex_unlock(&xs_state.response_mutex);
mutex_unlock(&xs_state.request_mutex);
up_write(&xs_state.watch_mutex);
- up_write(&xs_state.transaction_mutex);
+ mutex_unlock(&xs_state.transaction_mutex);
}
static int xenwatch_handle_callback(void *data)
@@ -880,8 +915,10 @@ int xs_init(void)
mutex_init(&xs_state.request_mutex);
mutex_init(&xs_state.response_mutex);
- init_rwsem(&xs_state.transaction_mutex);
+ mutex_init(&xs_state.transaction_mutex);
init_rwsem(&xs_state.watch_mutex);
+ atomic_set(&xs_state.transaction_count, 0);
+ init_waitqueue_head(&xs_state.transaction_wq);
task = kthread_run(xenwatch_thread, NULL, "xenwatch");
if (IS_ERR(task))
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home