From a3b1e000e63f4b030118d2238fd8584485ce7a69 Mon Sep 17 00:00:00 2001 From: Soeren Sandmann Date: Fri, 15 Apr 2005 20:38:58 +0000 Subject: [PATCH] updates Fri Apr 15 16:37:45 2005 Soeren Sandmann * TODO: updates * sysprof.c (sorry): If you hit profile when the module isn't loaded, pop up an annoying dialog. * sysprof-module.c: Clean-ups, remove various unused abstractions. --- ChangeLog | 9 ++++ TODO | 32 +++++++++-- sysprof-module.c | 136 ++++++++++------------------------------------- sysprof.c | 100 +++++++++++++++++----------------- 4 files changed, 117 insertions(+), 160 deletions(-) diff --git a/ChangeLog b/ChangeLog index f2931d74..48a58870 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +Fri Apr 15 16:37:45 2005 Soeren Sandmann + + * TODO: updates + + * sysprof.c (sorry): If you hit profile when the module isn't + loaded, pop up an annoying dialog. + + * sysprof-module.c: Clean-ups, remove various unused abstractions. + Sat Apr 9 17:49:13 2005 Søren Sandmann * COPYING: Add a copy of the GPL diff --git a/TODO b/TODO index 2c23c925..8309def0 100644 --- a/TODO +++ b/TODO @@ -103,8 +103,6 @@ Later: - Find out how to hack around gtk+ bug causing multiple double clicks to get eaten. -- Figure out what a 'disk profiler' is. - - Consider what it would take to take stacktraces of other languages - perl, @@ -121,7 +119,33 @@ Later: This function would behave essentially like a signal handler: couldn't call malloc(), couldn't call printf(), etc. - + +- Consider this usecase: + Someone is considering replacing malloc()/free() with a freelist + for a certain data structure. All use of this data structure is + confined to one function, foo(). It is now interesting to know + how much time that particular function spends on malloc() and free() + combined. + + Possible UI: + + - Select foo(), + - find an instance of malloc() + - shift-click on it, + - all traces with malloc are removed + - a new item "..." appears immeidately below foo() + - malloc is added below "..." + - same for free + - at this point, the desired data can be read at comulative + for "..." + + Actually, with this UI, you could potentially get rid of the + caller list: Just present the call tree under an root, + and use ... to single out the stuff you are interested in. + + Maybe also get rid of 'callers' by having a new "show details" + dialog or something. + - figure out a way to deal with both disk and CPU. Need to make sure that things that are UNINTERRUPTIBLE while there are RUNNING tasks are not considered bad. @@ -143,7 +167,7 @@ Later: traces has the same diskaccesses. Or visualize a set of squares with a color that is more saturated depending - on the number of unique stack traces that access it. The look for the + on the number of unique stack traces that access it. Then look for the lightly saturated ones. The input to the profiler would basically be diff --git a/sysprof-module.c b/sysprof-module.c index f8784116..15720d75 100644 --- a/sysprof-module.c +++ b/sysprof-module.c @@ -37,6 +37,7 @@ #include "sysprof-module.h" #include +#include MODULE_LICENSE("GPL"); MODULE_AUTHOR("Soeren Sandmann (sandmann@daimi.au.dk)"); @@ -51,34 +52,11 @@ static SysprofStackTrace * tail = &stack_traces[0]; DECLARE_WAIT_QUEUE_HEAD (wait_for_trace); DECLARE_WAIT_QUEUE_HEAD (wait_for_exit); -typedef void (* TimeoutFunc)(unsigned long data); +static int exiting; + static void on_timer(unsigned long); - -/* - * Wrappers to cover up kernel differences in timer handling - */ static struct timer_list timer; -static void -init_timeout (void) -{ - init_timer(&timer); -} - -static void -remove_timeout(void) -{ - del_timer (&timer); -} - -static void -add_timeout(unsigned int interval, - TimeoutFunc f) -{ - timer.function = f; - mod_timer(&timer, jiffies + INTERVAL); -} - typedef struct userspace_reader userspace_reader; struct userspace_reader { @@ -108,7 +86,8 @@ init_userspace_reader (userspace_reader *reader, * Do not walk the page table directly, use get_user_pages */ -static int x_access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write) +static int +x_access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write) { struct mm_struct *mm; struct vm_area_struct *vma; @@ -166,50 +145,8 @@ read_user_space (userspace_reader *reader, unsigned long address, unsigned long *result) { -#if 0 - unsigned long user_page = (address & PAGE_MASK); - int res; -#endif - return x_access_process_vm(reader->task, address, result, 4, 0); } -#if 0 - struct task_struct *tsk, unsigned long addr, void *buf, int len, int write); - - if (user_page == 0) - return 0; - - if (!reader->user_page || user_page != reader->user_page) { - int found; - struct page *page; - pte_t pte; - - found = get_user_pages (reader->task, reader->task->mm, - user_page, 1, 0, 0, &page, NULL); - - if (!found) - return 0; - - if (!pte_present (page)) - return 0; - - if (reader->user_page) - kunmap (reader->page); - - reader->kernel_page = (unsigned long)kmap (page); - reader->user_page = user_page; - reader->page = page; - - } - - if (get_user (res, (int *)(reader->kernel_page + (address - user_page))) != 0) - return 0; - - *result = res; - - return 1; -} -#endif static void done_userspace_reader (userspace_reader *reader) @@ -256,7 +193,7 @@ generate_stack_trace(struct task_struct *task, unsigned long addr; userspace_reader reader; int i; - + memset(trace, 0, sizeof (SysprofStackTrace)); trace->pid = task->pid; @@ -292,49 +229,32 @@ static void do_generate (void *data) { struct task_struct *task = data; - struct task_struct *g, *p; + + generate_stack_trace(task, head); + wake_up_process (task); + + if (head++ == &stack_traces[N_TRACES - 1]) + head = &stack_traces[0]; + + wake_up (&wait_for_trace); - /* Make sure the thread still exists */ - /* FIXME: this is probably not necessary anymore, now that - * we make the process sleep - */ - do_each_thread (g, p) { - if (p == task) { - generate_stack_trace(task, head); - - if (head++ == &stack_traces[N_TRACES - 1]) - head = &stack_traces[0]; - - wake_up (&wait_for_trace); - wake_up_process (task); - - goto out; - } - } while_each_thread (g, p); - - out: - add_timeout (INTERVAL, on_timer); + mod_timer(&timer, jiffies + INTERVAL); } static void on_timer(unsigned long dong) { -#if 0 - static const int cpu_profiler = 1; /* set to 0 to profile disk */ - - if (p->state == (cpu_profiler? TASK_RUNNING : TASK_UNINTERRUPTIBLE)) - ; -#endif - - if (current && current->state == TASK_RUNNING && current->pid != 0) { - INIT_WORK (&work, do_generate, current); - + if (current && current->state == TASK_RUNNING && current->pid != 0) + { set_current_state (TASK_UNINTERRUPTIBLE); + INIT_WORK (&work, do_generate, current); + schedule_work (&work); } - else { - add_timeout (INTERVAL, on_timer); + else + { + mod_timer(&timer, jiffies + INTERVAL); } } @@ -382,9 +302,9 @@ init_module(void) trace_proc_file->proc_fops->poll = procfile_poll; trace_proc_file->size = sizeof (SysprofStackTrace); - init_timeout(); - - add_timeout(INTERVAL, on_timer); + init_timer(&timer); + timer.function = on_timer; + mod_timer(&timer, jiffies + INTERVAL); printk(KERN_ALERT "starting sysprof module\n"); @@ -394,8 +314,10 @@ init_module(void) void cleanup_module(void) { - remove_timeout(); - + exiting = 1; + + del_timer (&timer); + remove_proc_entry("sysprof-trace", &proc_root); printk(KERN_ALERT "stopping sysprof module\n"); diff --git a/sysprof.c b/sysprof.c index 51b69538..28e91dd0 100644 --- a/sysprof.c +++ b/sysprof.c @@ -98,13 +98,6 @@ struct Application */ }; -static void -disaster (const char *what) -{ - fprintf (stderr, what); - exit (1); -} - static gboolean show_samples_timeout (gpointer data) { @@ -332,8 +325,10 @@ on_read (gpointer data) /* filename = trace.filename; */ for (i = 0; i < trace.n_addresses; ++i) + { process_ensure_map (process, trace.pid, (gulong)trace.addresses[i]); + } g_assert (!app->generating_profile); stack_stash_add_trace ( @@ -398,6 +393,31 @@ start_profiling (gpointer data) return FALSE; } +static void +sorry (GtkWidget *parent_window, + const gchar *format, + ...) +{ + va_list args; + char *message; + GtkWidget *dialog; + + va_start (args, format); + g_vasprintf (&message, format, args); + va_end (args); + + dialog = gtk_message_dialog_new (parent_window ? GTK_WINDOW (parent_window) : NULL, + GTK_DIALOG_DESTROY_WITH_PARENT, + GTK_MESSAGE_WARNING, + GTK_BUTTONS_OK, message); + g_free (message); + + gtk_window_set_title (GTK_WINDOW (dialog), APPLICATION_NAME " Warning"); + + gtk_dialog_run (GTK_DIALOG (dialog)); + gtk_widget_destroy (dialog); +} + static void on_start_toggled (GtkWidget *widget, gpointer data) { @@ -406,6 +426,29 @@ on_start_toggled (GtkWidget *widget, gpointer data) if (!gtk_toggle_tool_button_get_active ( GTK_TOGGLE_TOOL_BUTTON (app->start_button))) return; + + if (app->input_fd == -1) + { + int fd = open ("/proc/sysprof-trace", O_RDONLY); + if (fd < 0) + { + sorry (app->main_window, + "Can't open /proc/sysprof-trace. You need to insert\n" + "the sysprof kernel module. Type \n" + "\n" + " insmod sysprof-module.ko\n" + "\n" + "as root."); + + update_sensitivity (app); + return; + } + + app->input_fd = fd; + fd_add_watch (app->input_fd, app); + } + + fd_set_read_callback (app->input_fd, on_read); delete_data (app); @@ -688,9 +731,8 @@ ensure_profile (Application *app) if (app->profile) return; - /* take care of reentrancy */ app->profile = profile_new (app->stash); - + fill_lists (app); app->state = DISPLAYING; @@ -734,31 +776,6 @@ on_profile_toggled (GtkWidget *widget, gpointer data) } } -static void -sorry (GtkWidget *parent_window, - const gchar *format, - ...) -{ - va_list args; - char *message; - GtkWidget *dialog; - - va_start (args, format); - g_vasprintf (&message, format, args); - va_end (args); - - dialog = gtk_message_dialog_new (parent_window ? GTK_WINDOW (parent_window) : NULL, - GTK_DIALOG_DESTROY_WITH_PARENT, - GTK_MESSAGE_WARNING, - GTK_BUTTONS_OK, message); - free (message); - - gtk_window_set_title (GTK_WINDOW (dialog), APPLICATION_NAME " Warning"); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_widget_destroy (dialog); -} - static void on_reset_clicked (gpointer widget, gpointer data) { @@ -1242,21 +1259,6 @@ main (int argc, char **argv) app = application_new (); - app->input_fd = open ("/proc/sysprof-trace", O_RDONLY); - if (app->input_fd < 0) - { - disaster ("Can't open /proc/sysprof-trace. You need to insert\n" - "the sysprof kernel module. Type \n" - "\n" - " insmod sysprof-module.ko\n" - "\n" - "as root.\n" - "\n"); - } - - fd_add_watch (app->input_fd, app); - fd_set_read_callback (app->input_fd, on_read); - #if 0 nice (-19); g_timeout_add (10, on_timeout, app);