yet more trivial perfmon cleanups

PATCH New John Levon 1 participant 1 message Oct 23, 2003

Share & tools

John Levon

author

2003-10-23 00:27 UTC #0 lore raw

PATCH 1/1 yet more trivial perfmon cleanups

All the wrappers tend to make the code hard to follow IMHO. This also fixes a slight debug printk error, removes an always-NULL argument to fmt_exit, and makes it return void to reflect usage.

 3 files changed, 17 insertions(+), 48 deletions(-)

I'm not really sure what use passing pt_regs into the format's exit routine would ever be ? It's only done on create failure path.

There's a lot more of these trivial things that would make perfmon more readable IMHO, but I suppose now is not the right time to be doing stuff, so I'll stop here.

Just one last thing - why :

303 unsigned long ctx_used_monitors[4]; /* bitmask of monitor PMC being used */

and friends ? If the intention was to support more than 64 PMCs at some point, all the code using these things need to use set_bit() instead of just C, unless I'm missing something (i.e. [1]-[3] in these are never touched or read).

regards,
john

Index: linux-cvs/arch/ia64/kernel/perfmon.c =================================RCS file: /home/cvs/linux-2.5/arch/ia64/kernel/perfmon.c,v retrieving revision 1.37

arch/ia64/kernel/perfmon.c +13 ~~-43~~

diff -u -a -p -r1.37 perfmon.c
@@ -1150,41 +1150,6 @@ pfm_uuid_cmp(pfm_uuid_t a, pfm_uuid_t b)
 }
 
 static inline int
-pfm_buf_fmt_exit(pfm_buffer_fmt_t *fmt, struct task_struct *task, void *buf, struct pt_regs *regs)
-{
-	int ret = 0;
-	if (fmt->fmt_exit) ret = (*fmt->fmt_exit)(task, buf, regs);
-	return ret;
-}
-
-static inline int
-pfm_buf_fmt_getsize(pfm_buffer_fmt_t *fmt, struct task_struct *task, unsigned int flags, int cpu, void *arg, unsigned long *size)
-{
-	int ret = 0;
-	if (fmt->fmt_getsize) ret = (*fmt->fmt_getsize)(task, flags, cpu, arg, size);
-	return ret;
-}
-
-
-static inline int
-pfm_buf_fmt_validate(pfm_buffer_fmt_t *fmt, struct task_struct *task, unsigned int flags,
-		     int cpu, void *arg)
-{
-	int ret = 0;
-	if (fmt->fmt_validate) ret = (*fmt->fmt_validate)(task, flags, cpu, arg);
-	return ret;
-}
-
-static inline int
-pfm_buf_fmt_init(pfm_buffer_fmt_t *fmt, struct task_struct *task, void *buf, unsigned int flags,
-		     int cpu, void *arg)
-{
-	int ret = 0;
-	if (fmt->fmt_init) ret = (*fmt->fmt_init)(task, buf, flags, cpu, arg);
-	return ret;
-}
-
-static inline int
 pfm_buf_fmt_restart(pfm_buffer_fmt_t *fmt, struct task_struct *task, pfm_ovfl_ctrl_t *ctrl, void *buf, struct pt_regs *regs)
 {
 	int ret = 0;

@@ -1443,7 +1408,8 @@ pfm_free_smpl_buffer(pfm_context_t *ctx)
 		ctx->ctx_smpl_size,
 		ctx->ctx_smpl_vaddr));
 
-	pfm_buf_fmt_exit(fmt, current, NULL, NULL);
+	if (fmt->fmt_exit)
+		fmt->fmt_exit(current, NULL);
 
 	/*
 	 * free the buffer

@@ -1466,7 +1432,8 @@ pfm_exit_smpl_buffer(pfm_buffer_fmt_t *f
 {
 	if (fmt = NULL) return;
 
-	pfm_buf_fmt_exit(fmt, current, NULL, NULL);
+	if (fmt->fmt_exit)
+		fmt->fmt_exit(current, NULL);
 
 }

@@ -1516,7 +1483,7 @@ pfm_read(struct file *filp, char *buf, s
 	unsigned long flags;
   	DECLARE_WAITQUEUE(wait, current);
 	if (PFM_IS_FILE(filp) = 0) {
-		printk(KERN_ERR "perfmon: pfm_poll: bad magic [%d]\n", current->pid);
+		printk(KERN_ERR "perfmon: pfm_read: bad magic [%d]\n", current->pid);
 		return -EINVAL;
 	}

@@ -2413,7 +2380,8 @@ pfm_setup_buffer_fmt(struct task_struct 
 	 */
 	if (fmt->fmt_arg_size) fmt_arg = PFM_CTXARG_BUF_ARG(arg);
 
-	ret = pfm_buf_fmt_validate(fmt, task, ctx_flags, cpu, fmt_arg);
+	if (fmt->fmt_validate)
+		ret = fmt->fmt_validate(task, ctx_flags, cpu, fmt_arg);
 
 	DPRINT(("[%d] after validate(0x%x,%d,%p)=%d\n", task->pid, ctx_flags, cpu, fmt_arg, ret));

@@ -2425,7 +2393,8 @@ pfm_setup_buffer_fmt(struct task_struct 
 	/*
 	 * check if buffer format wants to use perfmon buffer allocation/mapping service
 	 */
-	ret = pfm_buf_fmt_getsize(fmt, task, ctx_flags, cpu, fmt_arg, &size);
+	if (fmt->fmt_getsize)
+		ret = fmt->fmt_getsize(task, ctx_flags, cpu, fmt_arg, &size);
 	if (ret) goto error;
 
 	if (size) {

@@ -2438,7 +2407,8 @@ pfm_setup_buffer_fmt(struct task_struct 
 		/* keep track of user address of buffer */
 		arg->ctx_smpl_vaddr = uaddr;
 	}
-	ret = pfm_buf_fmt_init(fmt, task, ctx->ctx_smpl_hdr, ctx_flags, cpu, fmt_arg);
+	if (fmt->fmt_init)
+		ret = fmt->fmt_init(task, ctx->ctx_smpl_hdr, ctx_flags, cpu, fmt_arg);
 
 error:
 	return ret;

@@ -2704,8 +2674,8 @@ pfm_context_create(pfm_context_t *ctx, v
 buffer_error:
 	pfm_free_fd(ctx->ctx_fd, filp);
 
-	if (ctx->ctx_buf_fmt) {
-		pfm_buf_fmt_exit(ctx->ctx_buf_fmt, current, NULL, regs);
+	if (ctx->ctx_buf_fmt && ctx->ctx_buf_fmt->fmt_exit) {
+		ctx->ctx_buf_fmt->fmt_exit(current, regs);
 	}
 error_file:
 	pfm_context_free(ctx);

Index: linux-cvs/arch/ia64/kernel/perfmon_default_smpl.c =================================RCS file: /home/cvs/linux-2.5/arch/ia64/kernel/perfmon_default_smpl.c,v retrieving revision 1.3

arch/ia64/kernel/perfmon_default_smpl.c +3 -4

diff -u -a -p -r1.3 perfmon_default_smpl.c
@@ -245,11 +245,10 @@ default_restart(struct task_struct *task
 	return 0;
 }
 
-static int
-default_exit(struct task_struct *task, void *buf, struct pt_regs *regs)
+static void
+default_exit(struct task_struct *task, struct pt_regs *regs)
 {
-	DPRINT(("[%d] exit(%p)\n", task->pid, buf));
-	return 0;
+	DPRINT(("[%d] exit\n", task->pid));
 }
 
 static pfm_buffer_fmt_t default_fmt={

Index: linux-cvs/include/asm//perfmon.h
=================================RCS file: /home/cvs/linux-2.5/include/asm-ia64/perfmon.h,v retrieving revision 1.14

include/asm//perfmon.h +1 -1

diff -u -a -p -r1.14 perfmon.h
@@ -231,7 +231,7 @@ typedef struct {
 	int		(*fmt_handler)(struct task_struct *task, void *buf, pfm_ovfl_arg_t *arg, struct pt_regs *regs, unsigned long stamp);
 	int		(*fmt_restart)(struct task_struct *task, pfm_ovfl_ctrl_t *ctrl, void *buf, struct pt_regs *regs);
 	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);
+	void		(*fmt_exit)(struct task_struct *task, struct pt_regs *regs);
 
 	struct list_head fmt_list;
 } pfm_buffer_fmt_t;