Remove ref-counting since it didn't actually do any good.

Wed May 18 22:21:52 2005  Søren Sandmann  <sandmann@redhat.com>

        * 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().

diff --git a/ChangeLog b/ChangeLog
index eaba5b9..9623b2f 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Wed May 18 22:21:52 2005  Søren Sandmann  <sandmann@redhat.com>
+
+       * 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  <sandmann@redhat.com>
 
        * module/sysprof-module.c: First attempt at making module robust

diff --git a/TODO b/TODO
index 1ecb30d..cfc0e1a 100644 (file)
--- 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 2a8d707..c811098 100644 (file)
--- 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 758cc09..6e933c9 100644 (file)
--- a/sysprof.c
+++ b/sysprof.c
@@ -27,6 +27,8 @@
 #include <glade/glade.h>
 #include <errno.h>
 #include <glib/gprintf.h>
+#include <sys/wait.h>
+#include <sys/types.h>
 
 #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;
+
+    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 (retval == 0);
+    return (exit_status == 0);
 }
 
 static void