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

diff --git a/ChangeLog b/ChangeLog
index 3e30c93..7ba4201 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 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
        algorithm, one pass to add the trace, and one to do the
        accounting.

diff --git a/TODO b/TODO
index 027c187..98d2630 100644 (file)
--- a/TODO
+++ b/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
-  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.
+         We do now copy the kernel stack to userspace and do a
+        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.
+
+  Notes about kernel symbol lookup:
+
+       - /proc/kallsym is there, but it includes things like labels.
+          There is no way to tell them from functions
+
+       - 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 though that for some labels we do actually want to
+         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:

diff --git a/collector.c b/collector.c
index 46df490..22d71e8 100644 (file)
--- a/collector.c
+++ b/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;

diff --git a/elfparser.c b/elfparser.c
index 53e0e32..688c4fb 100644 (file)
--- a/elfparser.c
+++ b/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);
     }

diff --git a/process.c b/process.c
index 11a2074..5d60cff 100644 (file)
--- a/process.c
+++ b/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,36 +436,32 @@ look_for_vmlinux (void)
 {
     struct utsname utsname;
     char *result;
-    GList *names;
-    GList *list;
+    char **s;
+    char *names[4];
+
     uname (&utsname);
-    
-    names = NULL;
-    
-    names = g_list_prepend (
-       names, g_strdup_printf (
-           "/usr/lib/debug/lib/modules/%s/vmlinux", utsname.release));
-    
-    names = g_list_prepend (
-       names, g_strdup_printf (
-           "/boot/vmlinux-%s", utsname.release));
-    
+
+    names[0] = g_strdup_printf (
+       "/usr/lib/debug/lib/modules/%s/vmlinux", utsname.release);
+    names[1] = g_strdup_printf (
+       "/lib/modules/%s/source/vmlinux", utsname.release);
+    names[2] = g_strdup_printf (
+       "/boot/vmlinux-%s", utsname.release);
+    names[3] = NULL;
+
     result = NULL;
-    
-    for (list = names; list != NULL; list = list->next)
+    for (s = names; *s; s++)
     {
-       char *name = list->data;
-       
-       if (file_exists (name))
+       if (file_exists (*s))
        {
-           result = g_strdup (name);
+           result = g_strdup (*s);
            break;
        }
     }
-    
-    g_list_foreach (names, (GFunc)g_free, NULL);
-    g_list_free (names);
-    
+
+    for (s = names; *s; s++)
+       g_free (*s);
+
     return result;
 }
 
@@ -570,6 +561,8 @@ get_kernel_symbols (void)
 {
     static GArray *kernel_syms;
     static gboolean initialized = FALSE;
+
+    find_kernel_binary();
     
     if (!initialized)
     {

diff --git a/sysprof.c b/sysprof.c
index ee6a949..fa3d552 100644 (file)
--- a/sysprof.c
+++ b/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_CALLBACK (on_delete), NULL);
+    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "quit")),
+                     "activate", G_CALLBACK (on_delete), NULL);
     
-    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "about")), "activate",
-                     G_CALLBACK (on_about_activated), app);
+    g_signal_connect (G_OBJECT (glade_xml_get_widget (xml, "about")),
+                     "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);
-    add_double_format_column (app->object_view, _("Self"), OBJECT_SELF, "%.2f ");
-    add_double_format_column (app->object_view, _("Total"), OBJECT_TOTAL, "%.2f ");
+    col = add_plain_text_column (app->object_view, _("Functions"),
+                                OBJECT_NAME);
+    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);
-    add_double_format_column (app->callers_view, _("Self"), CALLERS_SELF, "%.2f ");
-    add_double_format_column (app->callers_view, _("Total"), CALLERS_TOTAL, "%.2f ");
+    col = add_plain_text_column (app->callers_view, _("Callers"),
+                                CALLERS_NAME);
+    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);
-    add_double_format_column (app->descendants_view, _("Self"), DESCENDANTS_SELF, "%.2f ");
-    add_double_format_column (app->descendants_view, _("Cumulative"), DESCENDANTS_CUMULATIVE, "%.2f ");
+    col = add_plain_text_column (app->descendants_view, _("Descendants"),
+                                DESCENDANTS_NAME);
+    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_textview = glade_xml_get_widget (xml, "screenshot_textview");
-    app->screenshot_close_button = glade_xml_get_widget (xml, "screenshot_close_button");
+    app->screenshot_window =
+       glade_xml_get_widget (xml, "screenshot_window");
+    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);