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

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

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 

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)
+	if (head == tail)
-		return -EBUSY;
-	
-	ref();
-	if (head == tail) {
-		unref();
 		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)
+	if (head != tail)
-		return -EBUSY;
+		return POLLIN | POLLRDNORM;
 	
-	ref();
-	if (head != tail) {
-		unref();
-		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);

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;
 
-    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