From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967952AbdEWQbw (ORCPT ); Tue, 23 May 2017 12:31:52 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:40826 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936012AbdEWQbs (ORCPT ); Tue, 23 May 2017 12:31:48 -0400 Date: Tue, 23 May 2017 18:31:34 +0200 From: Peter Zijlstra To: Alexei Starovoitov Cc: "David S . Miller" , Brendan Gregg , Daniel Borkmann , Teng Qin , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH net-next 1/2] perf, bpf: add support for HW_CACHE and RAW events Message-ID: <20170523163134.saalqlizp5opc5tz@hirez.programming.kicks-ass.net> References: <20170522224840.810121-1-ast@fb.com> <20170522224840.810121-2-ast@fb.com> <20170523074247.esnr4fj2squn4w2d@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 23, 2017 at 07:38:08AM -0700, Alexei Starovoitov wrote: > On 5/23/17 12:42 AM, Peter Zijlstra wrote: > > On Mon, May 22, 2017 at 03:48:39PM -0700, Alexei Starovoitov wrote: > > > From: Teng Qin > > > > > > This commit adds support for attach BPF program to RAW and HW_CACHE type > > > events, and support for read HW_CACHE type event counters in BPF > > > program. Existing code logic already supports them, so this commit is > > > just update Enum value checks. > > > > So what I'm missing is why they were not supported previously, and what > > changed to allow it now. > > that code path simply wasn't tested previously. Nothing changed on > bpf side and on perf side. > Why it wasn't added on day one? There was no demand. Now people > use bpf more and more and few folks got confused that these types > of perf events were not supported, hence we're adding it. OK. Is there anything stopping people from wanting to use the dynamic types, as found in: /sys/bus/event_source/devices/*/type ? In which case, do we want something like this instead? diff --git a/kernel/events/core.c b/kernel/events/core.c index 971f7259108f..4aa5f3011cf8 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -8063,12 +8063,8 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd) bool is_kprobe, is_tracepoint; struct bpf_prog *prog; - if (event->attr.type == PERF_TYPE_HARDWARE || - event->attr.type == PERF_TYPE_SOFTWARE) - return perf_event_set_bpf_handler(event, prog_fd); - if (event->attr.type != PERF_TYPE_TRACEPOINT) - return -EINVAL; + return perf_event_set_bpf_handler(event, prog_fd); if (event->tp_event->prog) return -EEXIST;