diff --git a/ChangeLog b/ChangeLog index 3e30c93a..7ba4201f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2007-10-22 Soren Sandmann + + * 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 * profile.c (add_trace_to_tree): Make this a two-pass diff --git a/TODO b/TODO index 027c187f..98d26304 100644 --- 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: + 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. - 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 - special case this fake dso in the symbol lookup code). + - /proc/kallsym is there, but it includes things like labels. + 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 - could easily just dump them to userspace. + 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 46df4902..22d71e85 100644 --- 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 53e0e322..688c4fba 100644 --- 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 11a20741..5d60cffb 100644 --- a/process.c +++ b/process.c @@ -23,6 +23,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -414,14 +417,6 @@ process_get_from_pid (int pid) return p; } - - -#include -#include -#include -#include -#include - 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 ee6a9496..fa3d5528 100644 --- 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);