Dist and install udev rule.

2005-12-20  Kristian Høgsberg  <krh@redhat.com>

        * Makefile.am: Dist and install udev rule.

        * collector.c: (open_fd):
        * sysprof-text.c: (no_module):
        * sysprof.c: (on_start_toggled): Update device filename.

        * 60-sysprof.rules: New udev rule file to set permissions for
        sysprof char device.

        * module/sysprof-module.c: Switch kernel module to use a misc char
        device instead.  Start and stop the timer on device open and
        close instead of module load and unload.

60-sysprof.rules

@@ -0,0 +1,2 @@
+KERNEL=="sysprof-trace",                  MODE="0644"
+

ChangeLog

@@ -1,3 +1,18 @@
+2005-12-20  Kristian Høgsberg  <krh@redhat.com>
+
+	* Makefile.am: Dist and install udev rule.
+
+	* collector.c: (open_fd):
+	* sysprof-text.c: (no_module):
+	* sysprof.c: (on_start_toggled): Update device filename.
+
+	* 60-sysprof.rules: New udev rule file to set permissions for
+	sysprof char device.
+
+	* module/sysprof-module.c: Switch kernel module to use a misc char
+	device instead.  Start and stop the timer on device open and
+	close instead of module load and unload.
+
 Tue Dec 20 12:19:34 2005  Søren Sandmann  <sandmann@redhat.com>
 
 	* configure.ac: Add backslashes, pointed out by Ralph Siemsen.

Makefile.am

@@ -37,6 +37,9 @@ sysprof_CPPFLAGS =				\
 
 sysprof_LDADD = $(GUI_DEP_LIBS)
 
+udevdir = $(sysconfdir)/udev/rules.d
+dist_udev_DATA = 60-sysprof.rules
+
 pixmapsdir = $(datadir)/pixmaps
 
 dist_pkgdata_DATA = sysprof.glade

TODO

@@ -150,21 +150,6 @@ Before 1.2:
 
   Maybe get_user_pages() is the way forward at least for some stuff.
 
-* 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 and
-	  kobject 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 close by decreasing a ref count just before returning
-	  from the module. (There may still be return instructions etc.
-	  that will get run). This may not be an issue with the timer
-  	  based scanning we are using currently.
-
 - See if there is a way to make it distcheck
 
 - grep FIXME - not10
@@ -432,6 +417,21 @@ Later:
 
 DONE:
 
+* 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 and
+	  kobject 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 close by decreasing a ref count just before returning
+	  from the module. (There may still be return instructions etc.
+	  that will get run). This may not be an issue with the timer
+  	  based scanning we are using currently.
+
 * Find out why we get hangs with rawhide kernels. This only happens with the
   'trace "current"' code. See this mail:
 	

collector.c

@@ -173,12 +173,12 @@ open_fd (Collector *collector,
 {
     int fd;
     
-    fd = open ("/proc/sysprof-trace", O_RDONLY);
+    fd = open (SYSPROF_FILE, O_RDONLY);
     if (fd < 0)
     {
 	load_module();
 	
-	fd = open ("/proc/sysprof-trace", O_RDONLY);
+	fd = open (SYSPROF_FILE, O_RDONLY);
 	
 	if (fd < 0)
 	{

module/sysprof-module.c

@@ -27,6 +27,7 @@
 #include <linux/kernel.h>  /* Needed for KERN_ALERT */
 #include <linux/module.h>  /* Needed by all modules */
 #include <linux/sched.h>
+#include <linux/miscdevice.h>
 
 #include <linux/proc_fs.h>
 #include <asm/uaccess.h>
@@ -166,26 +167,30 @@ timer_notify (struct pt_regs *regs)
 }
 
 static int
-procfile_read(char *buffer, 
-	      char **buffer_location, 
-	      off_t offset, 
-	      int buffer_len,
-	      int *eof,
-	      void *data)
+sysprof_read(struct file *file, char *buffer, size_t count, loff_t *offset)
 {
+	SysprofStackTrace *trace;
+	
+	if (count < sizeof *tail)
+		return -EMSGSIZE;
+
 	if (head == tail)
 		return -EWOULDBLOCK;
 	
-	*buffer_location = (char *)tail;
-	
+	printk(KERN_NOTICE "sysprof: read one trace\n");	
+
+	trace = tail;
 	if (tail++ == &stack_traces[N_TRACES - 1])
 		tail = &stack_traces[0];
 	
-	return sizeof (SysprofStackTrace);
+	if (copy_to_user(buffer, tail, sizeof *tail))
+		return -EFAULT;
+
+	return sizeof *tail;
 }
 
 static unsigned int
-procfile_poll(struct file *filp, poll_table *poll_table)
+sysprof_poll(struct file *filp, poll_table *poll_table)
 {
 	if (head != tail)
 		return POLLIN | POLLRDNORM;
@@ -198,23 +203,47 @@ procfile_poll(struct file *filp, poll_table *poll_table)
 	return 0;
 }
 
-struct proc_dir_entry *trace_proc_file;
+static int
+sysprof_open(struct inode *inode, struct file *file)
+{
+	register_timer_hook (timer_notify);
+	
+	return 0;
+}
+
+static int
+sysprof_release(struct inode *inode, struct file *file)
+{
+	unregister_timer_hook (timer_notify);
+	
+	return 0;
+}
+
+static struct file_operations sysprof_fops = {
+	.owner =	THIS_MODULE,
+        .read =         sysprof_read,
+        .poll =         sysprof_poll,
+        .open =         sysprof_open,
+        .release =      sysprof_release,
+};
+
+static struct miscdevice sysprof_miscdev = {
+	MISC_DYNAMIC_MINOR,
+	"sysprof-trace",
+	&sysprof_fops
+};
 
 int
 init_module(void)
 {
-	trace_proc_file =
-		create_proc_entry ("sysprof-trace", S_IFREG | S_IRUGO, &proc_root);
-	
-	if (!trace_proc_file)
-		return 1;
-	
-	trace_proc_file->read_proc = procfile_read;
-	trace_proc_file->proc_fops->poll = procfile_poll;
-	trace_proc_file->size = sizeof (SysprofStackTrace);
+	int ret;
+
+	ret = misc_register(&sysprof_miscdev);
+	if (ret) {
+		printk(KERN_ERR "sysprof: failed to register misc device\n");
+		return ret;
+	}
 
-	register_timer_hook (timer_notify);
-	
 	printk(KERN_ALERT "sysprof: loaded\n");
 	
 	return 0;
@@ -223,9 +252,7 @@ init_module(void)
 void
 cleanup_module(void)
 {
-	unregister_timer_hook (timer_notify);
-	
-	remove_proc_entry("sysprof-trace", &proc_root);
+	misc_deregister(&sysprof_miscdev);
 
 	printk(KERN_ALERT "sysprof: unloaded\n");
 }

module/sysprof-module.h

@@ -20,6 +20,8 @@
 #ifndef SYSPROF_MODULE_H
 #define SYSPROF_MODULE_H
 
+#define SYSPROF_FILE "/dev/sysprof-trace"
+
 typedef struct SysprofStackTrace SysprofStackTrace;
 
 #define SYSPROF_MAX_ADDRESSES 512

sysprof-text.c

@@ -70,15 +70,13 @@ signal_handler (int signo, gpointer data)
     g_main_loop_quit (app->main_loop);
 }
 
-#define SYSPROF_FILE "/proc/sysprof-trace"
-
 static void
 no_module (void)
 {
     perror (SYSPROF_FILE);
     fprintf (stderr,
 	     "\n"
-	     "Can't open /proc/sysprof-trace. You need to insert "
+	     "Can't open " SYSPROF_FILE ". You need to insert "
 	     "the sysprof kernel module. Run\n"
 	     "\n"
 	     "       modprobe sysprof-module\n"

sysprof.c

@@ -27,6 +27,7 @@
 #include "treeviewutils.h"
 #include "profile.h"
 #include "collector.h"
+#include "module/sysprof-module.h"
 
 /* FIXME - not10 */
 #define _(a) a
@@ -366,7 +367,7 @@ on_start_toggled (GtkWidget *widget, gpointer data)
     {
 	/* FIXME: get the real error message */
 	sorry (app->main_window,
-	       "Can't open /proc/sysprof-trace. You need to insert\n"
+	       "Can't open " SYSPROF_FILE ". You need to insert\n"
 	       "the sysprof kernel module. Run\n"
 	       "\n"
 	       "       modprobe sysprof-module\n"