Updates

Sat Jun 18 22:45:04 2005  Søren Sandmann  <sandmann@redhat.com>

	* TODO: Updates

	* configure.ac: Check for Linux 2.6.11

	* process.c (get_pidname): Present pid=-1 as [kernel].

	* module/sysprof-module.c: Use register_timer_hook() instead of
	a kernel timer. Set trace.pid to -1 if interrupt happens in
	kernel.

7 files changed, 192 insertions, 14 deletions

diff --git a/ChangeLog b/ChangeLog
index 793df35..61ed202 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+Sat Jun 18 22:45:04 2005  Søren Sandmann  <sandmann@redhat.com>
+
+	* TODO: Updates
+
+	* configure.ac: Check for Linux 2.6.11
+
+	* process.c (get_pidname): Present pid=-1 as [kernel].
+
+	* module/sysprof-module.c: Use register_timer_hook() instead of
+	a kernel timer. Set trace.pid to -1 if interrupt happens in
+	kernel.
+
 Sun Jun 12 20:30:37 2005  Soeren Sandmann  <sandmann@redhat.com>
 
 	* sysprof.c (build_gui): Disable type-ahead search for all the
diff --git a/README b/README
index d85073e..95b1b1a 100644
--- a/README
+++ b/README
@@ -6,7 +6,7 @@ See http://www.daimi.au.dk/~sandmann/sysprof/ for more information
 
 - You need gtk+ 2.6.0 or better.
 
-- You need a 2.6 kernel. On SMP kernels the module produces unreliable data.
+- You need at least Linux 2.6.11
 
 - The module must be compiled with the same compiler that compiled the 
   kernel it is going to be used with. For most systems that is just
diff --git a/TODO b/TODO
index 134b315..000261c 100644
--- a/TODO
+++ b/TODO
@@ -14,11 +14,17 @@ Before 1.0:
    - Announce on Advogato
    - Announce on gnome-announce
    - Announce on devtools list (?)
-	
-Before 1.2:
 
+Before 1.2:
+	
 * See if the auto-expanding can be made more intelligent
 
+* Send entire stack to user space, then do stackwalking there
+
+* If interrupt happens in kernel mode, send both
+  kernel stack and user space stack, have userspace stitch them
+  together.
+
 * Correctness
 	- When the module is unloaded, kill all processes blocking in read
 		- or block unloading until all processes have exited
@@ -41,13 +47,17 @@ Before 1.2:
 	UI: "generate screenshot" menu item pops up a window with
 	a text area + a radio buttons "text/html". When you flick
 	them, the text area is automatically updated.
+
 - Fixing the oops in kernels < 2.6.11
 
+	- Probably just require 2.6.11 (necessary for timer interrupt
+          based anyway).
+
 	- Make the process waiting in poll() responsible for extracting
 	  the backtrace. Give a copy of the entire stack rather than doing
 	  the walk inside the kernel. That would allow us to do more complex 
-	  algorithms in userspace (though we'd lose the ability to do non-racy
-	  file naming).
+	  algorithms in userspace. Though we'd lose the ability to do non-racy
+	  file naming. We could pass a list of the process mappings though.
 
 		New model:
 			- Two arrays, 
@@ -201,6 +211,9 @@ http://www.linuxbase.org/spec/booksets/LSB-Embedded/LSB-Embedded/ehframe.html
 
 Later:
 
+- See if it is possible to group the X server activity under the process that
+  generated it.
+
 - .desktop file
 	[Is this worth it? You will often want to start it as root, 
 	 and you will need to insert the module from the command line]
@@ -348,6 +361,8 @@ Later:
 
 DONE:
 
+* Timer interrupt based
+
 * Interface
 	- Consider expanding a few more levels of a new descendants tree 
 		- Algorithm should be expand in proportion to the
diff --git a/configure.ac b/configure.ac
index df1a77e..bca61a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,6 +61,7 @@ AC_CHECK_LIB(bfd, bfd_get_error, [DEP_LIBS="$DEP_LIBS -lbfd -liberty"],
   AC_MSG_ERROR([libbfd is required to compile sysprof]),
   -liberty)
 
+
 # emit files
 
 AC_SUBST(DEP_LIBS)
@@ -69,4 +70,19 @@ AC_CONFIG_FILES([
 Makefile
 ])
 
+# Kernel version
+
+KMAJOR=`uname -r | cut -d"." -f 1`
+KMINOR=`uname -r | cut -d"." -f 2`
+KMICRO=`uname -r | cut -d"." -f 3 | cut -d"-" -f 1`
+
+if [[ $KMICRO -lt 11 ]] ; then
+   if [[ $KMICRO -gt 8 ]]; then
+      echo
+      echo Linux \>= 2.6.11 is required
+      echo
+      exit 1
+   fi
+fi
+
 AC_OUTPUT
diff --git a/module/sysprof-module.c b/module/sysprof-module.c
index f785731..09fb8d7 100644
--- a/module/sysprof-module.c
+++ b/module/sysprof-module.c
@@ -42,7 +42,7 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Soeren Sandmann (sandmann@daimi.au.dk)");
 
-#define SAMPLES_PER_SECOND (100)
+#define SAMPLES_PER_SECOND (256)
 #define INTERVAL (HZ / SAMPLES_PER_SECOND)
 #define N_TRACES 256
 
@@ -52,8 +52,12 @@ static SysprofStackTrace *	tail = &stack_traces[0];
 DECLARE_WAIT_QUEUE_HEAD (wait_for_trace);
 DECLARE_WAIT_QUEUE_HEAD (wait_for_exit);
 
+#if 0
 static void on_timer(unsigned long);
+#endif
+#if 0
 static struct timer_list timer;
+#endif
 
 typedef struct userspace_reader userspace_reader;
 struct userspace_reader
@@ -63,6 +67,7 @@ struct userspace_reader
 	unsigned long *cache;
 };
 
+#if 0
 /* This function was mostly cutted and pasted from ptrace.c
  */
 
@@ -93,7 +98,9 @@ put_mm (struct mm_struct *mm)
 	mmput(mm);
 #endif
 }
+#endif
 
+#if 0
 static int
 x_access_process_vm (struct task_struct *tsk,
 		     unsigned long addr,
@@ -153,7 +160,9 @@ x_access_process_vm (struct task_struct *tsk,
 	
 	return buf - old_buf;
 }
+#endif
 
+#if 0
 static int
 init_userspace_reader (userspace_reader *reader,
 		       struct task_struct *task)
@@ -165,7 +174,9 @@ init_userspace_reader (userspace_reader *reader,
 	reader->cache_address = 0x0;
 	return 1;
 }
+#endif
 
+#if 0
 static int
 page_readable (userspace_reader *reader, unsigned long address)
 {
@@ -188,7 +199,9 @@ page_readable (userspace_reader *reader, unsigned long address)
 		
 		return 1;
 }
+#endif
 
+#if 0
 static int
 read_user_space (userspace_reader *reader,
 		 unsigned long address,
@@ -219,12 +232,15 @@ read_user_space (userspace_reader *reader,
 	*result = reader->cache[index];
 	return 1;
 }
+#endif
 
+#if 0
 static void
 done_userspace_reader (userspace_reader *reader)
 {
 	kfree (reader->cache);
 }
+#endif
 
 typedef struct StackFrame StackFrame;
 struct StackFrame {
@@ -232,6 +248,7 @@ struct StackFrame {
 	unsigned long return_address;
 };
 
+#if 0
 static int
 read_frame (userspace_reader *reader, unsigned long addr, StackFrame *frame)
 {
@@ -249,7 +266,9 @@ read_frame (userspace_reader *reader, unsigned long addr, StackFrame *frame)
 
 	return 1;
 }
+#endif
 
+#if 0
 static struct pt_regs *
 get_regs (struct task_struct *task)
 {
@@ -268,16 +287,13 @@ get_regs (struct task_struct *task)
 	return task_pt_regs (task);
 #endif
 }
+#endif
 
+#if 0
 static void
 generate_stack_trace(struct task_struct *task,
 		     SysprofStackTrace *trace)
 {
-#ifdef CONFIG_HIGHMEM
-#  define START_OF_STACK 0xFF000000
-#else
-#  define START_OF_STACK 0xBFFFFFFF
-#endif
 	struct pt_regs *regs = get_regs (task);  // task->thread.regs;
 	
 	StackFrame frame;
@@ -319,14 +335,16 @@ generate_stack_trace(struct task_struct *task,
 	else
 		trace->truncated = 0;
 }
+#endif
 
+#if 0
 static void
 do_generate (void *data)
 {
 	struct task_struct *task = data;
 
 	generate_stack_trace(task, head);
-		
+			
 	if (head++ == &stack_traces[N_TRACES - 1])
 		head = &stack_traces[0];
 
@@ -341,9 +359,11 @@ do_generate (void *data)
 	
 	mod_timer(&timer, jiffies + INTERVAL);
 }
+#endif
 
 struct work_struct work;
 
+#if 0
 static void
 on_timer(unsigned long dong)
 {
@@ -360,6 +380,110 @@ on_timer(unsigned long dong)
 		mod_timer(&timer, jiffies + INTERVAL);
 	}
 }
+#endif
+
+/**
+ * pages_present() from OProfile
+ *
+ * Copyright 2002 OProfile authors
+ *
+ * author John Levon
+ * author David Smith
+ */
+
+#ifdef CONFIG_X86_4G
+/* With a 4G kernel/user split, user pages are not directly
+ * accessible from the kernel, so don't try
+ */
+static int pages_present(StackFrame * head)
+{
+	return 0;
+}
+#else
+/* check that the page(s) containing the frame head are present */
+static int pages_present(StackFrame * head)
+{
+	struct mm_struct * mm = current->mm;
+
+	/* FIXME: only necessary once per page */
+	if (!check_user_page_readable(mm, (unsigned long)head))
+		return 0;
+
+	return check_user_page_readable(mm, (unsigned long)(head + 1));
+}
+#endif /* CONFIG_X86_4G */
+
+static int
+timer_notify (struct pt_regs *regs)
+{
+#ifdef CONFIG_HIGHMEM
+#  define START_OF_STACK 0xFF000000
+#else
+#  define START_OF_STACK 0xBFFFFFFF
+#endif
+	StackFrame *frame;
+	static int n_samples;
+	SysprofStackTrace *trace = head;
+	int i;
+	int is_user;
+
+	if ((n_samples++ % INTERVAL) != 0)
+		return 0;
+
+	is_user = user_mode(regs);
+
+	if (!current || current->pid == 0)
+		return 0;
+	
+	if (is_user && current->state != TASK_RUNNING)
+		return 0;
+
+	if (!is_user)
+	{
+		/* kernel */
+		
+		trace->pid = -1;
+		trace->truncated = 0;
+		trace->n_addresses = 1;
+		trace->addresses[0] = 0x0;
+	}
+	else
+	{
+		memset(trace, 0, sizeof (SysprofStackTrace));
+		
+		trace->pid = current->pid;
+		trace->truncated = 0;
+		
+		trace->addresses[0] = (void *)regs->eip;
+		
+		i = 1;
+		
+		frame = (StackFrame *)regs->ebp;
+		
+		while (pages_present (frame)				&&
+		       i < SYSPROF_MAX_ADDRESSES			&&
+		       ((unsigned long)frame) < START_OF_STACK		&&
+		       (unsigned long)frame >= regs->esp)
+		{
+			trace->addresses[i++] = (void *)frame->return_address;
+			frame = (StackFrame *)frame->next;
+		}
+		
+		trace->n_addresses = i;
+
+		if (i == SYSPROF_MAX_ADDRESSES)
+			trace->truncated = 1;
+		else
+			trace->truncated = 0;
+	}
+	
+	if (head++ == &stack_traces[N_TRACES - 1])
+		head = &stack_traces[0];
+	
+	wake_up (&wait_for_trace);
+	
+	return 0;
+}
 
 static int
 procfile_read(char *buffer, 
@@ -407,10 +531,14 @@ init_module(void)
 	trace_proc_file->read_proc = procfile_read;
 	trace_proc_file->proc_fops->poll = procfile_poll;
 	trace_proc_file->size = sizeof (SysprofStackTrace);
+
+	register_timer_hook (timer_notify);
 	
+#if 0
 	init_timer(&timer);
 	timer.function = on_timer;
 	mod_timer(&timer, jiffies + INTERVAL);
+#endif
 	
 	printk(KERN_ALERT "starting sysprof module\n");
 	
@@ -420,8 +548,12 @@ init_module(void)
 void
 cleanup_module(void)
 {
+#if 0
 	del_timer (&timer);
+#endif
 
+	unregister_timer_hook (timer_notify);
+	
 	remove_proc_entry("sysprof-trace", &proc_root);
 }
 
diff --git a/module/sysprof-module.h b/module/sysprof-module.h
index 6d6255e..66a11ae 100644
--- a/module/sysprof-module.h
+++ b/module/sysprof-module.h
@@ -26,7 +26,7 @@ typedef struct SysprofStackTrace SysprofStackTrace;
 
 struct SysprofStackTrace
 {
-    int	pid;
+    int	pid;		/* -1 if in kernel */
     int truncated;
     int n_addresses;	/* note: this can be 1 if the process was compiled
 			 * with -fomit-frame-pointer or is otherwise weird
diff --git a/process.c b/process.c
index 699eb6b..a28e10c 100644
--- a/process.c
+++ b/process.c
@@ -292,7 +292,10 @@ get_statname (int pid)
 static char *
 get_pidname (int pid)
 {
-    return g_strdup_printf ("[pid %d]", pid);
+    if (pid == -1)
+	return g_strdup_printf ("kernel", pid);
+    else
+	return g_strdup_printf ("pid %d", pid);
 }
 
 static char *