On Mon, 2009-06-22 at 08:58 -0400, Christoph Hellwig wrote:
> But talking about syscalls the sys_perf_counter_open prototype is
> really ugly - it uses either the pid or cpu argument which is a pretty
> clear indicator it should actually be two sys calls.

Would something like the below be any better?

It would allow us to later add something like PERF_TARGET_SOCKET and
things like that.

(utterly untested)

---
 include/linux/perf_counter.h |   12 ++++++++++++
 include/linux/syscalls.h     |    3 ++-
 kernel/perf_counter.c        |   24 +++++++++++++++++++++++-
 tools/perf/builtin-record.c  |   11 ++++++++++-
 tools/perf/builtin-stat.c    |   12 ++++++++++--
 tools/perf/builtin-top.c     |   11 ++++++++++-
 tools/perf/perf.h            |    7 +++----
 7 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index 5e970c7..f7dd22e 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -189,6 +189,18 @@ struct perf_counter_attr {
        __u64                   __reserved_3;
 };
 
+enum perf_counter_target_ids {
+       PERF_TARGET_PID         = 0,
+       PERF_TARGET_CPU         = 1,
+
+       PERF_TARGET_MAX         /* non-ABI */
+};
+
+struct perf_counter_target {
+       __u32                   id;
+       __u64                   val;
+};
+
 /*
  * Ioctls that can be done on a perf counter fd:
  */
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 80de700..670edad 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -760,5 +760,6 @@ int kernel_execve(const char *filename, char *const argv[], 
char *const envp[]);
 
 asmlinkage long sys_perf_counter_open(
                struct perf_counter_attr __user *attr_uptr,
-               pid_t pid, int cpu, int group_fd, unsigned long flags);
+               struct perf_counter_target __user *target,
+               int group_fd, unsigned long flags);
 #endif
diff --git a/kernel/perf_counter.c b/kernel/perf_counter.c
index fa20a9d..205f0b6 100644
--- a/kernel/perf_counter.c
+++ b/kernel/perf_counter.c
@@ -3969,15 +3969,18 @@ err_size:
  */
 SYSCALL_DEFINE5(perf_counter_open,
                struct perf_counter_attr __user *, attr_uptr,
-               pid_t, pid, int, cpu, int, group_fd, unsigned long, flags)
+               struct perf_counter_target __user *, target_uptr,
+               int, group_fd, unsigned long, flags)
 {
        struct perf_counter *counter, *group_leader;
        struct perf_counter_attr attr;
+       struct perf_counter_target target;
        struct perf_counter_context *ctx;
        struct file *counter_file = NULL;
        struct file *group_file = NULL;
        int fput_needed = 0;
        int fput_needed2 = 0;
+       int pid, cpu;
        int ret;
 
        /* for future expandability... */
@@ -3998,6 +4001,25 @@ SYSCALL_DEFINE5(perf_counter_open,
                        return -EINVAL;
        }
 
+       ret = copy_from_user(&target, target_uptr, sizeof(target));
+       if (ret)
+               return ret;
+
+       switch (target.id) {
+       case PERF_TARGET_PID:
+               pid = target.val;
+               cpu = -1;
+               break;
+
+       case PERF_TARGET_CPU:
+               cpu = target.val;
+               pid = -1;
+               break;
+
+       default:
+               return -EINVAL;
+       }
+
        /*
         * Get the target context (task or percpu):
         */
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4ef78a5..b172d30 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -374,6 +374,7 @@ static struct perf_header_attr *get_header_attr(struct 
perf_counter_attr *a, int
 static void create_counter(int counter, int cpu, pid_t pid)
 {
        struct perf_counter_attr *attr = attrs + counter;
+       struct perf_counter_target target;
        struct perf_header_attr *h_attr;
        int track = !counter; /* only the first counter needs these */
        struct {
@@ -409,8 +410,16 @@ static void create_counter(int counter, int cpu, pid_t pid)
        attr->inherit           = (cpu < 0) && inherit;
        attr->disabled          = 1;
 
+       if (cpu < 0) {
+               target.id = PERF_TARGET_PID;
+               target.val = pid;
+       } else {
+               target.id = PERF_TARGET_CPU;
+               target.val = cpu;
+       }
+
 try_again:
-       fd[nr_cpu][counter] = sys_perf_counter_open(attr, pid, cpu, group_fd, 
0);
+       fd[nr_cpu][counter] = sys_perf_counter_open(attr, &target, group_fd, 0);
 
        if (fd[nr_cpu][counter] < 0) {
                int err = errno;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 27921a8..342e642 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -106,6 +106,7 @@ static u64                  runtime_cycles_noise;
 static void create_perf_stat_counter(int counter, int pid)
 {
        struct perf_counter_attr *attr = attrs + counter;
+       struct perf_counter_target target;
 
        if (scale)
                attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
@@ -114,8 +115,12 @@ static void create_perf_stat_counter(int counter, int pid)
        if (system_wide) {
                unsigned int cpu;
 
+               target.id = PERF_TARGET_CPU;
+
                for (cpu = 0; cpu < nr_cpus; cpu++) {
-                       fd[cpu][counter] = sys_perf_counter_open(attr, -1, cpu, 
-1, 0);
+                       target.val = cpu;
+
+                       fd[cpu][counter] = sys_perf_counter_open(attr, &target, 
-1, 0);
                        if (fd[cpu][counter] < 0 && verbose)
                                fprintf(stderr, ERR_PERF_OPEN, counter,
                                        fd[cpu][counter], strerror(errno));
@@ -125,7 +130,10 @@ static void create_perf_stat_counter(int counter, int pid)
                attr->disabled       = 1;
                attr->enable_on_exec = 1;
 
-               fd[0][counter] = sys_perf_counter_open(attr, pid, -1, -1, 0);
+               target.id = PERF_TARGET_PID;
+               target.val = pid;
+
+               fd[0][counter] = sys_perf_counter_open(attr, &target, -1, 0);
                if (fd[0][counter] < 0 && verbose)
                        fprintf(stderr, ERR_PERF_OPEN, counter,
                                fd[0][counter], strerror(errno));
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 95d5c0a..facc870 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -548,6 +548,7 @@ int group_fd;
 
 static void start_counter(int i, int counter)
 {
+       struct perf_counter_target target;
        struct perf_counter_attr *attr;
        unsigned int cpu;
 
@@ -560,8 +561,16 @@ static void start_counter(int i, int counter)
        attr->sample_type       = PERF_SAMPLE_IP | PERF_SAMPLE_TID;
        attr->freq              = freq;
 
+       if (cpu < 0) {
+               target.id = PERF_TARGET_PID;
+               target.val = target_pid;
+       } else {
+               target.id = PERF_TARGET_CPU;
+               target.val = cpu;
+       }
+
 try_again:
-       fd[i][counter] = sys_perf_counter_open(attr, target_pid, cpu, group_fd, 
0);
+       fd[i][counter] = sys_perf_counter_open(attr, &target, group_fd, 0);
 
        if (fd[i][counter] < 0) {
                int err = errno;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e5148e2..3d663d7 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -85,12 +85,11 @@ static inline unsigned long long rdclock(void)
 
 static inline int
 sys_perf_counter_open(struct perf_counter_attr *attr,
-                     pid_t pid, int cpu, int group_fd,
-                     unsigned long flags)
+                     struct perf_counter_target *target,
+                     int group_fd, unsigned long flags)
 {
        attr->size = sizeof(*attr);
-       return syscall(__NR_perf_counter_open, attr, pid, cpu,
-                      group_fd, flags);
+       return syscall(__NR_perf_counter_open, attr, target, group_fd, flags);
 }
 
 #define MAX_COUNTERS                   256



------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time, 
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize  
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel

Reply via email to