lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <561C6CAE.7080503@huawei.com> Date: Tue, 13 Oct 2015 10:30:06 +0800 From: xiakaixu <xiakaixu@...wei.com> To: Alexei Starovoitov <ast@...mgrid.com> CC: <davem@...emloft.net>, <acme@...nel.org>, <mingo@...hat.com>, <a.p.zijlstra@...llo.nl>, <masami.hiramatsu.pt@...achi.com>, <jolsa@...nel.org>, <daniel@...earbox.net>, <wangnan0@...wei.com>, <linux-kernel@...r.kernel.org>, <pi3orama@....com>, <hekuang@...wei.com>, <netdev@...r.kernel.org> Subject: Re: [RFC PATCH 1/2] perf: Add the flag sample_disable not to output data on samples δΊ 2015/10/13 3:20, Alexei Starovoitov ει: > 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? Thanks for your comments! I've tried perf_event_enable/disable, but there is a warning caused by cpu_function_call. The main reason as follows, int smp_call_function_single(...) { ... WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled() && !oops_in_progress); ... } So I added the extra atomic flag filed in order to avoid this problem. > > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists