Message ID | 20231211045543.31741-2-khuey@kylehuey.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp6834769vqy; Sun, 10 Dec 2023 20:56:06 -0800 (PST) X-Google-Smtp-Source: AGHT+IG51fz7r3gIdQcvK2vSeWnpjXfbVOyehmWdg4mGH1PQJVktiuBiZPF4SDka6FYMPOuyY0O0 X-Received: by 2002:a05:6358:d58b:b0:170:30db:1188 with SMTP id ms11-20020a056358d58b00b0017030db1188mr2513094rwb.24.1702270565936; Sun, 10 Dec 2023 20:56:05 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702270565; cv=none; d=google.com; s=arc-20160816; b=YW7bqyYD2Al+UOSFE/lenxf3Juawk/JxV84U5dpDEEgyF4WxRFX4csYDTcU+BPgn/U GXn0Coy4zktLKMfMhbqxtumHvYJZ2B4OZ64ibpDf79A1irhMrVjFY+oBdy8X9dl/DbAZ Ql0GRlTt9S7IbY0rw3Gf3uvbYHFnGaQn+PpBjGn1VkP+pfVuMhNNNMH9Bpawz12DlMnN x4o4x1htZiMiEFWdguKhtk6yWQLpjpr44bohpsBIpX1Ok36WYrHMAR74hRMSjYiIrysZ oyfrds6R0P1fw6YAK+lirQzuG7eevWNG9xABJrJRVficJTrztLTeY7ZpwksmnEu9sC7T VGYw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=4LaOcUMKpaO4TscNM+bwybPG9qx56qSeDWtKT8IysKk=; fh=TgNdCu0nkwWF6I0gv1FaAZU9nxNhZAbxhuG8wY4k0B8=; b=GOgsOYT/mme45Hk6wuQIbIkiMIsb6E+z5rVbttOz+rN8sQgOmjEstxbz1CKxf8obG8 WxSx2x0xsEbXcRzXupj3fSSq3z1OeoKuKcice5AsBZRaXxc2v3jvm6S8ZR49VYw3GH3F p/H/JY37DKJdruUobEe/Z57kOfGROlKefkGLZ4JZ/S325KwzyG2qph7YdKPvCk7ZQRnq mIGmQC9dkOgbYKJCgoB48n6ZMeWwqyCIMEKrNYnaemlKEBYAjTPAZJHaLOv04jQSfKam HMUf6wezAG9ImJPHimoA8uK2h7ib7PQx6NPFGOeHxmKjyzLvRMTA8tMfHX+gmTqm3l+j 8u5w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kylehuey.com header.s=google header.b=M3R04DG+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id o11-20020a17090ab88b00b002868f5b2c7csi5020722pjr.33.2023.12.10.20.56.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Dec 2023 20:56:05 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; dkim=pass header.i=@kylehuey.com header.s=google header.b=M3R04DG+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 5DF7780677DA; Sun, 10 Dec 2023 20:56:03 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229570AbjLKEzv (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Sun, 10 Dec 2023 23:55:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53240 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232982AbjLKEzs (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 10 Dec 2023 23:55:48 -0500 Received: from mail-yw1-x1135.google.com (mail-yw1-x1135.google.com [IPv6:2607:f8b0:4864:20::1135]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00C65ED for <linux-kernel@vger.kernel.org>; Sun, 10 Dec 2023 20:55:54 -0800 (PST) Received: by mail-yw1-x1135.google.com with SMTP id 00721157ae682-5d33574f64eso39483907b3.3 for <linux-kernel@vger.kernel.org>; Sun, 10 Dec 2023 20:55:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kylehuey.com; s=google; t=1702270554; x=1702875354; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=4LaOcUMKpaO4TscNM+bwybPG9qx56qSeDWtKT8IysKk=; b=M3R04DG+k5gvzaElC3f8cfdbgyW2SN0/jlAMkC9xioLyoYthHoXeq0o/70HRyD/hC9 mMCNPvjyAeIofw0hVH+aEMqMNDjLManpbBClzlJrbP8KwIlvVyFLGWn7HMfeIWYd79mQ Ir6P+rdfe1ruTQHr1XrqVjcTEq199D/RXum+K+DYNL95xxEGM9sj+bBdtFI5VeUI1gJa obiMRMwdcYuAFLc34LMY7KE4HQqOQdZEcwbiqtOnJGLpfL1/9n8ESaiZ7dIlxKNgFzZD tDvxBLU/cssGK6CxDnS64XwHRGS0J4yn4K3aSxVJ8a8o1+S0jZ0oFvVV/wBZ6I9r8zOH Pviw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702270554; x=1702875354; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4LaOcUMKpaO4TscNM+bwybPG9qx56qSeDWtKT8IysKk=; b=WHgpd5eNUs7KL1BbCbzfi5lU1p+QSqpqKed0G/bbRf888lFjn555ERyXqR4MDRzIGo IRTn+obJY/MeDGRW6p4lZmw0RubCxYGc2JrGRr09itlbJ12AK3WWby4pBH5pdtWI2UL+ L8aAGQAuHfxc0b23jEfk5hNWNbRaVTJ/6kTOxEbokN7AGCH0W7sY0LrfiweKkAUOHkzX vf3iy7iqaC7j/9qgZe81ljh441GigkxStTuublPpvfBJ80Y8wllWcjmrHSYnamLXQomt QD4WFzPm3C7gLWkm8D2O1kDYwbQxxk9JNX7kwcfmwvZ+fmEUBQwoWi5C+9/qpk4rWtI9 t/uQ== X-Gm-Message-State: AOJu0Yxt2vXt+RbcW6QGSB2ZCLQgvtDAA5QuZ7471XRqmNG6eQlGoW2W x5U6kbBuBBF57g8fNlV3V4ADkQ== X-Received: by 2002:a0d:d5cd:0:b0:5d7:1940:b36c with SMTP id x196-20020a0dd5cd000000b005d71940b36cmr3212062ywd.56.1702270554159; Sun, 10 Dec 2023 20:55:54 -0800 (PST) Received: from zhadum.home.kylehuey.com (c-76-126-33-191.hsd1.ca.comcast.net. [76.126.33.191]) by smtp.gmail.com with ESMTPSA id e11-20020a170902b78b00b001d2ffeac9d3sm3300623pls.186.2023.12.10.20.55.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Dec 2023 20:55:53 -0800 (PST) From: Kyle Huey <me@kylehuey.com> X-Google-Original-From: Kyle Huey <khuey@kylehuey.com> To: Kyle Huey <khuey@kylehuey.com>, linux-kernel@vger.kernel.org, Andrii Nakryiko <andrii.nakryiko@gmail.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Marco Elver <elver@google.com>, Yonghong Song <yonghong.song@linux.dev> Cc: Robert O'Callahan <robert@ocallahan.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, linux-perf-users@vger.kernel.org, bpf@vger.kernel.org Subject: [PATCH v3 1/4] perf/bpf: Call bpf handler directly, not through overflow machinery Date: Sun, 10 Dec 2023 20:55:40 -0800 Message-Id: <20231211045543.31741-2-khuey@kylehuey.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231211045543.31741-1-khuey@kylehuey.com> References: <20231211045543.31741-1-khuey@kylehuey.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Sun, 10 Dec 2023 20:56:03 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784960061221884284 X-GMAIL-MSGID: 1784960061221884284 |
Series |
Combine perf and bpf for fast eval of hw breakpoint conditions
|
|
Commit Message
Kyle Huey
Dec. 11, 2023, 4:55 a.m. UTC
To ultimately allow bpf programs attached to perf events to completely
suppress all of the effects of a perf event overflow (rather than just the
sample output, as they do today), call bpf_overflow_handler() from
__perf_event_overflow() directly rather than modifying struct perf_event's
overflow_handler. Return the bpf program's return value from
bpf_overflow_handler() so that __perf_event_overflow() knows how to
proceed. Remove the now unnecessary orig_overflow_handler from struct
perf_event.
This patch is solely a refactoring and results in no behavior change.
Signed-off-by: Kyle Huey <khuey@kylehuey.com>
Suggested-by: Namhyung Kim <namhyung@kernel.org>
---
include/linux/perf_event.h | 6 +-----
kernel/events/core.c | 28 +++++++++++++++-------------
2 files changed, 16 insertions(+), 18 deletions(-)
Comments
On Mon, 11 Dec 2023 at 05:55, Kyle Huey <me@kylehuey.com> wrote: > > To ultimately allow bpf programs attached to perf events to completely > suppress all of the effects of a perf event overflow (rather than just the > sample output, as they do today), call bpf_overflow_handler() from > __perf_event_overflow() directly rather than modifying struct perf_event's > overflow_handler. Return the bpf program's return value from > bpf_overflow_handler() so that __perf_event_overflow() knows how to > proceed. Remove the now unnecessary orig_overflow_handler from struct > perf_event. > > This patch is solely a refactoring and results in no behavior change. > > Signed-off-by: Kyle Huey <khuey@kylehuey.com> > Suggested-by: Namhyung Kim <namhyung@kernel.org> > --- > include/linux/perf_event.h | 6 +----- > kernel/events/core.c | 28 +++++++++++++++------------- > 2 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 5547ba68e6e4..312b9f31442c 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -810,7 +810,6 @@ struct perf_event { > perf_overflow_handler_t overflow_handler; > void *overflow_handler_context; > #ifdef CONFIG_BPF_SYSCALL > - perf_overflow_handler_t orig_overflow_handler; > struct bpf_prog *prog; > u64 bpf_cookie; > #endif > @@ -1337,10 +1336,7 @@ __is_default_overflow_handler(perf_overflow_handler_t overflow_handler) > #ifdef CONFIG_BPF_SYSCALL > static inline bool uses_default_overflow_handler(struct perf_event *event) > { > - if (likely(is_default_overflow_handler(event))) > - return true; > - > - return __is_default_overflow_handler(event->orig_overflow_handler); > + return is_default_overflow_handler(event); > } > #else > #define uses_default_overflow_handler(event) \ > diff --git a/kernel/events/core.c b/kernel/events/core.c > index b704d83a28b2..54f6372d2634 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -9515,6 +9515,12 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r > return true; > } > > +#ifdef CONFIG_BPF_SYSCALL > +static int bpf_overflow_handler(struct perf_event *event, > + struct perf_sample_data *data, > + struct pt_regs *regs); > +#endif To avoid more #ifdefs we usually add a stub, something like: #ifdef ... static int bpf_overflow_handler(...); #else static inline int bpf_overflow_handler(...) { return 0; } #endif Then you can avoid more #ifdefs below, esp. when it surrounds an if-statement it easily leads to confusion or subtle bugs in future changes. The compiler will optimize out the constants and the generated code will be the same. > /* > * Generic event overflow handling, sampling. > */ > @@ -9584,7 +9590,10 @@ static int __perf_event_overflow(struct perf_event *event, > irq_work_queue(&event->pending_irq); > } > > - READ_ONCE(event->overflow_handler)(event, data, regs); > +#ifdef CONFIG_BPF_SYSCALL > + if (!(event->prog && !bpf_overflow_handler(event, data, regs))) > +#endif > + READ_ONCE(event->overflow_handler)(event, data, regs); > > if (*perf_event_fasync(event) && event->pending_kill) { > event->pending_wakeup = 1; > @@ -10394,9 +10403,9 @@ static void perf_event_free_filter(struct perf_event *event) > } > > #ifdef CONFIG_BPF_SYSCALL > -static void bpf_overflow_handler(struct perf_event *event, > - struct perf_sample_data *data, > - struct pt_regs *regs) > +static int bpf_overflow_handler(struct perf_event *event, > + struct perf_sample_data *data, > + struct pt_regs *regs) > { > struct bpf_perf_event_data_kern ctx = { > .data = data, > @@ -10417,10 +10426,8 @@ static void bpf_overflow_handler(struct perf_event *event, > rcu_read_unlock(); > out: > __this_cpu_dec(bpf_prog_active); > - if (!ret) > - return; > > - event->orig_overflow_handler(event, data, regs); > + return ret; > } > > static int perf_event_set_bpf_handler(struct perf_event *event, > @@ -10456,8 +10463,6 @@ static int perf_event_set_bpf_handler(struct perf_event *event, > > event->prog = prog; > event->bpf_cookie = bpf_cookie; > - event->orig_overflow_handler = READ_ONCE(event->overflow_handler); > - WRITE_ONCE(event->overflow_handler, bpf_overflow_handler); > return 0; > } > > @@ -10468,7 +10473,6 @@ static void perf_event_free_bpf_handler(struct perf_event *event) > if (!prog) > return; > > - WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); > event->prog = NULL; > bpf_prog_put(prog); > } > @@ -11928,13 +11932,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > overflow_handler = parent_event->overflow_handler; > context = parent_event->overflow_handler_context; > #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING) > - if (overflow_handler == bpf_overflow_handler) { > + if (parent_event->prog) { > struct bpf_prog *prog = parent_event->prog; > > bpf_prog_inc(prog); > event->prog = prog; > - event->orig_overflow_handler = > - parent_event->orig_overflow_handler; > } > #endif > } > -- > 2.34.1 >
On Mon, Dec 11, 2023 at 6:20 AM Marco Elver <elver@google.com> wrote: > > On Mon, 11 Dec 2023 at 05:55, Kyle Huey <me@kylehuey.com> wrote: > > > > To ultimately allow bpf programs attached to perf events to completely > > suppress all of the effects of a perf event overflow (rather than just the > > sample output, as they do today), call bpf_overflow_handler() from > > __perf_event_overflow() directly rather than modifying struct perf_event's > > overflow_handler. Return the bpf program's return value from > > bpf_overflow_handler() so that __perf_event_overflow() knows how to > > proceed. Remove the now unnecessary orig_overflow_handler from struct > > perf_event. > > > > This patch is solely a refactoring and results in no behavior change. > > > > Signed-off-by: Kyle Huey <khuey@kylehuey.com> > > Suggested-by: Namhyung Kim <namhyung@kernel.org> > > --- > > include/linux/perf_event.h | 6 +----- > > kernel/events/core.c | 28 +++++++++++++++------------- > > 2 files changed, 16 insertions(+), 18 deletions(-) > > > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > > index 5547ba68e6e4..312b9f31442c 100644 > > --- a/include/linux/perf_event.h > > +++ b/include/linux/perf_event.h > > @@ -810,7 +810,6 @@ struct perf_event { > > perf_overflow_handler_t overflow_handler; > > void *overflow_handler_context; > > #ifdef CONFIG_BPF_SYSCALL > > - perf_overflow_handler_t orig_overflow_handler; > > struct bpf_prog *prog; > > u64 bpf_cookie; > > #endif > > @@ -1337,10 +1336,7 @@ __is_default_overflow_handler(perf_overflow_handler_t overflow_handler) > > #ifdef CONFIG_BPF_SYSCALL > > static inline bool uses_default_overflow_handler(struct perf_event *event) > > { > > - if (likely(is_default_overflow_handler(event))) > > - return true; > > - > > - return __is_default_overflow_handler(event->orig_overflow_handler); > > + return is_default_overflow_handler(event); > > } > > #else > > #define uses_default_overflow_handler(event) \ > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index b704d83a28b2..54f6372d2634 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -9515,6 +9515,12 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r > > return true; > > } > > > > +#ifdef CONFIG_BPF_SYSCALL > > +static int bpf_overflow_handler(struct perf_event *event, > > + struct perf_sample_data *data, > > + struct pt_regs *regs); > > +#endif > > To avoid more #ifdefs we usually add a stub, something like: > > #ifdef ... > static int bpf_overflow_handler(...); > #else > static inline int bpf_overflow_handler(...) { return 0; } > #endif > > Then you can avoid more #ifdefs below, esp. when it surrounds an > if-statement it easily leads to confusion or subtle bugs in future > changes. The compiler will optimize out the constants and the > generated code will be the same. This would not allow removing any #ifdefs because event->prog is only present if CONFIG_BPF_SYSCALL is defined. - Kyle > > /* > > * Generic event overflow handling, sampling. > > */ > > @@ -9584,7 +9590,10 @@ static int __perf_event_overflow(struct perf_event *event, > > irq_work_queue(&event->pending_irq); > > } > > > > - READ_ONCE(event->overflow_handler)(event, data, regs); > > +#ifdef CONFIG_BPF_SYSCALL > > + if (!(event->prog && !bpf_overflow_handler(event, data, regs))) > > +#endif > > + READ_ONCE(event->overflow_handler)(event, data, regs); > > > > if (*perf_event_fasync(event) && event->pending_kill) { > > event->pending_wakeup = 1; > > @@ -10394,9 +10403,9 @@ static void perf_event_free_filter(struct perf_event *event) > > } > > > > #ifdef CONFIG_BPF_SYSCALL > > -static void bpf_overflow_handler(struct perf_event *event, > > - struct perf_sample_data *data, > > - struct pt_regs *regs) > > +static int bpf_overflow_handler(struct perf_event *event, > > + struct perf_sample_data *data, > > + struct pt_regs *regs) > > { > > struct bpf_perf_event_data_kern ctx = { > > .data = data, > > @@ -10417,10 +10426,8 @@ static void bpf_overflow_handler(struct perf_event *event, > > rcu_read_unlock(); > > out: > > __this_cpu_dec(bpf_prog_active); > > - if (!ret) > > - return; > > > > - event->orig_overflow_handler(event, data, regs); > > + return ret; > > } > > > > static int perf_event_set_bpf_handler(struct perf_event *event, > > @@ -10456,8 +10463,6 @@ static int perf_event_set_bpf_handler(struct perf_event *event, > > > > event->prog = prog; > > event->bpf_cookie = bpf_cookie; > > - event->orig_overflow_handler = READ_ONCE(event->overflow_handler); > > - WRITE_ONCE(event->overflow_handler, bpf_overflow_handler); > > return 0; > > } > > > > @@ -10468,7 +10473,6 @@ static void perf_event_free_bpf_handler(struct perf_event *event) > > if (!prog) > > return; > > > > - WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); > > event->prog = NULL; > > bpf_prog_put(prog); > > } > > @@ -11928,13 +11932,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > > overflow_handler = parent_event->overflow_handler; > > context = parent_event->overflow_handler_context; > > #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING) > > - if (overflow_handler == bpf_overflow_handler) { > > + if (parent_event->prog) { > > struct bpf_prog *prog = parent_event->prog; > > > > bpf_prog_inc(prog); > > event->prog = prog; > > - event->orig_overflow_handler = > > - parent_event->orig_overflow_handler; > > } > > #endif > > } > > -- > > 2.34.1 > >
On Sun, Dec 10, 2023 at 8:55 PM Kyle Huey <me@kylehuey.com> wrote: > > To ultimately allow bpf programs attached to perf events to completely > suppress all of the effects of a perf event overflow (rather than just the > sample output, as they do today), call bpf_overflow_handler() from > __perf_event_overflow() directly rather than modifying struct perf_event's > overflow_handler. Return the bpf program's return value from > bpf_overflow_handler() so that __perf_event_overflow() knows how to > proceed. Remove the now unnecessary orig_overflow_handler from struct > perf_event. > > This patch is solely a refactoring and results in no behavior change. > > Signed-off-by: Kyle Huey <khuey@kylehuey.com> > Suggested-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Song Liu <song@kernel.org>
On Sun, Dec 10, 2023 at 8:55 PM Kyle Huey <me@kylehuey.com> wrote: > > To ultimately allow bpf programs attached to perf events to completely > suppress all of the effects of a perf event overflow (rather than just the > sample output, as they do today), call bpf_overflow_handler() from > __perf_event_overflow() directly rather than modifying struct perf_event's > overflow_handler. Return the bpf program's return value from > bpf_overflow_handler() so that __perf_event_overflow() knows how to > proceed. Remove the now unnecessary orig_overflow_handler from struct > perf_event. > > This patch is solely a refactoring and results in no behavior change. > > Signed-off-by: Kyle Huey <khuey@kylehuey.com> > Suggested-by: Namhyung Kim <namhyung@kernel.org> > --- > include/linux/perf_event.h | 6 +----- > kernel/events/core.c | 28 +++++++++++++++------------- > 2 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 5547ba68e6e4..312b9f31442c 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -810,7 +810,6 @@ struct perf_event { > perf_overflow_handler_t overflow_handler; > void *overflow_handler_context; > #ifdef CONFIG_BPF_SYSCALL > - perf_overflow_handler_t orig_overflow_handler; > struct bpf_prog *prog; > u64 bpf_cookie; > #endif > @@ -1337,10 +1336,7 @@ __is_default_overflow_handler(perf_overflow_handler_t overflow_handler) > #ifdef CONFIG_BPF_SYSCALL > static inline bool uses_default_overflow_handler(struct perf_event *event) > { > - if (likely(is_default_overflow_handler(event))) > - return true; > - > - return __is_default_overflow_handler(event->orig_overflow_handler); > + return is_default_overflow_handler(event); > } > #else > #define uses_default_overflow_handler(event) \ > diff --git a/kernel/events/core.c b/kernel/events/core.c > index b704d83a28b2..54f6372d2634 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -9515,6 +9515,12 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r > return true; > } > > +#ifdef CONFIG_BPF_SYSCALL > +static int bpf_overflow_handler(struct perf_event *event, > + struct perf_sample_data *data, > + struct pt_regs *regs); > +#endif > + > /* > * Generic event overflow handling, sampling. > */ > @@ -9584,7 +9590,10 @@ static int __perf_event_overflow(struct perf_event *event, > irq_work_queue(&event->pending_irq); > } > > - READ_ONCE(event->overflow_handler)(event, data, regs); > +#ifdef CONFIG_BPF_SYSCALL > + if (!(event->prog && !bpf_overflow_handler(event, data, regs))) This condition is hard to follow. Please consider simplifying it. Thanks, Song > +#endif > + READ_ONCE(event->overflow_handler)(event, data, regs); > > if (*perf_event_fasync(event) && event->pending_kill) { > event->pending_wakeup = 1; > @@ -10394,9 +10403,9 @@ static void perf_event_free_filter(struct perf_event *event) > } > > #ifdef CONFIG_BPF_SYSCALL > -static void bpf_overflow_handler(struct perf_event *event, > - struct perf_sample_data *data, > - struct pt_regs *regs) > +static int bpf_overflow_handler(struct perf_event *event, > + struct perf_sample_data *data, > + struct pt_regs *regs) > { > struct bpf_perf_event_data_kern ctx = { > .data = data, > @@ -10417,10 +10426,8 @@ static void bpf_overflow_handler(struct perf_event *event, > rcu_read_unlock(); > out: > __this_cpu_dec(bpf_prog_active); > - if (!ret) > - return; > > - event->orig_overflow_handler(event, data, regs); > + return ret; > } > > static int perf_event_set_bpf_handler(struct perf_event *event, > @@ -10456,8 +10463,6 @@ static int perf_event_set_bpf_handler(struct perf_event *event, > > event->prog = prog; > event->bpf_cookie = bpf_cookie; > - event->orig_overflow_handler = READ_ONCE(event->overflow_handler); > - WRITE_ONCE(event->overflow_handler, bpf_overflow_handler); > return 0; > } > > @@ -10468,7 +10473,6 @@ static void perf_event_free_bpf_handler(struct perf_event *event) > if (!prog) > return; > > - WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); > event->prog = NULL; > bpf_prog_put(prog); > } > @@ -11928,13 +11932,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > overflow_handler = parent_event->overflow_handler; > context = parent_event->overflow_handler_context; > #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING) > - if (overflow_handler == bpf_overflow_handler) { > + if (parent_event->prog) { > struct bpf_prog *prog = parent_event->prog; > > bpf_prog_inc(prog); > event->prog = prog; > - event->orig_overflow_handler = > - parent_event->orig_overflow_handler; > } > #endif > } > -- > 2.34.1 > >
On Tue, Jan 2, 2024 at 3:05 PM Song Liu <song@kernel.org> wrote: > > On Sun, Dec 10, 2023 at 8:55 PM Kyle Huey <me@kylehuey.com> wrote: > > > > To ultimately allow bpf programs attached to perf events to completely > > suppress all of the effects of a perf event overflow (rather than just the > > sample output, as they do today), call bpf_overflow_handler() from > > __perf_event_overflow() directly rather than modifying struct perf_event's > > overflow_handler. Return the bpf program's return value from > > bpf_overflow_handler() so that __perf_event_overflow() knows how to > > proceed. Remove the now unnecessary orig_overflow_handler from struct > > perf_event. > > > > This patch is solely a refactoring and results in no behavior change. > > > > Signed-off-by: Kyle Huey <khuey@kylehuey.com> > > Suggested-by: Namhyung Kim <namhyung@kernel.org> > > --- > > include/linux/perf_event.h | 6 +----- > > kernel/events/core.c | 28 +++++++++++++++------------- > > 2 files changed, 16 insertions(+), 18 deletions(-) > > > > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > > index 5547ba68e6e4..312b9f31442c 100644 > > --- a/include/linux/perf_event.h > > +++ b/include/linux/perf_event.h > > @@ -810,7 +810,6 @@ struct perf_event { > > perf_overflow_handler_t overflow_handler; > > void *overflow_handler_context; > > #ifdef CONFIG_BPF_SYSCALL > > - perf_overflow_handler_t orig_overflow_handler; > > struct bpf_prog *prog; > > u64 bpf_cookie; > > #endif > > @@ -1337,10 +1336,7 @@ __is_default_overflow_handler(perf_overflow_handler_t overflow_handler) > > #ifdef CONFIG_BPF_SYSCALL > > static inline bool uses_default_overflow_handler(struct perf_event *event) > > { > > - if (likely(is_default_overflow_handler(event))) > > - return true; > > - > > - return __is_default_overflow_handler(event->orig_overflow_handler); > > + return is_default_overflow_handler(event); > > } > > #else > > #define uses_default_overflow_handler(event) \ > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index b704d83a28b2..54f6372d2634 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -9515,6 +9515,12 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r > > return true; > > } > > > > +#ifdef CONFIG_BPF_SYSCALL > > +static int bpf_overflow_handler(struct perf_event *event, > > + struct perf_sample_data *data, > > + struct pt_regs *regs); > > +#endif > > + > > /* > > * Generic event overflow handling, sampling. > > */ > > @@ -9584,7 +9590,10 @@ static int __perf_event_overflow(struct perf_event *event, > > irq_work_queue(&event->pending_irq); > > } > > > > - READ_ONCE(event->overflow_handler)(event, data, regs); > > +#ifdef CONFIG_BPF_SYSCALL > > + if (!(event->prog && !bpf_overflow_handler(event, data, regs))) > > This condition is hard to follow. Please consider simplifying it. > > Thanks, > Song It gets simplified later in patch 3/4. - Kyle > > +#endif > > + READ_ONCE(event->overflow_handler)(event, data, regs); > > > > if (*perf_event_fasync(event) && event->pending_kill) { > > event->pending_wakeup = 1; > > @@ -10394,9 +10403,9 @@ static void perf_event_free_filter(struct perf_event *event) > > } > > > > #ifdef CONFIG_BPF_SYSCALL > > -static void bpf_overflow_handler(struct perf_event *event, > > - struct perf_sample_data *data, > > - struct pt_regs *regs) > > +static int bpf_overflow_handler(struct perf_event *event, > > + struct perf_sample_data *data, > > + struct pt_regs *regs) > > { > > struct bpf_perf_event_data_kern ctx = { > > .data = data, > > @@ -10417,10 +10426,8 @@ static void bpf_overflow_handler(struct perf_event *event, > > rcu_read_unlock(); > > out: > > __this_cpu_dec(bpf_prog_active); > > - if (!ret) > > - return; > > > > - event->orig_overflow_handler(event, data, regs); > > + return ret; > > } > > > > static int perf_event_set_bpf_handler(struct perf_event *event, > > @@ -10456,8 +10463,6 @@ static int perf_event_set_bpf_handler(struct perf_event *event, > > > > event->prog = prog; > > event->bpf_cookie = bpf_cookie; > > - event->orig_overflow_handler = READ_ONCE(event->overflow_handler); > > - WRITE_ONCE(event->overflow_handler, bpf_overflow_handler); > > return 0; > > } > > > > @@ -10468,7 +10473,6 @@ static void perf_event_free_bpf_handler(struct perf_event *event) > > if (!prog) > > return; > > > > - WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); > > event->prog = NULL; > > bpf_prog_put(prog); > > } > > @@ -11928,13 +11932,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, > > overflow_handler = parent_event->overflow_handler; > > context = parent_event->overflow_handler_context; > > #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING) > > - if (overflow_handler == bpf_overflow_handler) { > > + if (parent_event->prog) { > > struct bpf_prog *prog = parent_event->prog; > > > > bpf_prog_inc(prog); > > event->prog = prog; > > - event->orig_overflow_handler = > > - parent_event->orig_overflow_handler; > > } > > #endif > > } > > -- > > 2.34.1 > > > >
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 5547ba68e6e4..312b9f31442c 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -810,7 +810,6 @@ struct perf_event { perf_overflow_handler_t overflow_handler; void *overflow_handler_context; #ifdef CONFIG_BPF_SYSCALL - perf_overflow_handler_t orig_overflow_handler; struct bpf_prog *prog; u64 bpf_cookie; #endif @@ -1337,10 +1336,7 @@ __is_default_overflow_handler(perf_overflow_handler_t overflow_handler) #ifdef CONFIG_BPF_SYSCALL static inline bool uses_default_overflow_handler(struct perf_event *event) { - if (likely(is_default_overflow_handler(event))) - return true; - - return __is_default_overflow_handler(event->orig_overflow_handler); + return is_default_overflow_handler(event); } #else #define uses_default_overflow_handler(event) \ diff --git a/kernel/events/core.c b/kernel/events/core.c index b704d83a28b2..54f6372d2634 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -9515,6 +9515,12 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r return true; } +#ifdef CONFIG_BPF_SYSCALL +static int bpf_overflow_handler(struct perf_event *event, + struct perf_sample_data *data, + struct pt_regs *regs); +#endif + /* * Generic event overflow handling, sampling. */ @@ -9584,7 +9590,10 @@ static int __perf_event_overflow(struct perf_event *event, irq_work_queue(&event->pending_irq); } - READ_ONCE(event->overflow_handler)(event, data, regs); +#ifdef CONFIG_BPF_SYSCALL + if (!(event->prog && !bpf_overflow_handler(event, data, regs))) +#endif + READ_ONCE(event->overflow_handler)(event, data, regs); if (*perf_event_fasync(event) && event->pending_kill) { event->pending_wakeup = 1; @@ -10394,9 +10403,9 @@ static void perf_event_free_filter(struct perf_event *event) } #ifdef CONFIG_BPF_SYSCALL -static void bpf_overflow_handler(struct perf_event *event, - struct perf_sample_data *data, - struct pt_regs *regs) +static int bpf_overflow_handler(struct perf_event *event, + struct perf_sample_data *data, + struct pt_regs *regs) { struct bpf_perf_event_data_kern ctx = { .data = data, @@ -10417,10 +10426,8 @@ static void bpf_overflow_handler(struct perf_event *event, rcu_read_unlock(); out: __this_cpu_dec(bpf_prog_active); - if (!ret) - return; - event->orig_overflow_handler(event, data, regs); + return ret; } static int perf_event_set_bpf_handler(struct perf_event *event, @@ -10456,8 +10463,6 @@ static int perf_event_set_bpf_handler(struct perf_event *event, event->prog = prog; event->bpf_cookie = bpf_cookie; - event->orig_overflow_handler = READ_ONCE(event->overflow_handler); - WRITE_ONCE(event->overflow_handler, bpf_overflow_handler); return 0; } @@ -10468,7 +10473,6 @@ static void perf_event_free_bpf_handler(struct perf_event *event) if (!prog) return; - WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); event->prog = NULL; bpf_prog_put(prog); } @@ -11928,13 +11932,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, overflow_handler = parent_event->overflow_handler; context = parent_event->overflow_handler_context; #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING) - if (overflow_handler == bpf_overflow_handler) { + if (parent_event->prog) { struct bpf_prog *prog = parent_event->prog; bpf_prog_inc(prog); event->prog = prog; - event->orig_overflow_handler = - parent_event->orig_overflow_handler; } #endif }