Use an array instead of a list. Look for vmlinux in the source directory.

2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>

        * process.c (look_for_vmlinux): Use an array instead of a
        list. Look for vmlinux in the source directory.

        * elfparser.c (elf_parser_get_crc32): Only use MADV_DONTNEED if
        the data is file-backed.

        * TODO: updates.

        Various formatting fixes


svn path=/trunk/; revision=385

ChangeLog

@@ -1,3 +1,15 @@
+2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>
+
+	* process.c (look_for_vmlinux): Use an array instead of a
+	list. Look for vmlinux in the source directory.
+
+	* elfparser.c (elf_parser_get_crc32): Only use MADV_DONTNEED if
+	the data is file-backed.
+
+	* TODO: updates.
+
+	Various formatting fixes
+	
 2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>
 
 	* profile.c (add_trace_to_tree): Make this a two-pass

TODO

@@ -389,18 +389,37 @@ Before 1.2:
 		  placed? (We should avoid any "location of vmlinux" type
 		  questions if at all possible).
 
-  regarding crossing system call barriers: Find out about the virtual dso
+         We do now copy the kernel stack to userspace and do a
-  that linux uses to do fast system calls:
+	 heuristic stack walk there. It may be better at some point to
+	 use dump_trace() in the kernel since that will do some
+	 filtering by itself.
 
-	http://lkml.org/lkml/2002/12/18/218
+  Notes about kernel symbol lookup:
 
-  and what that actually makes the stack look like. (We may want to just
+  	- /proc/kallsym is there, but it includes things like labels.
-  special case this fake dso in the symbol lookup code).
+          There is no way to tell them from functions
 
-  Maybe get_user_pages() is the way forward at least for some stuff.
+	- We can search for a kernel binary with symbols. If the
+ 	  kernel-debug package is installed, or if the user compiled
+ 	  his own kernel, we will find one. This is a regular elf file.
+	  It also includes labels, but we can tell by the STT_INFO field
+	  which is which.
 
-  note btw. that the kernel pages are only one or two pages, so we 
+	  Note though that for some labels we do actually want to
-  could easily just dump them to userspace.
+	  treat them as functions. For example the "page_fault" label,
+	  which is function by itself. We can recognize these by the
+	  fact that their symbols have a size. However, the _start
+	  function in normal applications does not have a size, so the
+	  heuristic should be something like this:
+
+	     - If the address is inside the range of some symbol, use
+               that symbol
+	     
+	     - Otherwise, if the closest symbol is a function with
+	       size 0, use that function.
+
+	  This means the datastructure will probably have to be done a
+	  little differently.
 
 - See if there is a way to make it distcheck
 
@@ -743,6 +762,19 @@ Later:
 
 -=-=-=-=-=-=-=-=-=-=-=-=-=-=- ALREADY DONE: -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 
+* regarding crossing system call barriers: Find out about the virtual dso
+  that linux uses to do fast system calls:
+
+	http://lkml.org/lkml/2002/12/18/218
+
+  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.
+
+  note btw. that the kernel pages are only one or two pages, so we 
+  could easily just dump them to userspace.
+
 * In profile.c, change "non_recursive" to "cumulative", and
   "marked_non_recursive" to a boolean "charged". This is tricky code,
   so be careful. Possibly make it a two-pass operation:

collector.c

@@ -275,7 +275,8 @@ open_fd (Collector *collector,
 	}
     }
     
-    map_area = mmap (NULL, sizeof (SysprofMmapArea), PROT_READ, MAP_SHARED, fd, 0);
+    map_area = mmap (NULL, sizeof (SysprofMmapArea),
+		     PROT_READ, MAP_SHARED, fd, 0);
     
     if (map_area == MAP_FAILED)
     {
@@ -387,7 +388,12 @@ lookup_symbol (Process *process, gpointer address,
 	gulong offset;
 	sym = process_lookup_kernel_symbol ((gulong)address, &offset);
 
-	/* If offset is 0, it is a callback, not a return address */
+	/* If offset is 0, it is a callback, not a return address.
+	 *
+	 * If "first_kernel_addr" is true, then the address is an
+	 * instruction pointer, not a return address, so it may
+	 * legitimately be at offset 0.
+	 */
 	if (offset == 0 && !first_kernel_addr)
 	    sym = NULL;
 
@@ -438,6 +444,7 @@ resolve_symbols (GList *trace, gint size, gpointer data)
 	
 	if (address == GINT_TO_POINTER (0x01))
 	    in_kernel = FALSE;
+	
 	symbol = lookup_symbol (process, address, info->unique_symbols,
 				in_kernel, first_kernel_addr);
 	first_kernel_addr = FALSE;

elfparser.c

@@ -258,11 +258,7 @@ elf_parser_new (const char *filename,
 #if 0
     g_print ("Elf file: %s  (debug: %s)\n",
 	     filename, elf_parser_get_debug_link (parser, NULL));
-#endif
     
-    parser->file = file;
-    
-#if 0
     if (!parser->symbols)
 	g_print ("at this point %s has no symbols\n", filename);
 #endif
@@ -339,7 +335,8 @@ elf_parser_get_crc32 (ElfParser *parser)
      * We could be more exact here, but it's only a few minor
      * pagefaults.
      */
-    madvise ((char *)data, length, MADV_DONTNEED);
+    if (parser->file)
+	madvise ((char *)data, length, MADV_DONTNEED);
 
     return ~crc & 0xffffffff;
 }
@@ -461,19 +458,23 @@ read_table (ElfParser *parser,
 	    parser->symbols[n_functions].address = addr;
 	    parser->symbols[n_functions].offset = offset;
 
+	    n_functions++;
+
 #if 0
-	    g_print ("   symbol: %s:   %d\n", get_string_indirect (parser->parser,
+	    g_print ("symbol: %s:   %d\n", get_string_indirect (parser->parser,
 								   parser->sym_format, "st_name",
 								   str_table->offset), addr);
 #endif
-	    n_functions++;
-	}
 #if 0
+	    g_print ("   sym %d in %p (info: %d:%d) (func:global  %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL);
+#endif
+	}
 	else if (addr != 0)
 	{
-	    g_print ("rejecting %d in %p (info: %d:%d) (func:global  %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL);
+#if 0
-	}
+	    g_print ("   rejecting %d in %p (info: %d:%d) (func:global  %d:%d)\n", addr, parser, info & 0xf, info >> 4, STT_FUNC, STB_GLOBAL);
 #endif
+	}
 	
 	bin_parser_seek_record (parser->parser, parser->sym_format, 1);
     }

process.c

@@ -23,6 +23,9 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/stat.h>
+#include <sys/utsname.h>
+#include <sys/types.h>
+#include <fcntl.h>
 #include <errno.h>
 #include <unistd.h>
 #include <string.h>
@@ -414,14 +417,6 @@ process_get_from_pid (int pid)
     return p;
 }
 
-
-
-#include <sys/utsname.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <string.h>
-
 static gboolean
 file_exists (const char *name)
 {
@@ -441,35 +436,31 @@ look_for_vmlinux (void)
 {
     struct utsname utsname;
     char *result;
-    GList *names;
+    char **s;
-    GList *list;
+    char *names[4];
+
     uname (&utsname);
 
-    names = NULL;
+    names[0] = g_strdup_printf (
-    
+	"/usr/lib/debug/lib/modules/%s/vmlinux", utsname.release);
-    names = g_list_prepend (
+    names[1] = g_strdup_printf (
-	names, g_strdup_printf (
+	"/lib/modules/%s/source/vmlinux", utsname.release);
-	    "/usr/lib/debug/lib/modules/%s/vmlinux", utsname.release));
+    names[2] = g_strdup_printf (
-    
+	"/boot/vmlinux-%s", utsname.release);
-    names = g_list_prepend (
+    names[3] = NULL;
-	names, g_strdup_printf (
-	    "/boot/vmlinux-%s", utsname.release));
 
     result = NULL;
-    
+    for (s = names; *s; s++)
-    for (list = names; list != NULL; list = list->next)
     {
-	char *name = list->data;
+	if (file_exists (*s))
-	
-	if (file_exists (name))
 	{
-	    result = g_strdup (name);
+	    result = g_strdup (*s);
 	    break;
 	}
     }
 
-    g_list_foreach (names, (GFunc)g_free, NULL);
+    for (s = names; *s; s++)
-    g_list_free (names);
+	g_free (*s);
 
     return result;
 }
@@ -571,6 +562,8 @@ get_kernel_symbols (void)
     static GArray *kernel_syms;
     static gboolean initialized = FALSE;
 
+    find_kernel_binary();
+    
     if (!initialized)
     {
 	char *kallsyms;

sysprof.c

@@ -965,9 +965,11 @@ expand_descendants_tree (Application *app)
 	n_children = gtk_tree_model_iter_n_children (model, &best_iter);
 	
 	if (n_children && (best_value / top_value) > 0.04 &&
-	    (n_children + gtk_tree_path_get_depth (best_path)) / (double)max_rows < (best_value / top_value) )
+	    (n_children + gtk_tree_path_get_depth (best_path)) /
+	    (double)max_rows < (best_value / top_value) )
 	{
-	    gtk_tree_view_expand_row (GTK_TREE_VIEW (app->descendants_view), best_path, FALSE);
+	    gtk_tree_view_expand_row (
+		GTK_TREE_VIEW (app->descendants_view), best_path, FALSE);
 	    n_rows += n_children;
 	    
 	    if (gtk_tree_path_get_depth (best_path) < 4)
@@ -1480,11 +1482,11 @@ build_gui (Application *app)
     g_signal_connect (G_OBJECT (app->screenshot_item), "activate",
 		      G_CALLBACK (on_screenshot_activated), app);
     
-    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "quit")), "activate",
+    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "quit")),
-		      G_CALLBACK (on_delete), NULL);
+		      "activate", G_CALLBACK (on_delete), NULL);
     
-    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "about")), "activate",
+    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "about")),
-		      G_CALLBACK (on_about_activated), app);
+		      "activate", G_CALLBACK (on_about_activated), app);
     
     /* TreeViews */
     
@@ -1492,41 +1494,58 @@ build_gui (Application *app)
     app->object_view =
 	(GtkTreeView *)glade_xml_get_widget (xml, "object_view");
     gtk_tree_view_set_enable_search (app->object_view, FALSE);
-    col = add_plain_text_column (app->object_view, _("Functions"), OBJECT_NAME);
+    col = add_plain_text_column (app->object_view, _("Functions"),
-    add_double_format_column (app->object_view, _("Self"), OBJECT_SELF, "%.2f ");
+				 OBJECT_NAME);
-    add_double_format_column (app->object_view, _("Total"), OBJECT_TOTAL, "%.2f ");
+    add_double_format_column (app->object_view, _("Self"),
+			      OBJECT_SELF, "%.2f ");
+    add_double_format_column (app->object_view, _("Total"),
+			      OBJECT_TOTAL, "%.2f ");
     selection = gtk_tree_view_get_selection (app->object_view);
-    g_signal_connect (selection, "changed", G_CALLBACK (on_object_selection_changed), app);
+    g_signal_connect (selection, "changed",
+		      G_CALLBACK (on_object_selection_changed), app);
     gtk_tree_view_column_set_expand (col, TRUE);
     
     /* callers view */
-    app->callers_view = (GtkTreeView *)glade_xml_get_widget (xml, "callers_view");
+    app->callers_view =
+	(GtkTreeView *)glade_xml_get_widget (xml, "callers_view");
     gtk_tree_view_set_enable_search (app->callers_view, FALSE);
-    col = add_plain_text_column (app->callers_view, _("Callers"), CALLERS_NAME);
+    col = add_plain_text_column (app->callers_view, _("Callers"),
-    add_double_format_column (app->callers_view, _("Self"), CALLERS_SELF, "%.2f ");
+				 CALLERS_NAME);
-    add_double_format_column (app->callers_view, _("Total"), CALLERS_TOTAL, "%.2f ");
+    add_double_format_column (app->callers_view, _("Self"),
+			      CALLERS_SELF, "%.2f ");
+    add_double_format_column (app->callers_view, _("Total"),
+			      CALLERS_TOTAL, "%.2f ");
     g_signal_connect (app->callers_view, "row-activated",
 		      G_CALLBACK (on_callers_row_activated), app);
     gtk_tree_view_column_set_expand (col, TRUE);
     
     /* descendants view */
-    app->descendants_view = (GtkTreeView *)glade_xml_get_widget (xml, "descendants_view");
+    app->descendants_view =
+	(GtkTreeView *)glade_xml_get_widget (xml, "descendants_view");
     gtk_tree_view_set_enable_search (app->descendants_view, FALSE);
-    col = add_plain_text_column (app->descendants_view, _("Descendants"), DESCENDANTS_NAME);
+    col = add_plain_text_column (app->descendants_view, _("Descendants"),
-    add_double_format_column (app->descendants_view, _("Self"), DESCENDANTS_SELF, "%.2f ");
+				 DESCENDANTS_NAME);
-    add_double_format_column (app->descendants_view, _("Cumulative"), DESCENDANTS_CUMULATIVE, "%.2f ");
+    add_double_format_column (app->descendants_view, _("Self"),
+			      DESCENDANTS_SELF, "%.2f ");
+    add_double_format_column (app->descendants_view, _("Cumulative"),
+			      DESCENDANTS_CUMULATIVE, "%.2f ");
     g_signal_connect (app->descendants_view, "row-activated",
 		      G_CALLBACK (on_descendants_row_activated), app);
     g_signal_connect (app->descendants_view, "row_expanded",
-		      G_CALLBACK (on_descendants_row_expanded_or_collapsed), app);
+		      G_CALLBACK (on_descendants_row_expanded_or_collapsed),
+		      app);
     g_signal_connect (app->descendants_view, "row_collapsed",
-		      G_CALLBACK (on_descendants_row_expanded_or_collapsed), app);
+		      G_CALLBACK (on_descendants_row_expanded_or_collapsed),
+		      app);
     gtk_tree_view_column_set_expand (col, TRUE);
     
     /* screenshot window */
-    app->screenshot_window = glade_xml_get_widget (xml, "screenshot_window");
+    app->screenshot_window =
-    app->screenshot_textview = glade_xml_get_widget (xml, "screenshot_textview");
+	glade_xml_get_widget (xml, "screenshot_window");
-    app->screenshot_close_button = glade_xml_get_widget (xml, "screenshot_close_button");
+    app->screenshot_textview =
+	glade_xml_get_widget (xml, "screenshot_textview");
+    app->screenshot_close_button =
+	glade_xml_get_widget (xml, "screenshot_close_button");
     
     g_signal_connect (app->screenshot_window, "delete_event",
 		      G_CALLBACK (on_screenshot_window_delete), app);