On 10/12/15 2:02 AM, Kaixu Xia wrote:
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index f57d7fe..25e073d 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -39,6 +39,7 @@ struct bpf_map { u32 max_entries; const struct bpf_map_ops *ops; struct work_struct work; + atomic_t perf_sample_disable; };struct bpf_map_type_list { diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 092a0e8..0606d1d 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -483,6 +483,8 @@ struct perf_event { perf_overflow_handler_t overflow_handler; void *overflow_handler_context; + atomic_t *sample_disable;
this looks fragile and unnecessary. Why add such field to generic bpf_map and carry its pointer into perf_event? Single extra field in perf_event would have been enough. Even better is to avoid adding any fields. There is already event->state why not to use that? The proper perf_event_enable/disable are so heavy that another mechanism needed? cpu_function_call is probably too much to do from bpf program, but that can be simplified? Based on the use case from cover letter, sounds like you want something like soft_disable? Then extending event->state would make the most sense. Also consider the case of re-entrant event enable/disable. So inc/dec of a flag may be needed? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
