Messages in this thread | | | Date | Mon, 4 Jul 2011 19:59:27 +0200 | From | Robert Richter <> | Subject | Re: [RFC] [PATCH] perf: Attaching an event to a specific PMU |
| |
On 03.07.11 14:04:31, Peter Zijlstra wrote: > On Sun, 2011-07-03 at 17:04 +0200, Robert Richter wrote: > > > > this is a prototype implementation for attaching an event to a > > specific PMU. If there is a general acceptance for this approach I > > will create patches for upstream integration and base my current IBS > > patches on it. > > But there's already an infrastructure to do this, simply stick the > contents of /sys/bus/event_source/devices/*/type in > perf_event_attr::type and you're done. > > I don't see the need for /dev crap.
Using sysfs by reading from /sys/bus/event_source/devices/*/type would work too. But there are several reasons why I like my approach.
First of all, it follows the idea of grouping events. Attaching events to a specific pmu is not different from attaching them to a specific event group. It is actually the same if we think of one group for events that may be scheduled on only one pmu. Thus, treating a pmu like a group event is the logical step for intuitive usage of the perf_open syscall. This way we have symmetrical implementations for binding events to groups or pmus.
The syscall interface has already everything needed for this. Following the concept of attaching events to a group with a file descriptor, we simply must create a file descriptor pointing to a pmu device. This works without extending the perf_open syscall interface, no changes to the interface are needed.
Device nodes are the general approach for controlling devices from user-space, they are integral part of the Linux device driver model. With a device file descriptor opened from a device node we can explicitly point to a pmu device.
Representing a device with a device node is common programming practice. Usage of device nodes is not deprecated. There are existing frameworks to easily create such devices. With dynamically device node allocation and udev there are solutions for drawbacks of /dev. Why not having a device node for pmus? What are your concerns using /dev?
The patch introduces a pmu device class and devices are grouped by class now. They can be found in /dev/pmu/* instead of searching for device attributes somewhere in the system hirarchie at /sys/bus/event_source/devices/*/type. A device node is easier to handle.
The implementation only needs about 150 lines of kernel code. It is straight and separated. There is nothing special what makes it hard to read or maintain. The code is using typical kernel device allocation methods. Do you think this patch makes kernel code too complex?
There is much easier user-space code:
pmu = open("/dev/pmu/proto", O_RDONLY); if (pmu == -1) err(1, "pmu not found");
attr.config = 0xf00ba2;
event = sys_perf_event_open(&attr, 0, -1, pmu, 0);
vs.
pmu = open("/sys/bus/event_source/devices/proto/type", O_RDONLY); if (pmu == -1) err(1, "pmu not found"); size = read(pmu, buf, BUFSIZ - 1); if (size < 0) { close(pmu); err(1, "failed to read pmu type"); } buf[size] = '0';
attr.type = atoi(buf); attr.config = 0xf00ba2;
event = sys_perf_event_open(&attr, 0, -1, -1, 0);
There are also additional header includes and variable declarations needed.
(Btw, current kernel code does not support dynamically allocated pmu types due to a check in perf_copy_attr():
if (attr->type >= PERF_TYPE_MAX) return -EINVAL; )
Beside of that, using /sys/ is racy. There is no protection against unregistering the pmu. Probably this might not cause big problems in practice, but it can be done better. With open/close we can protect the pmu from being removed.
Overall, my approach improves the perf design. It adds a better and more intuitve access to perf from user space with clear and common methods and interfaces. Please let me know the concerns you have.
Thanks,
-Robert
-- Advanced Micro Devices, Inc. Operating System Research Center
| |