From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932312AbbJMD2Q (ORCPT ); Mon, 12 Oct 2015 23:28:16 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:33652 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932115AbbJMD2O (ORCPT ); Mon, 12 Oct 2015 23:28:14 -0400 Message-ID: <561C7A1F.6040702@huawei.com> Date: Tue, 13 Oct 2015 11:27:27 +0800 From: "Wangnan (F)" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Alexei Starovoitov , Kaixu Xia , , , , , , , CC: , , , Subject: Re: [RFC PATCH 2/2] bpf: Implement bpf_perf_event_sample_enable/disable() helpers References: <1444640563-159175-1-git-send-email-xiakaixu@huawei.com> <1444640563-159175-3-git-send-email-xiakaixu@huawei.com> <561C0A1E.2080500@plumgrid.com> In-Reply-To: <561C0A1E.2080500@plumgrid.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.66.109] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/