[ltt-dev] [PATCH] lttng-remove-unneeded-marker-mutex-which-cause-circ-dep

[Mathieu Desnoyers]

Current lttng was having circular locking deps in ltt-trace-control. I
removed some locking around mkdir/rmdir/hotplug notfier handling.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at polymtl.ca>
Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
---
 ltt/ltt-trace-control.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Index: linux-2.6-lttng/ltt/ltt-trace-control.c
===================================================================
--- linux-2.6-lttng.orig/ltt/ltt-trace-control.c	2009-04-07 16:41:15.000000000 -0400
+++ linux-2.6-lttng/ltt/ltt-trace-control.c	2009-04-07 17:27:53.000000000 -0400
@@ -34,10 +34,23 @@ struct dentry *ltt_control_dir, *ltt_set
 
 /*
  * the traces_lock nests inside control_lock.
+ * control_lock protects the consistency of directories presented in ltt
+ * directory.
  */
 static DEFINE_MUTEX(control_lock);
 
 /*
+ * big note about locking for marker control files :
+ * If a marker control file is added/removed manually racing with module
+ * load/unload, there may be warning messages appearing, but those two
+ * operations should be able to execute concurrently without any lock
+ * synchronizing their operation one wrt another.
+ * Locking the marker mutex, module mutex and also keeping a mutex here
+ * from mkdir/rmdir _and_ from the notifier called from module load/unload makes
+ * life miserable and just asks for deadlocks.
+ */
+
+/*
  * lookup a file/dir in parent dir.
  * only designed to work well for debugfs.
  * (although it maybe ok for other fs)
@@ -869,8 +882,10 @@ static ssize_t marker_info_read(struct f
 	    !_is_marker_present(channel, marker)) {
 		len += snprintf(buf + len, PAGE_SIZE - len,
 				"Marker Pre-enabled\n");
+		unlock_markers();
 		goto out;
 	}
+	unlock_markers();
 
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
@@ -899,7 +914,6 @@ static ssize_t marker_info_read(struct f
 	marker_iter_stop(&iter);
 
 out:
-	unlock_markers();
 	if (len >= PAGE_SIZE) {
 		len = PAGE_SIZE;
 		buf[PAGE_SIZE] = '\0';
@@ -1136,8 +1150,6 @@ static int build_marker_control_files(vo
 	if (!markers_control_dir)
 		return -EEXIST;
 
-	lock_markers();
-
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -1148,7 +1160,6 @@ static int build_marker_control_files(vo
 	marker_iter_stop(&iter);
 
 out:
-	unlock_markers();
 	return err;
 }
 
@@ -1171,8 +1182,6 @@ static int remove_marker_control_dir(str
 		return -ENOENT;
 	name = marker_d->d_name.name;
 
-	lock_markers();
-
 	marker_iter_reset(&iter);
 	marker_iter_start(&iter);
 	for (; iter.marker != NULL; marker_iter_next(&iter)) {
@@ -1190,7 +1199,6 @@ static int remove_marker_control_dir(str
 
 end:
 	marker_iter_stop(&iter);
-	unlock_markers();
 	return 0;
 }
 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68