Messages in this thread | | | Date | Tue, 13 Oct 2015 12:34:52 +0800 | From | "Wangnan (F)" <> | Subject | Re: [RFC PATCH 2/2] bpf: Implement bpf_perf_event_sample_enable/disable() helpers |
| |
On 2015/10/13 12:16, Alexei Starovoitov wrote: > On 10/12/15 8:51 PM, Wangnan (F) wrote: >>> why 'set disable' is needed ? >>> the example given in cover letter shows the use case where you want >>> to receive samples only within sys_write() syscall. >>> The example makes sense, but sys_write() is running on this cpu, so >>> just >>> disabling it on the current one is enough. >>> >> >> Our real use case is control of the system-wide sampling. For example, >> we need sampling all CPUs when smartphone start refershing its display. >> We need all CPUs because in Android system there are plenty of threads >> get involed into this behavior. We can't achieve this by controling >> sampling on only one CPU. This is the reason we need 'set enable' >> and 'set disable'. > > ok, but that use case may have different enable/disable pattern. > In sys_write example ultra-fast enable/disable is must have, since > the whole syscall is fast and overhead should be minimal. > but for display refresh? we're talking milliseconds, no? > Can you just ioctl() it from user space? > If cost of enable/disable is high or the time range between toggling is > long, then doing it from the bpf program doesn't make sense. Instead > the program can do bpf_perf_event_output() to send a notification to > user space that condition is met and the user space can ioctl() events. >
OK. I think I understand your design principle that, everything inside BPF should be as fast as possible.
Make userspace control events using ioctl make things harder. You know that 'perf record' itself doesn't care too much about events it reveived. It only copies data to perf.data, but what we want is to use perf record simply like this:
# perf record -e evt=cycles -e control.o/pmu=evt/ -a sleep 100
And in control.o we create uprobe point to mark the start and finish of a frame:
SEC("target=/a/b/c.o\nstartFrame=0x123456") int startFrame(void *) { bpf_pmu_enable(pmu); return 1; }
SEC("target=/a/b/c.o\nfinishFrame=0x234568") int finishFrame(void *) { bpf_pmu_disable(pmu); return 1; }
I think it is make sence also.
I still think perf is not necessary be independent each other. You know we have PERF_EVENT_IOC_SET_OUTPUT which can set multiple events output through one ringbuffer. This way perf events are connected.
I think the 'set disable/enable' design in this patchset satisify the design goal that in BPF program we only do simple and fast things. The only inconvience is we add something into map, which is ugly. What about using similar implementation like PERF_EVENT_IOC_SET_OUTPUT, creating a new ioctl like PERF_EVENT_IOC_SET_ENABLER, then let perf to select an event as 'enabler', then BPF can still control one atomic variable to enable/disable a set of events.
Thank you.
| |