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: <561C7A1F.6040702@huawei.com> Date: Tue, 13 Oct 2015 11:27:27 +0800 From: "Wangnan (F)" <wangnan0@...wei.com> To: Alexei Starovoitov <ast@...mgrid.com>, Kaixu Xia <xiakaixu@...wei.com>, <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> CC: <linux-kernel@...r.kernel.org>, <pi3orama@....com>, <hekuang@...wei.com>, <netdev@...r.kernel.org> Subject: Re: [RFC PATCH 2/2] bpf: Implement bpf_perf_event_sample_enable/disable() helpers On 2015/10/13 3:29, Alexei Starovoitov wrote: > On 10/12/15 2:02 AM, Kaixu Xia wrote: >> +extern const struct bpf_func_proto bpf_perf_event_sample_enable_proto; >> +extern const struct bpf_func_proto bpf_perf_event_sample_disable_proto; > > externs are unnecessary. Just make them static. > Also I prefer single helper that takes a flag, so we can extend it > instead of adding func_id for every little operation. > > To avoid conflicts if you touch kernel/bpf/* or bpf.h please always > base your patches of net-next. > > > + atomic_set(&map->perf_sample_disable, 0); > > global flag per map is no go. > events are independent and should be treated as such. > Then how to avoid racing? For example, when one core disabling all events in a map, another core is enabling all of them. This racing may causes sereval perf events in a map dump samples while other events not. To avoid such racing I think some locking must be introduced, then cost is even higher. The reason why we introduce an atomic pointer is because each operation should controls a set of events, not one event, due to the per-cpu manner of perf events. Thank you. > Please squash these two patches, since they're part of one logical > feature. Splitting them like this only makes review harder. > > -- > 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/ -- 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