From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yonghong Song Subject: Re: [PATCH net-next] bpf/tracing: fix kernel/events/core.c compilation error Date: Wed, 13 Dec 2017 09:26:05 -0800 Message-ID: <874e5098-6a7c-0e16-1e62-e177da656dac@fb.com> References: <20171213074252.823255-1-yhs@fb.com> <475c2645-01fc-7f25-da78-7ff180760b97@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Cc: To: Alexei Starovoitov , Daniel Borkmann , , Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:47792 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753477AbdLMR0y (ORCPT ); Wed, 13 Dec 2017 12:26:54 -0500 In-Reply-To: <475c2645-01fc-7f25-da78-7ff180760b97@fb.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 12/13/17 7:50 AM, Alexei Starovoitov wrote: > On 12/13/17 7:44 AM, Daniel Borkmann wrote: >> On 12/13/2017 08:42 AM, Yonghong Song wrote: >>> Commit f371b304f12e ("bpf/tracing: allow user space to >>> query prog array on the same tp") introduced a perf >>> ioctl command to query prog array attached to the >>> same perf tracepoint. The commit introduced a >>> compilation error when either CONFIG_BPF_SYSCALL or >>> CONFIG_EVENT_TRACING is not defined: >>>   kernel/events/core.o: In function `perf_ioctl': >>>   core.c:(.text+0x98c4): undefined reference to >>> `bpf_event_query_prog_array' >>> >>> This patch fixed this error. >>> >>> Fixes: f371b304f12e ("bpf/tracing: allow user space to query prog >>> array on the same tp") >>> Reported-by: Stephen Rothwell >>> Signed-off-by: Yonghong Song >> >> Looking at _perf_ioctl(), we also have perf_event_set_bpf_prog() >> there. It's basically under CONFIG_EVENT_TRACING, which later calls >> perf_event_attach_bpf_prog() which is under CONFIG_BPF_EVENTS, so >> where we have the dummy handler returning -EOPNOTSUPP when BPF >> events is not set. bpf_trace.c is only built when CONFIG_BPF_EVENTS >> is set and that by itself depends on BPF_SYSCALL already. So it would >> be more correct to do the same thing here ... >> >> #if defined(CONFIG_EVENT_TRACING) && defined(CONFIG_BPF_EVENTS) >> [...] > > +1 > #ifdef CONFIG_BPF_EVENTS > works, whereas CONFIG_EVENT_TRACING probably not, since kprobe > can be disabled independently which will turn off BPF_EVENTS > and body of bpf_event_query_prog_array() will be gone. I tested to enable/disable uprobe/kprobe/both and my patch works. But I did not test enable a non uprobe/kprobe tracing event (e.g., CONFIG_FUNCTION_TRACER) where CONFIG_TRACING and CONFIG_EVENT_TRACING is on but CONFIG_UPROBES_EVENT/CONFIG_KPROBES_EVENT is off and then my patch breaks. Looks like #ifdef CONFIG_BPF_EVENTS is suffice. This config will enable to include bpf_trace.c with the real definition. It will depend on KPROBE_EVENTS or UPROBE_EVENTS and either of them will enable CONFIG_TRACING and then CONFIG_EVENT_TRACING. Will resubmit the patch after testing.