great-sage-equal-to-heaven:~/vertigo/sysprof% head -13 ChangeLog

Sat Nov 12 19:44:43 2005  Søren Sandmann  <sandmann@redhat.com>

        * module/sysprof-module.c (read_frame): New function that uses
        copy_from_user_inatomic() as check_user_pages_readable() has
        disappeared in recent kernels.

        * module/sysprof-module.c (timer_notify): Use it here.

        * TODO: Updates

        * configure.ac: Change the wording of the CVS HEAD warning as this
        change seems to also have fixed the lockup with rawhide kernels.

ChangeLog

@@ -1,3 +1,16 @@
+Sat Nov 12 19:44:43 2005  Søren Sandmann  <sandmann@redhat.com>
+
+	* module/sysprof-module.c (read_frame): New function that uses
+	copy_from_user_inatomic() as check_user_pages_readable() has
+	disappeared in recent kernels.
+	
+	* module/sysprof-module.c (timer_notify): Use it here.
+	
+	* TODO: Updates
+
+	* configure.ac: Change the wording of the CVS HEAD warning as this
+	change seems to also have fixed the lockup with rawhide kernels.
+
 Wed Nov  9 00:24:11 2005  Soeren Sandmann  <sandmann@redhat.com>
 
 	* treeviewutils.[ch]: Add new tree_view_foreach_visible()

TODO

@@ -18,6 +18,8 @@ Before 1.0.1:
 
 Before 1.2:
 
+* Add "sysprof --version"
+
 * With kernel module not installed, select Profiler->Start, then dismiss
   the alert. This causes the start button to appear prelighted. Probably
   just another gtk+ bug.
@@ -134,7 +136,8 @@ Before 1.2:
 	when the interrupt happens in kernel mode. (Unfortunately, this
 	causes lockups on many kernels).
 	
-	We don't take any stacktraces of the kernel though. Things that need to be
+	We don't take any stacktraces of the kernel though. Things that
+	need to be
 	investigated:
 		- does the kernel come with dwarf debug information?
 		- does the kernel come with some other debug info
@@ -150,6 +153,8 @@ Before 1.2:
   and what that actually makes the stack look like. (We may want to just
   special case this fake dso in the symbol lookup code).
 
+  Maybe get_user_pages() is the way forward at least for some stuff.
+
 * Correctness
 	- When the module is unloaded, kill all processes blocking in read
 		- or block unloading until all processes have exited

collector.c

@@ -126,11 +126,13 @@ on_read (gpointer data)
 	return;
     
 #if 0
-    int i;
-    g_print ("pid: %d\n", trace.pid);
-    for (i=0; i < trace.n_addresses; ++i)
-	g_print ("rd: %08x\n", trace.addresses[i]);
-    g_print ("-=-\n");
+    {
+	int i;
+	g_print ("pid: %d (%d)\n", trace.pid, trace.n_addresses);
+	for (i=0; i < trace.n_addresses; ++i)
+	    g_print ("rd: %08x\n", trace.addresses[i]);
+	g_print ("-=-\n");
+    }
 #endif
     
     if (rd > 0)

configure.ac

@@ -120,17 +120,19 @@ AC_OUTPUT
 echo
 echo "%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%@%"
 echo "@"
-echo "%      This is cvs HEAD of sysprof. "
-echo "@"
-echo "%      The kernel module in this version has bugs that"
-echo "@      cause hangs with some kernels, notably the Fedora"
-echo "%      Rawhide ones."
+echo "@      Thank you for testing cvs HEAD of sysprof."
+echo "%"
+echo "%      There are currently no known bugs in the kernel"
+echo "@      module in this version, but there could easily be"
+echo "%      unknown ones. Please report any crashes or lockups"
+echo "@      that you experience."
+echo "%"
 echo "@"
 echo "%      If you need a stable version of sysprof, either"
 echo "@      get version 1.0 from"
 echo "%"
 echo "@          http://www.daimi.au.dk/~sandmann/sysprof"
-echo "%      "
+echo "%"
 echo "@      or do"
 echo "%"
 echo "@          cvs -z3 upd -r sysprof-1-0"

module/sysprof-module.c

@@ -220,6 +220,27 @@ static int pages_present(StackFrame * head)
 }
 #endif /* CONFIG_X86_4G */
 
+static int
+read_frame (void *frame_pointer, StackFrame *frame)
+{
+#if 0
+	/* This is commented out because we seem to be called with
+	 * (current_thread_info()->addr_limit.seg)) == 0
+	 * which means access_ok() _always_ fails.
+	 *
+	 * Not sure why (or even if) this isn't the case for oprofile
+	 */
+	if (!access_ok(VERIFY_READ, frame_pointer, sizeof(StackFrame)))
+		return 1;
+#endif
+
+	if (__copy_from_user_inatomic (
+		    frame, frame_pointer, sizeof (StackFrame)))
+		return 2;
+
+	return 0;
+}
+
 static int timer_notify (struct pt_regs *regs)
 {
 #ifdef CONFIG_HIGHMEM
@@ -227,7 +248,7 @@ static int timer_notify (struct pt_regs *regs)
 #else
 #  define START_OF_STACK 0xBFFFFFFF
 #endif
-	StackFrame *frame;
+	void *frame_pointer;
 	static int n_samples;
 	SysprofStackTrace *trace = head;
 	int i;
@@ -256,17 +277,39 @@ static int timer_notify (struct pt_regs *regs)
 	}
 
 	trace->addresses[i++] = (void *)regs->REG_INS_PTR;
-	
-	frame = (StackFrame *)regs->REG_FRAME_PTR;
-	
-	while (pages_present (frame)				&&
-	       i < SYSPROF_MAX_ADDRESSES			&&
-	       ((unsigned long)frame) < START_OF_STACK		&&
-	       (unsigned long)frame >= regs->REG_STACK_PTR)
+
+#if 0
+	if (is_user)
 	{
-		trace->addresses[i++] = (void *)frame->return_address;
-		frame = (StackFrame *)frame->next;
+#endif
+		StackFrame frame;
+		int result;
+
+		frame_pointer = (void *)regs->REG_FRAME_PTR;
+
+		while (((result = read_frame (frame_pointer, &frame)) == 0)		&&
+		       i < SYSPROF_MAX_ADDRESSES			&&
+		       ((unsigned long)frame_pointer) < START_OF_STACK	&&
+		       (unsigned long)frame_pointer >= regs->REG_STACK_PTR)
+		{
+			trace->addresses[i++] = (void *)frame.return_address;
+			frame_pointer = (StackFrame *)frame.next;
+		}
+
+#if 0
+		if (result) {
+			trace->addresses[i++] = (void *)0x23456789;
+			trace->addresses[i++] = current_thread_info()->addr_limit.seg;
+			trace->addresses[i++] = regs->REG_FRAME_PTR;
+			trace->addresses[i++] = result;
+			trace->addresses[i++] = 0x98765432;
+		}
+		else
+			trace->addresses[i++] = 0x10101010;
+#endif
+#if 0
 	}
+#endif
 	
 	trace->n_addresses = i;