Skip to contentSkip to conversation

Linux Kernel Markers Fixes

PATCH New Mathieu Desnoyers 1 participant 4 messages Nov 13, 2007 Documentation

lore
Share & tools
lore

Mathieu Desnoyers

author
#0 lore raw

cover Linux Kernel Markers Fixes

Hi Andrew,

Here are some fixes for the Linux Kernel Markers present in 2.6.24-rc2-git3. There is a code path (module load) that may update markers without holding the markers mutex, si I fixed that. The rest is just an tiny documentation improvement.

The patchset applies on top of 2.6.24-rc2-git3 in this order:

#for -mm
markers-fix-markers-mutex-upon-module-load.patch
markers-document-markers-format-strings.patch
markers-fix-example-format-string.patch

They should be merged in mainline before 2.6.24 final (especially the mutex fix).

Mathieu

Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
author
#1 lore raw

PATCH 1/3 Linux Kernel Markers - Fix marker mutex not taken upon module load

Upon module load, we must take the markers mutex. It implies that the marker mutex must be nested inside the module mutex.

It implies changing the nesting order : now the marker mutex nests inside the module mutex. Make the necessary changes to reverse the order in which the mutexes are taken.

Includes some cleanup from Dave Hansen <[email protected]>.

Signed-off-by Mathieu Desnoyers <[email protected]>

---

 kernel/marker.c |   41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)
kernel/marker.c +17 -24
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
@@ -28,7 +28,7 @@ extern struct marker __start___markers[]
 extern struct marker __stop___markers[];
 
 /*
- * module_mutex nests inside markers_mutex. Markers mutex protects the builtin
+ * markers_mutex nests inside module_mutex. Markers mutex protects the builtin
  * and module markers, the hash table and deferred_sync.
  */
 static DEFINE_MUTEX(markers_mutex);

@@ -257,7 +257,6 @@ static void disable_marker(struct marker
  * @refcount: number of references left to the given probe_module (out)
  *
  * Updates the probe callback corresponding to a range of markers.
- * Must be called with markers_mutex held.
  */
 void marker_update_probe_range(struct marker *begin,
 	struct marker *end, struct module *probe_module,

@@ -266,6 +265,7 @@ void marker_update_probe_range(struct ma
 	struct marker *iter;
 	struct marker_entry *mark_entry;
 
+	mutex_lock(&markers_mutex);
 	for (iter = begin; iter < end; iter++) {
 		mark_entry = get_marker(iter->name);
 		if (mark_entry && mark_entry->refcount) {

@@ -281,6 +281,7 @@ void marker_update_probe_range(struct ma
 			disable_marker(iter);
 		}
 	}
+	mutex_unlock(&markers_mutex);
 }
 
 /*

@@ -293,7 +294,6 @@ static void marker_update_probes(struct 
 {
 	int refcount = 0;
 
-	mutex_lock(&markers_mutex);
 	/* Core kernel markers */
 	marker_update_probe_range(__start___markers,
 			__stop___markers, probe_module, &refcount);

@@ -303,7 +303,6 @@ static void marker_update_probes(struct 
 		synchronize_sched();
 		deferred_sync = 0;
 	}
-	mutex_unlock(&markers_mutex);
 }
 
 /**

@@ -320,7 +319,7 @@ int marker_probe_register(const char *na
 			marker_probe_func *probe, void *private)
 {
 	struct marker_entry *entry;
-	int ret = 0, need_update = 0;
+	int ret = 0;
 
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);

@@ -335,11 +334,11 @@ int marker_probe_register(const char *na
 	ret = add_marker(name, format, probe, private);
 	if (ret)
 		goto end;
-	need_update = 1;
+	mutex_unlock(&markers_mutex);
+	marker_update_probes(NULL);
+	return ret;
 end:
 	mutex_unlock(&markers_mutex);
-	if (need_update)
-		marker_update_probes(NULL);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(marker_probe_register);

@@ -355,7 +354,6 @@ void *marker_probe_unregister(const char
 	struct module *probe_module;
 	struct marker_entry *entry;
 	void *private;
-	int need_update = 0;
 
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);

@@ -368,11 +366,11 @@ void *marker_probe_unregister(const char
 	probe_module = __module_text_address((unsigned long)entry->probe);
 	private = remove_marker(name);
 	deferred_sync = 1;
-	need_update = 1;
+	mutex_unlock(&markers_mutex);
+	marker_update_probes(probe_module);
+	return private;
 end:
 	mutex_unlock(&markers_mutex);
-	if (need_update)
-		marker_update_probes(probe_module);
 	return private;
 }
 EXPORT_SYMBOL_GPL(marker_probe_unregister);

@@ -392,7 +390,6 @@ void *marker_probe_unregister_private_da
 	struct marker_entry *entry;
 	int found = 0;
 	unsigned int i;
-	int need_update = 0;
 
 	mutex_lock(&markers_mutex);
 	for (i = 0; i < MARKER_TABLE_SIZE; i++) {

@@ -414,11 +411,11 @@ iter_end:
 	probe_module = __module_text_address((unsigned long)entry->probe);
 	private = remove_marker(entry->name);
 	deferred_sync = 1;
-	need_update = 1;
+	mutex_unlock(&markers_mutex);
+	marker_update_probes(probe_module);
+	return private;
 end:
 	mutex_unlock(&markers_mutex);
-	if (need_update)
-		marker_update_probes(probe_module);
 	return private;
 }
 EXPORT_SYMBOL_GPL(marker_probe_unregister_private_data);

@@ -434,7 +431,7 @@ EXPORT_SYMBOL_GPL(marker_probe_unregiste
 int marker_arm(const char *name)
 {
 	struct marker_entry *entry;
-	int ret = 0, need_update = 0;
+	int ret = 0;
 
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);

@@ -447,11 +444,9 @@ int marker_arm(const char *name)
 	 */
 	if (entry->refcount++)
 		goto end;
-	need_update = 1;
 end:
 	mutex_unlock(&markers_mutex);
-	if (need_update)
-		marker_update_probes(NULL);
+	marker_update_probes(NULL);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(marker_arm);

@@ -467,7 +462,7 @@ EXPORT_SYMBOL_GPL(marker_arm);
 int marker_disarm(const char *name)
 {
 	struct marker_entry *entry;
-	int ret = 0, need_update = 0;
+	int ret = 0;
 
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);

@@ -486,11 +481,9 @@ int marker_disarm(const char *name)
 		ret = -EPERM;
 		goto end;
 	}
-	need_update = 1;
 end:
 	mutex_unlock(&markers_mutex);
-	if (need_update)
-		marker_update_probes(NULL);
+	marker_update_probes(NULL);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(marker_disarm);
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
author
#2 lore raw

PATCH 2/3 Linux Kernel Markers - Document format string

In reply to Mathieu Desnoyers

Describes the format string standard further: Use of field names before the type specifiers..

Signed-off-by Mathieu Desnoyers <[email protected]>
---
 Documentation/markers.txt |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Documentation/markers.txt +4 -2
Index: linux-2.6-lttng/Documentation/markers.txt
===================================================================
@@ -35,12 +35,14 @@ In order to use the macro trace_mark, yo
 
 And,
 
-trace_mark(subsystem_event, "%d %s", someint, somestring);
+trace_mark(subsystem_event, "myint %d mystring %s", someint, somestring);
 Where :
 - subsystem_event is an identifier unique to your event
     - subsystem is the name of your subsystem.
     - event is the name of the event to mark.
-- "%d %s" is the formatted string for the serializer.
+- "myint %d mystring %s" is the formatted string for the serializer. "myint" and
+  "mystring" are repectively the field names associated with the first and
+  second parameter.
 - someint is an integer.
 - somestring is a char pointer.
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
author
#3 lore raw

PATCH 3/3 Linux Kernel Markers - fix samples to follow format string standard

In reply to Mathieu Desnoyers

Add the field names to marker example format string.

Signed-off-by Mathieu Desnoyers <[email protected]>
---
 samples/markers/marker-example.c |    3 ++-
 samples/markers/probe-example.c  |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)
samples/markers/marker-example.c +2 -1
Index: linux-2.6-lttng/samples/markers/marker-example.c
===================================================================
@@ -19,7 +19,8 @@ static int my_open(struct inode *inode, 
 {
 	int i;
 
-	trace_mark(subsystem_event, "%d %s", 123, "example string");
+	trace_mark(subsystem_event, "integer %d string %s", 123,
+		"example string");
 	for (i = 0; i < 10; i++)
 		trace_mark(subsystem_eventb, MARK_NOARGS);
 	return -EPERM;
samples/markers/probe-example.c +1 -1
Index: linux-2.6-lttng/samples/markers/probe-example.c
===================================================================
@@ -53,7 +53,7 @@ void probe_subsystem_eventb(const struct
 static struct probe_data probe_array[] =
 {
 	{	.name = "subsystem_event",
-		.format = "%d %s",
+		.format = "integer %d string %s",
 		.probe_func = probe_subsystem_event },
 	{	.name = "subsystem_eventb",
 		.format = MARK_NOARGS,
Signature · 3 lines
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68