From d9de1e5a368c82d74b8396a3b86023be6746dba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Sandmann?= Date: Thu, 19 May 2005 02:27:18 +0000 Subject: [PATCH] Remove ref-counting since it didn't actually do any good. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wed May 18 22:21:52 2005 Søren Sandmann * module/sysprof-module.c: Remove ref-counting since it didn't actually do any good. * sysprof.c (load_module): Use g_spawn_command_line_sync() instaed of system(). --- ChangeLog | 8 +++++ TODO | 18 +++++++++--- module/sysprof-module.c | 65 +++++++---------------------------------- sysprof.c | 20 +++++++++++-- 4 files changed, 49 insertions(+), 62 deletions(-) diff --git a/ChangeLog b/ChangeLog index eaba5b94..9623b2fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Wed May 18 22:21:52 2005 Søren Sandmann + + * module/sysprof-module.c: Remove ref-counting since it didn't + actually do any good. + + * sysprof.c (load_module): Use g_spawn_command_line_sync() instaed + of system(). + Sun May 15 11:56:30 2005 Søren Sandmann * module/sysprof-module.c: First attempt at making module robust diff --git a/TODO b/TODO index 1ecb30d5..cfc0e1a5 100644 --- a/TODO +++ b/TODO @@ -1,9 +1,5 @@ Before 0.9 -* Correctness - - When the module is unloaded, kill all processes blocking in read - - or block unloading until all processes have exited - * Interface - hook up menu items view/start etc (or possibly get rid of them or move them) @@ -17,6 +13,20 @@ Before 1.0: Before 1.2: +* Correctness + - When the module is unloaded, kill all processes blocking in read + - or block unloading until all processes have exited + Unfortunately this is basically impossible to do with a /proc + file (no open() notification). So, for 1.0 this will have to be + a dont-do-that-then. For 1.2, we should do it with a sysfs + file instead. + + - When the module is unloaded, can we somehow *guarantee* that no + kernel thread is active? Doesn't look like it; however we can + get very close by decreasing a ref count just before returning + from the module. (There may still be return instructions etc. + that will get run). + - See if there is a way to make it distcheck - grep FIXME - not10 - translation should be hooked up diff --git a/module/sysprof-module.c b/module/sysprof-module.c index 2a8d7074..c8110987 100644 --- a/module/sysprof-module.c +++ b/module/sysprof-module.c @@ -52,8 +52,6 @@ static SysprofStackTrace * tail = &stack_traces[0]; DECLARE_WAIT_QUEUE_HEAD (wait_for_trace); DECLARE_WAIT_QUEUE_HEAD (wait_for_exit); -static int exiting; - static void on_timer(unsigned long); static struct timer_list timer; @@ -72,22 +70,6 @@ struct userspace_reader * Source/target buffer must be kernel space, * Do not walk the page table directly, use get_user_pages */ - -static atomic_t ref_count; -static void ref(void) -{ - atomic_inc (&ref_count); -} -static void unref(void) -{ - atomic_dec (&ref_count); - wake_up_interruptible (&wait_for_exit); -} -static int refcount(void) -{ - return atomic_read (&ref_count); -} - static struct mm_struct * get_mm (struct task_struct *tsk) { @@ -354,11 +336,10 @@ do_generate (void *data) * the time we get here, it will be leaked. If __put_task_struct9) * was exported, then we could do this properly */ + atomic_dec (&(task)->usage); mod_timer(&timer, jiffies + INTERVAL); - - unref(); } struct work_struct work; @@ -366,16 +347,12 @@ struct work_struct work; static void on_timer(unsigned long dong) { - if (exiting) - return; - if (current && current->state == TASK_RUNNING && current->pid != 0) { get_task_struct (current); INIT_WORK (&work, do_generate, current); - ref(); schedule_work (&work); } else @@ -392,19 +369,14 @@ procfile_read(char *buffer, int *eof, void *data) { - if (exiting) - return -EBUSY; - - ref(); - if (head == tail) { - unref(); + if (head == tail) return -EWOULDBLOCK; - } - wait_event_interruptible (wait_for_trace, head != tail); + *buffer_location = (char *)tail; + if (tail++ == &stack_traces[N_TRACES - 1]) tail = &stack_traces[0]; - unref(); + return sizeof (SysprofStackTrace); } @@ -412,28 +384,20 @@ struct proc_dir_entry *trace_proc_file; static unsigned int procfile_poll(struct file *filp, poll_table *poll_table) { - if (exiting) - return -EBUSY; - - ref(); - if (head != tail) { - unref(); + if (head != tail) return POLLIN | POLLRDNORM; - } + poll_wait(filp, &wait_for_trace, poll_table); - if (head != tail) { - unref(); + + if (head != tail) return POLLIN | POLLRDNORM; - } - unref(); + return 0; } int init_module(void) { - ref(); - trace_proc_file = create_proc_entry ("sysprof-trace", S_IFREG | S_IRUGO, &proc_root); @@ -456,18 +420,9 @@ init_module(void) void cleanup_module(void) { - exiting = 1; - - unref(); - - wait_event_interruptible (wait_for_exit, (refcount() == 0)); - del_timer (&timer); remove_proc_entry("sysprof-trace", &proc_root); - - printk(KERN_ALERT "stopping sysprof module (refcount: %d)\n", - refcount()); } module_init (init_module); diff --git a/sysprof.c b/sysprof.c index 758cc095..6e933c9a 100644 --- a/sysprof.c +++ b/sysprof.c @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include "binfile.h" #include "watch.h" @@ -471,13 +473,25 @@ sorry (GtkWidget *parent_window, gtk_widget_destroy (dialog); } - static gboolean load_module (void) { - int retval = system ("/sbin/modprobe sysprof-module"); + int exit_status = -1; + char *dummy1, *dummy2; - return (retval == 0); + if (g_spawn_command_line_sync ("/sbin/modprobe sysprof-module", + &dummy1, &dummy2, + &exit_status, + NULL)) + { + if (WIFEXITED (exit_status)) + exit_status = WEXITSTATUS (exit_status); + + g_free (dummy1); + g_free (dummy2); + } + + return (exit_status == 0); } static void