[PATCH] small cleanups + fixes for perfmon.c

[John Levon <levon@movementarian.org>]

The following patch, compile-tested only so far, does the following :

o removes the [UN]LOCK_BUF_FMT_LIST defines - there is no abstraction
  benefit to them, and they just look ugly
o fixes two KERN_ERR that should be KERN_INFO
o removes the "nolock" parameter to the find function (was unused)
o homogenises the names related to buffer formats
o removes the hand-crafted list in favour of standard linux list methods
o reduces code duplication

OK ?

regards
john

Index: linux-ia64/arch/ia64/kernel/perfmon.c
=================================RCS file: /home/cvs/linux-2.5/arch/ia64/kernel/perfmon.c,v
retrieving revision 1.35
diff -u -a -p -r1.35 perfmon.c
--- linux-ia64/arch/ia64/kernel/perfmon.c	19 Sep 2003 21:01:14 -0000	1.35
+++ linux-ia64/arch/ia64/kernel/perfmon.c	8 Oct 2003 12:21:04 -0000
@@ -513,10 +513,8 @@ static pfm_session_t		pfm_sessions;	/* g
 static struct proc_dir_entry 	*perfmon_dir;
 static pfm_uuid_t		pfm_null_uuid = {0,};
 
-static spinlock_t		pfm_smpl_fmt_lock;
-static pfm_buffer_fmt_t		*pfm_buffer_fmt_list;
-#define LOCK_BUF_FMT_LIST()	    spin_lock(&pfm_smpl_fmt_lock)
-#define UNLOCK_BUF_FMT_LIST()	    spin_unlock(&pfm_smpl_fmt_lock)
+static spinlock_t		pfm_buffer_fmt_lock;
+static LIST_HEAD(pfm_buffer_fmt_list);
 
 /* sysctl() controls */
 static pfm_sysctl_t pfm_sysctl;
@@ -1206,12 +1204,36 @@ pfm_buf_fmt_restart_active(pfm_buffer_fm
 	return ret;
 }
 
+static pfm_buffer_fmt_t *
+__pfm_find_buffer_fmt(pfm_uuid_t uuid)
+{
+	struct list_head * pos;
+	pfm_buffer_fmt_t * entry;
+
+	list_for_each(pos, &pfm_buffer_fmt_list) {
+		entry = list_entry(pos, pfm_buffer_fmt_t, fmt_list);
+		if (pfm_uuid_cmp(uuid, entry->fmt_uuid) = 0)
+			return entry;
+	}
+	return NULL;
+}
 
+/*
+ * find a buffer format based on its uuid
+ */
+static pfm_buffer_fmt_t *
+pfm_find_buffer_fmt(pfm_uuid_t uuid)
+{
+	pfm_buffer_fmt_t * fmt;
+	spin_lock(&pfm_buffer_fmt_lock);
+	fmt = __pfm_find_buffer_fmt(uuid);
+	spin_unlock(&pfm_buffer_fmt_lock);
+	return fmt;
+}
 
 int
 pfm_register_buffer_fmt(pfm_buffer_fmt_t *fmt)
 {
-	pfm_buffer_fmt_t *p;
 	int ret = 0;
 
 	/* some sanity checks */
@@ -1224,78 +1246,43 @@ pfm_register_buffer_fmt(pfm_buffer_fmt_t
 	 * XXX: need check validity of fmt_arg_size
 	 */
 
-	LOCK_BUF_FMT_LIST();
-	p = pfm_buffer_fmt_list;
-
-
-	while (p) {
-		if (pfm_uuid_cmp(fmt->fmt_uuid, p->fmt_uuid) = 0) break;
-		p = p->fmt_next;
-	}
+	spin_lock(&pfm_buffer_fmt_lock);
 
-	if (p) {
+	if (__pfm_find_buffer_fmt(fmt->fmt_uuid)) {
 		printk(KERN_ERR "perfmon: duplicate sampling format: %s\n", fmt->fmt_name);
 		ret = -EBUSY;
-	} else {
-		fmt->fmt_prev = NULL;
-		fmt->fmt_next = pfm_buffer_fmt_list;
-		pfm_buffer_fmt_list = fmt;
-		printk(KERN_ERR "perfmon: added sampling format %s\n", fmt->fmt_name);
+		goto out;
 	}
-	UNLOCK_BUF_FMT_LIST();
 
+	list_add(&fmt->fmt_list, &pfm_buffer_fmt_list);
+	printk(KERN_INFO "perfmon: added sampling format %s\n", fmt->fmt_name);
+
+out:
+	spin_unlock(&pfm_buffer_fmt_lock);
 	return ret;
 }
 
 int
 pfm_unregister_buffer_fmt(pfm_uuid_t uuid)
 {
-	pfm_buffer_fmt_t *p;
+	pfm_buffer_fmt_t *fmt;
 	int ret = 0;
 
-	LOCK_BUF_FMT_LIST();
-	p = pfm_buffer_fmt_list;
-	while (p) {
-		if (memcmp(uuid, p->fmt_uuid, sizeof(pfm_uuid_t)) = 0) break;
-		p = p->fmt_next;
-	}
-	if (p) {
-		if (p->fmt_prev)
-			p->fmt_prev->fmt_next = p->fmt_next;
-		else
-			pfm_buffer_fmt_list = p->fmt_next;
-
-		if (p->fmt_next)
-			p->fmt_next->fmt_prev = p->fmt_prev;
+	spin_lock(&pfm_buffer_fmt_lock);
 
-		printk(KERN_ERR "perfmon: removed sampling format: %s\n",  p->fmt_name);
-		p->fmt_next = p->fmt_prev = NULL;
-	} else {
+	fmt = __pfm_find_buffer_fmt(uuid);
+	if (!fmt) {
 		printk(KERN_ERR "perfmon: cannot unregister format, not found\n");
 		ret = -EINVAL;
+		goto out;
 	}
-	UNLOCK_BUF_FMT_LIST();
 
-	return ret;
+	list_del_init(&fmt->fmt_list);
+	printk(KERN_INFO "perfmon: removed sampling format: %s\n", fmt->fmt_name);
 
-}
-
-/*
- * find a buffer format based on its uuid
- */
-static pfm_buffer_fmt_t *
-pfm_find_buffer_fmt(pfm_uuid_t uuid, int nolock)
-{
-	pfm_buffer_fmt_t *p;
-
-	LOCK_BUF_FMT_LIST();
-	for (p = pfm_buffer_fmt_list; p ; p = p->fmt_next) {
-		if (pfm_uuid_cmp(uuid, p->fmt_uuid) = 0) break;
-	}
-
-	UNLOCK_BUF_FMT_LIST();
-
-	return p;
+out:
+	spin_unlock(&pfm_buffer_fmt_lock);
+	return ret;
 }
 
 static int
@@ -2419,8 +2406,7 @@ pfm_setup_buffer_fmt(struct task_struct 
 	int ret = 0;
 #define PFM_CTXARG_BUF_ARG(a)	(pfm_buffer_fmt_t *)(a+1)
 
-	/* invoke and lock buffer format, if found */
-	fmt = pfm_find_buffer_fmt(arg->ctx_smpl_buf_id, 0);
+	fmt = pfm_find_buffer_fmt(arg->ctx_smpl_buf_id);
 	if (fmt = NULL) {
 		DPRINT(("[%d] cannot find buffer format\n", task->pid));
 		return -EINVAL;
@@ -2528,8 +2514,7 @@ pfm_ctx_getsize(void *arg, size_t *sz)
 
 	if (!pfm_uuid_cmp(req->ctx_smpl_buf_id, pfm_null_uuid)) return 0;
 
-	/* no buffer locking here, will be called again */
-	fmt = pfm_find_buffer_fmt(req->ctx_smpl_buf_id, 1);
+	fmt = pfm_find_buffer_fmt(req->ctx_smpl_buf_id);
 	if (fmt = NULL) {
 		DPRINT(("cannot find buffer format\n"));
 		return -EINVAL;
@@ -5445,7 +5430,8 @@ static int
 pfm_proc_info(char *page)
 {
 	char *p = page;
-	pfm_buffer_fmt_t *b;
+	struct list_head * pos;
+	pfm_buffer_fmt_t * entry;
 	unsigned long psr;
 	int i;
 
@@ -5495,29 +5481,30 @@ pfm_proc_info(char *page)
 			pfm_sessions.pfs_ptrace_use_dbregs);
 	UNLOCK_PFS();
 
-	LOCK_BUF_FMT_LIST();
+	spin_lock(&pfm_buffer_fmt_lock);
 
-	for (b = pfm_buffer_fmt_list; b ; b = b->fmt_next) {
+	list_for_each(pos, &pfm_buffer_fmt_list) {
+		entry = list_entry(pos, pfm_buffer_fmt_t, fmt_list);
 		p += sprintf(p, "format                    : %02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x %s\n",
-				b->fmt_uuid[0],
-				b->fmt_uuid[1],
-				b->fmt_uuid[2],
-				b->fmt_uuid[3],
-				b->fmt_uuid[4],
-				b->fmt_uuid[5],
-				b->fmt_uuid[6],
-				b->fmt_uuid[7],
-				b->fmt_uuid[8],
-				b->fmt_uuid[9],
-				b->fmt_uuid[10],
-				b->fmt_uuid[11],
-				b->fmt_uuid[12],
-				b->fmt_uuid[13],
-				b->fmt_uuid[14],
-				b->fmt_uuid[15],
-				b->fmt_name);
+				entry->fmt_uuid[0],
+				entry->fmt_uuid[1],
+				entry->fmt_uuid[2],
+				entry->fmt_uuid[3],
+				entry->fmt_uuid[4],
+				entry->fmt_uuid[5],
+				entry->fmt_uuid[6],
+				entry->fmt_uuid[7],
+				entry->fmt_uuid[8],
+				entry->fmt_uuid[9],
+				entry->fmt_uuid[10],
+				entry->fmt_uuid[11],
+				entry->fmt_uuid[12],
+				entry->fmt_uuid[13],
+				entry->fmt_uuid[14],
+				entry->fmt_uuid[15],
+				entry->fmt_name);
 	}
-	UNLOCK_BUF_FMT_LIST();
+	spin_unlock(&pfm_buffer_fmt_lock);
 
 	return p - page;
 }
@@ -6338,7 +6325,7 @@ pfm_init(void)
 	 * initialize all our spinlocks
 	 */
 	spin_lock_init(&pfm_sessions.pfs_lock);
-	spin_lock_init(&pfm_smpl_fmt_lock);
+	spin_lock_init(&pfm_buffer_fmt_lock);
 
 	init_pfm_fs();
 
Index: linux-ia64/include/asm-ia64//perfmon.h
=================================RCS file: /home/cvs/linux-2.5/include/asm-ia64/perfmon.h,v
retrieving revision 1.13
diff -u -a -p -r1.13 perfmon.h
--- linux-ia64/include/asm-ia64//perfmon.h	22 Aug 2003 00:40:41 -0000	1.13
+++ linux-ia64/include/asm-ia64//perfmon.h	8 Oct 2003 12:21:53 -0000
@@ -223,7 +223,7 @@ typedef struct {
 } pfm_ovfl_arg_t;
 
 
-typedef struct _pfm_buffer_fmt_t {
+typedef struct {
 	char		*fmt_name;
 	pfm_uuid_t	fmt_uuid;
 	size_t		fmt_arg_size;
@@ -237,8 +237,7 @@ typedef struct _pfm_buffer_fmt_t {
 	int		(*fmt_restart_active)(struct task_struct *task, pfm_ovfl_ctrl_t *ctrl, void *buf, struct pt_regs *regs);
 	int		(*fmt_exit)(struct task_struct *task, void *buf, struct pt_regs *regs);
 
-	struct _pfm_buffer_fmt_t *fmt_next;
-	struct _pfm_buffer_fmt_t *fmt_prev;
+	struct list_head fmt_list;
 } pfm_buffer_fmt_t;
 
 extern int pfm_register_buffer_fmt(pfm_buffer_fmt_t *fmt);