Li, Thanks for your comment. Good catch on perf_detach_cgroup(). I will resubmit this patch (part 4/5).
On Wed, Jan 5, 2011 at 4:13 AM, Li Zefan <l...@cn.fujitsu.com> wrote: > Stephane Eranian wrote: >> This kernel patch adds the ability to filter monitoring based on >> container groups (cgroups). This is for use in per-cpu mode only. >> >> The cgroup to monitor is passed as a file descriptor in the pid >> argument to the syscall. The file descriptor must be opened to >> the cgroup name in the cgroup filesystem. For instance, if the >> cgroup name is foo and cgroupfs is mounted in /cgroup, then the >> file descriptor is opened to /cgroup/foo. Cgroup mode is >> activated by passing PERF_FLAG_PID_CGROUP in the flags argument >> to the syscall. >> >> For instance to measure in cgroup foo on CPU1 assuming >> cgroupfs is mounted under /cgroup: >> >> struct perf_event_attr attr; >> int cgroup_fd, fd; >> >> cgroup_fd = open("/cgroup/foo", O_RDONLY); >> fd = perf_event_open(&attr, cgroup_fd, 1, -1, PERF_FLAG_PID_CGROUP); >> close(cgroup_fd); >> >> Signed-off-by: Stephane Eranian <eran...@google.com> > > For the cgroup part: > > Acked-by: Li Zefan <l...@cn.fujitsu.com> > > a few comments below: > >> > ... >> +config CGROUP_PERF >> + bool "Enable perf_event per-cpu per-container group (cgroup) >> monitoring" >> + depends on PERF_EVENTS && CGROUPS > > Depending on CGROUPS is already implicated by "if CGROUPS" > >> + help >> + This option extends the per-cpu mode to restrict monitoring to >> + threads which belong to the cgroup specificied and run on the >> + designated cpu. >> + >> + Say N if unsure. >> + >> menuconfig CGROUP_SCHED >> bool "Group CPU scheduler" >> depends on EXPERIMENTAL > ... >> +static inline int perf_cgroup_connect(int fd, struct perf_event *event, >> + struct perf_event_attr *attr, >> + struct perf_event *group_leader) >> +{ >> + struct perf_cgroup *cgrp; >> + struct cgroup_subsys_state *css; >> + struct file *file; >> + int ret = 0, fput_needed; >> + >> + file = fget_light(fd, &fput_needed); >> + if (!file) >> + return -EBADF; >> + >> + css = cgroup_css_from_dir(file, perf_subsys_id); >> + if (IS_ERR(css)) >> + return PTR_ERR(css); >> + >> + cgrp = container_of(css, struct perf_cgroup, css); >> + event->cgrp = cgrp; >> + >> + /* >> + * all events in a group must monitor >> + * the same cgroup because a thread belongs >> + * to only one perf cgroup at a time >> + */ >> + if (group_leader && group_leader->cgrp != cgrp) { >> + perf_detach_cgroup(event); > > This doesn't seem right, because we haven't got the cgroup > refcount. > >> + ret = -EINVAL; >> + } else { >> + /* must be done before we fput() the file */ >> + perf_get_cgroup(event); >> + } >> + fput_light(file, fput_needed); >> + return ret; >> +} > ------------------------------------------------------------------------------ Learn how Oracle Real Application Clusters (RAC) One Node allows customers to consolidate database storage, standardize their database environment, and, should the need arise, upgrade to a full multi-node Oracle RAC database without downtime or disruption http://p.sf.net/sfu/oracle-sfdevnl _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel