Skip to contentSkip to conversation

ftrace: prevent freeing of all failed updates

PATCH Applied Abhishek Sagar 3 participants 5 messages Jun 1, 2008 – Jun 10, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 ftrace: prevent freeing of all failed updates

Prevent freeing of records which cause problems and correspond to function from core kernel text. A new flag, FTRACE_FL_CONVERTED is used to mark a record as "converted". All other records are patched lazily to NOPs. Failed records now also remain on frace_hash table. Each invocation of ftrace_record_ip now checks whether the traced function has ever been recorded (including past failures) and doesn't re-record it again.

---

include/linux/ftrace.h +1 -0
index b482fe8..fc4979a 100644
@@ -49,6 +49,7 @@ enum {
 	FTRACE_FL_FILTER	= (1 << 2),
 	FTRACE_FL_ENABLED	= (1 << 3),
 	FTRACE_FL_NOTRACE	= (1 << 4),
+	FTRACE_FL_CONVERTED	= (1 << 5),
 };
 
 struct dyn_ftrace {
kernel/trace/ftrace.c +46 -30

152 lines not on this page · raw message

maintainer
#1 lore raw

On Sun, 1 Jun 2008, Abhishek Sagar wrote:
5B> @@ -611,18 +612,33 @@ static int __ftrace_update_code(void *ignore)

Quoted 11 lines
kernel/trace/ftrace.c +6 -2
 	/* No locks needed, the machine is stopped! */
 	for (i = 0; i < FTRACE_HASHSIZE; i++) {
-		if (hlist_empty(&ftrace_hash[i]))
-			continue;
+		/* all CPUS are stopped, we are safe to modify code */
+		hlist_for_each_entry_safe(p, t, n, &ftrace_hash[i], node) {
+			/* Skip over failed records which have not been
+			 * freed. */
+			if (p->flags & FTRACE_FL_FAILED)
+				continue;

kernel/trace/ftrace.c static int __ftrace_update_code(void *ignore) · line 628

Quoted 10 lines
kernel/trace/ftrace.c +7 -2
 
-		head = ftrace_hash[i];
-		INIT_HLIST_HEAD(&ftrace_hash[i]);
+			/* Unconverted records are always at the head of the
+			 * hash bucket. Once we encounter a converted record,
+			 * simply skip over to the next bucket. Saves ftraced
+			 * some processor cycles (ftrace does its bid for
+			 * global warming :-p ). */
+			if (p->flags & (FTRACE_FL_CONVERTED))
+				break;

If we unload a module and reload it, will it ever get converted again?

-- Steve

#2 lore raw
Quoted 1 line · Steven Rostedt

If we unload a module and reload it, will it ever get converted again?

The intent was always to filter core kernel functions to prevent their freeing. Here's a fix which should allow re-recording of module call-sites.

---

kernel/trace/ftrace.c +2 -3
index ec54cb7..77c9392 100644
@@ -437,7 +437,7 @@ static void ftrace_replace_code(int enable)
 			if (failed && (rec->flags & FTRACE_FL_CONVERTED)) {
 				rec->flags |= FTRACE_FL_FAILED;
 				if ((system_state == SYSTEM_BOOTING) ||
-				    !kernel_text_address(rec->ip)) {
+				    !core_kernel_text(rec->ip)) {
 					ftrace_del_hash(rec);
 					ftrace_free_rec(rec);
 				}
@@ -658,10 +658,9 @@ static int __ftrace_update_code(void *ignore)
 				ftrace_update_cnt++;
 			} else {
 				if ((system_state == SYSTEM_BOOTING) ||
-				    !kernel_text_address(p->ip)) {
+				    !core_kernel_text(p->ip)) {
 					ftrace_del_hash(p);
 					ftrace_free_rec(p);
-
 				}
 			}
 		}
Applied
#4 lore raw
In reply to Abhishek Sagar
Quoted 7 lines · Abhishek Sagar

Prevent freeing of records which cause problems and correspond to function from core kernel text. A new flag, FTRACE_FL_CONVERTED is used to mark a record as "converted". All other records are patched lazily to NOPs. Failed records now also remain on frace_hash table. Each invocation of ftrace_record_ip now checks whether the traced function has ever been recorded (including past failures) and doesn't re-record it again.

applied to tip/tracing/ftrace, thanks Abhishek.

Ingo

Applied
#3 lore raw
In reply to Abhishek Sagar
Quoted 6 lines · Abhishek Sagar
Quoted 1 line · Steven Rostedt

If we unload a module and reload it, will it ever get converted again?

The intent was always to filter core kernel functions to prevent their freeing. Here's a fix which should allow re-recording of module call-sites.

applied to tip/tracing/ftrace, thanks.

Ingo