Add just enough support for static key so that we can use it from
tracepoints. Tracepoints rely on `static_key_false` even though it is
deprecated, so we add the same functionality to Rust.
This patch only provides a generic implementation without code patching
(matching the one used when CONFIG_JUMP_LABEL is disabled). Later
patches add support for inline asm implementations that use runtime
patching.
When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
function, so a Rust helper is defined for `static_key_count` in this
case. If Rust is compiled with LTO, this call should get inlined. The
helper can be eliminated once we have the necessary inline asm to make
atomic operations from Rust.
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/jump_label.c | 15 +++++++++++++++
rust/kernel/jump_label.rs | 29 +++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
5 files changed, 47 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ae82e9c941af..e0846e7e93e6 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,7 @@
#include <linux/ethtool.h>
#include <linux/firmware.h>
#include <linux/jiffies.h>
+#include <linux/jump_label.h>
#include <linux/mdio.h>
#include <linux/phy.h>
#include <linux/refcount.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 30f40149f3a9..17e1b60d178f 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -12,6 +12,7 @@
#include "build_assert.c"
#include "build_bug.c"
#include "err.c"
+#include "jump_label.c"
#include "kunit.c"
#include "mutex.c"
#include "page.c"
diff --git a/rust/helpers/jump_label.c b/rust/helpers/jump_label.c
new file mode 100644
index 000000000000..0e9ed15903f6
--- /dev/null
+++ b/rust/helpers/jump_label.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2024 Google LLC.
+ */
+
+#include <linux/jump_label.h>
+
+#ifndef CONFIG_JUMP_LABEL
+int rust_helper_static_key_count(struct static_key *key)
+{
+ return static_key_count(key);
+}
+EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
+#endif
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
new file mode 100644
index 000000000000..011e1fc1d19a
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+//!
+//! C header: [`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
+
+/// Branch based on a static key.
+///
+/// Takes three arguments:
+///
+/// * `key` - the path to the static variable containing the `static_key`.
+/// * `keytyp` - the type of `key`.
+/// * `field` - the name of the field of `key` that contains the `static_key`.
+///
+/// # Safety
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_key_false {
+ ($key:path, $keytyp:ty, $field:ident) => {{
+ let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+ let _key: *const $crate::bindings::static_key = ::core::ptr::addr_of!((*_key).$field);
+
+ $crate::bindings::static_key_count(_key.cast_mut()) > 0
+ }};
+}
+pub use static_key_false;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 22a3bfa5a9e9..6dfafa69a84e 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
pub mod firmware;
pub mod init;
pub mod ioctl;
+pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
pub mod list;
--
2.46.1.824.gd892dcdcdd-googOn Tue, Oct 01, 2024 at 01:29:58PM +0000, Alice Ryhl wrote:
> Add just enough support for static key so that we can use it from
> tracepoints. Tracepoints rely on `static_key_false` even though it is
> deprecated, so we add the same functionality to Rust.
Instead of extending the old deprecated static key interface into Rust,
can we just change tracepoints to use the new one?
/me makes a note to go convert the other users...
From: Josh Poimboeuf <jpoimboe@kernel.org>
Subject: [PATCH] tracepoints: Use new static branch API
The old static key API based on 'struct static_key' is deprecated.
Convert tracepoints to use the new API.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
include/linux/tracepoint-defs.h | 4 ++--
include/linux/tracepoint.h | 8 ++++----
kernel/tracepoint.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 4dc4955f0fbf..60a6e8314d4c 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -31,7 +31,7 @@ struct tracepoint_func {
struct tracepoint {
const char *name; /* Tracepoint name */
- struct static_key key;
+ struct static_key_false key;
struct static_call_key *static_call_key;
void *static_call_tramp;
void *iterator;
@@ -83,7 +83,7 @@ struct bpf_raw_event_map {
#ifdef CONFIG_TRACEPOINTS
# define tracepoint_enabled(tp) \
- static_key_false(&(__tracepoint_##tp).key)
+ static_branch_unlikely(&(__tracepoint_##tp).key)
#else
# define tracepoint_enabled(tracepoint) false
#endif
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6be396bb4297..ab5162fc3e4a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -228,7 +228,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE_RCU(name, proto, args, cond) \
static inline void trace_##name##_rcuidle(proto) \
{ \
- if (static_key_false(&__tracepoint_##name.key)) \
+ if (static_branch_unlikely(&__tracepoint_##name.key)) \
__DO_TRACE(name, \
TP_ARGS(args), \
TP_CONDITION(cond), 1); \
@@ -254,7 +254,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
extern struct tracepoint __tracepoint_##name; \
static inline void trace_##name(proto) \
{ \
- if (static_key_false(&__tracepoint_##name.key)) \
+ if (static_branch_unlikely(&__tracepoint_##name.key)) \
__DO_TRACE(name, \
TP_ARGS(args), \
TP_CONDITION(cond), 0); \
@@ -291,7 +291,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
static inline bool \
trace_##name##_enabled(void) \
{ \
- return static_key_false(&__tracepoint_##name.key); \
+ return static_branch_unlikely(&__tracepoint_##name.key);\
}
/*
@@ -308,7 +308,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
struct tracepoint __tracepoint_##_name __used \
__section("__tracepoints") = { \
.name = __tpstrtab_##_name, \
- .key = STATIC_KEY_INIT_FALSE, \
+ .key = STATIC_KEY_FALSE_INIT, \
.static_call_key = &STATIC_CALL_KEY(tp_func_##_name), \
.static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
.iterator = &__traceiter_##_name, \
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 8d1507dd0724..dc160cc0b616 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -358,7 +358,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
tracepoint_update_call(tp, tp_funcs);
/* Both iterator and static call handle NULL tp->funcs */
rcu_assign_pointer(tp->funcs, tp_funcs);
- static_key_enable(&tp->key);
+ static_branch_enable(&tp->key);
break;
case TP_FUNC_2: /* 1->2 */
/* Set iterator static call */
@@ -414,7 +414,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
if (tp->unregfunc && static_key_enabled(&tp->key))
tp->unregfunc();
- static_key_disable(&tp->key);
+ static_branch_disable(&tp->key);
/* Set iterator static call */
tracepoint_update_call(tp, tp_funcs);
/* Both iterator and static call handle NULL tp->funcs */
--
2.46.0
On Tue, 1 Oct 2024 14:15:43 -0700 Josh Poimboeuf <jpoimboe@kernel.org> wrote: > On Tue, Oct 01, 2024 at 01:29:58PM +0000, Alice Ryhl wrote: > > Add just enough support for static key so that we can use it from > > tracepoints. Tracepoints rely on `static_key_false` even though it is > > deprecated, so we add the same functionality to Rust. > > Instead of extending the old deprecated static key interface into Rust, > can we just change tracepoints to use the new one? > > /me makes a note to go convert the other users... > > From: Josh Poimboeuf <jpoimboe@kernel.org> > Subject: [PATCH] tracepoints: Use new static branch API > > The old static key API based on 'struct static_key' is deprecated. > Convert tracepoints to use the new API. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Alice, Can you send a v10 with the added acks and whitespace fixes as well as using static_branch_unlikely(), and I'll pull it into my tree. Base it off of v6.12-rc2. Thanks, -- Steve
On Thu, Oct 10, 2024 at 9:43 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 1 Oct 2024 14:15:43 -0700 > Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > On Tue, Oct 01, 2024 at 01:29:58PM +0000, Alice Ryhl wrote: > > > Add just enough support for static key so that we can use it from > > > tracepoints. Tracepoints rely on `static_key_false` even though it is > > > deprecated, so we add the same functionality to Rust. > > > > Instead of extending the old deprecated static key interface into Rust, > > can we just change tracepoints to use the new one? > > > > /me makes a note to go convert the other users... > > > > From: Josh Poimboeuf <jpoimboe@kernel.org> > > Subject: [PATCH] tracepoints: Use new static branch API > > > > The old static key API based on 'struct static_key' is deprecated. > > Convert tracepoints to use the new API. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > > Alice, > > Can you send a v10 with the added acks and whitespace fixes as well as > using static_branch_unlikely(), and I'll pull it into my tree. > > Base it off of v6.12-rc2. Here you go: https://lore.kernel.org/all/20241011-tracepoint-v10-0-7fbde4d6b525@google.com/ Alice
On Tue, 1 Oct 2024 14:15:43 -0700
Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> On Tue, Oct 01, 2024 at 01:29:58PM +0000, Alice Ryhl wrote:
> > Add just enough support for static key so that we can use it from
> > tracepoints. Tracepoints rely on `static_key_false` even though it is
> > deprecated, so we add the same functionality to Rust.
>
> Instead of extending the old deprecated static key interface into Rust,
> can we just change tracepoints to use the new one?
Sure, care to send the patch properly, and I can add it to my queue.
-- Steve
>
> /me makes a note to go convert the other users...
>
> From: Josh Poimboeuf <jpoimboe@kernel.org>
> Subject: [PATCH] tracepoints: Use new static branch API
>
> The old static key API based on 'struct static_key' is deprecated.
> Convert tracepoints to use the new API.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> include/linux/tracepoint-defs.h | 4 ++--
> include/linux/tracepoint.h | 8 ++++----
> kernel/tracepoint.c | 4 ++--
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 4dc4955f0fbf..60a6e8314d4c 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -31,7 +31,7 @@ struct tracepoint_func {
>
> struct tracepoint {
> const char *name; /* Tracepoint name */
> - struct static_key key;
> + struct static_key_false key;
> struct static_call_key *static_call_key;
> void *static_call_tramp;
> void *iterator;
> @@ -83,7 +83,7 @@ struct bpf_raw_event_map {
>
> #ifdef CONFIG_TRACEPOINTS
> # define tracepoint_enabled(tp) \
> - static_key_false(&(__tracepoint_##tp).key)
> + static_branch_unlikely(&(__tracepoint_##tp).key)
> #else
> # define tracepoint_enabled(tracepoint) false
> #endif
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 6be396bb4297..ab5162fc3e4a 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -228,7 +228,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> #define __DECLARE_TRACE_RCU(name, proto, args, cond) \
> static inline void trace_##name##_rcuidle(proto) \
> { \
> - if (static_key_false(&__tracepoint_##name.key)) \
> + if (static_branch_unlikely(&__tracepoint_##name.key)) \
> __DO_TRACE(name, \
> TP_ARGS(args), \
> TP_CONDITION(cond), 1); \
> @@ -254,7 +254,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> extern struct tracepoint __tracepoint_##name; \
> static inline void trace_##name(proto) \
> { \
> - if (static_key_false(&__tracepoint_##name.key)) \
> + if (static_branch_unlikely(&__tracepoint_##name.key)) \
> __DO_TRACE(name, \
> TP_ARGS(args), \
> TP_CONDITION(cond), 0); \
> @@ -291,7 +291,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> static inline bool \
> trace_##name##_enabled(void) \
> { \
> - return static_key_false(&__tracepoint_##name.key); \
> + return static_branch_unlikely(&__tracepoint_##name.key);\
> }
>
> /*
> @@ -308,7 +308,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> struct tracepoint __tracepoint_##_name __used \
> __section("__tracepoints") = { \
> .name = __tpstrtab_##_name, \
> - .key = STATIC_KEY_INIT_FALSE, \
> + .key = STATIC_KEY_FALSE_INIT, \
> .static_call_key = &STATIC_CALL_KEY(tp_func_##_name), \
> .static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
> .iterator = &__traceiter_##_name, \
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 8d1507dd0724..dc160cc0b616 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -358,7 +358,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
> tracepoint_update_call(tp, tp_funcs);
> /* Both iterator and static call handle NULL tp->funcs */
> rcu_assign_pointer(tp->funcs, tp_funcs);
> - static_key_enable(&tp->key);
> + static_branch_enable(&tp->key);
> break;
> case TP_FUNC_2: /* 1->2 */
> /* Set iterator static call */
> @@ -414,7 +414,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
> if (tp->unregfunc && static_key_enabled(&tp->key))
> tp->unregfunc();
>
> - static_key_disable(&tp->key);
> + static_branch_disable(&tp->key);
> /* Set iterator static call */
> tracepoint_update_call(tp, tp_funcs);
> /* Both iterator and static call handle NULL tp->funcs */
On Tue, Oct 01, 2024 at 05:46:06PM -0400, Steven Rostedt wrote: > On Tue, 1 Oct 2024 14:15:43 -0700 > Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > On Tue, Oct 01, 2024 at 01:29:58PM +0000, Alice Ryhl wrote: > > > Add just enough support for static key so that we can use it from > > > tracepoints. Tracepoints rely on `static_key_false` even though it is > > > deprecated, so we add the same functionality to Rust. > > > > Instead of extending the old deprecated static key interface into Rust, > > can we just change tracepoints to use the new one? > > Sure, care to send the patch properly, and I can add it to my queue. Done: https://lore.kernel.org/03da49e81c50eb2a9f47918409e4c590c0f28060.1727817249.git.jpoimboe@kernel.org -- Josh
On Tue, 01 Oct 2024 13:29:58 +0000
Alice Ryhl <aliceryhl@google.com> wrote:
> Add just enough support for static key so that we can use it from
> tracepoints. Tracepoints rely on `static_key_false` even though it is
> deprecated, so we add the same functionality to Rust.
>
> This patch only provides a generic implementation without code patching
> (matching the one used when CONFIG_JUMP_LABEL is disabled). Later
> patches add support for inline asm implementations that use runtime
> patching.
>
> When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
> function, so a Rust helper is defined for `static_key_count` in this
> case. If Rust is compiled with LTO, this call should get inlined. The
> helper can be eliminated once we have the necessary inline asm to make
> atomic operations from Rust.
>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/jump_label.c | 15 +++++++++++++++
> rust/kernel/jump_label.rs | 29 +++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 47 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index ae82e9c941af..e0846e7e93e6 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -14,6 +14,7 @@
> #include <linux/ethtool.h>
> #include <linux/firmware.h>
> #include <linux/jiffies.h>
> +#include <linux/jump_label.h>
> #include <linux/mdio.h>
> #include <linux/phy.h>
> #include <linux/refcount.h>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 30f40149f3a9..17e1b60d178f 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -12,6 +12,7 @@
> #include "build_assert.c"
> #include "build_bug.c"
> #include "err.c"
> +#include "jump_label.c"
> #include "kunit.c"
> #include "mutex.c"
> #include "page.c"
> diff --git a/rust/helpers/jump_label.c b/rust/helpers/jump_label.c
> new file mode 100644
> index 000000000000..0e9ed15903f6
> --- /dev/null
> +++ b/rust/helpers/jump_label.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2024 Google LLC.
> + */
> +
> +#include <linux/jump_label.h>
> +
Nit, the above line has a spurious space.
-- Steve
> +#ifndef CONFIG_JUMP_LABEL
> +int rust_helper_static_key_count(struct static_key *key)
> +{
> + return static_key_count(key);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
> +#endif
> diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
> new file mode 100644
On Tue, Oct 1, 2024 at 4:11 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 01 Oct 2024 13:29:58 +0000 > Alice Ryhl <aliceryhl@google.com> wrote: > > > Add just enough support for static key so that we can use it from > > tracepoints. Tracepoints rely on `static_key_false` even though it is > > deprecated, so we add the same functionality to Rust. > > > > This patch only provides a generic implementation without code patching > > (matching the one used when CONFIG_JUMP_LABEL is disabled). Later > > patches add support for inline asm implementations that use runtime > > patching. > > > > When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline > > function, so a Rust helper is defined for `static_key_count` in this > > case. If Rust is compiled with LTO, this call should get inlined. The > > helper can be eliminated once we have the necessary inline asm to make > > atomic operations from Rust. > > > > Reviewed-by: Boqun Feng <boqun.feng@gmail.com> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > > --- > > rust/bindings/bindings_helper.h | 1 + > > rust/helpers/helpers.c | 1 + > > rust/helpers/jump_label.c | 15 +++++++++++++++ > > rust/kernel/jump_label.rs | 29 +++++++++++++++++++++++++++++ > > rust/kernel/lib.rs | 1 + > > 5 files changed, 47 insertions(+) > > > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > > index ae82e9c941af..e0846e7e93e6 100644 > > --- a/rust/bindings/bindings_helper.h > > +++ b/rust/bindings/bindings_helper.h > > @@ -14,6 +14,7 @@ > > #include <linux/ethtool.h> > > #include <linux/firmware.h> > > #include <linux/jiffies.h> > > +#include <linux/jump_label.h> > > #include <linux/mdio.h> > > #include <linux/phy.h> > > #include <linux/refcount.h> > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > > index 30f40149f3a9..17e1b60d178f 100644 > > --- a/rust/helpers/helpers.c > > +++ b/rust/helpers/helpers.c > > @@ -12,6 +12,7 @@ > > #include "build_assert.c" > > #include "build_bug.c" > > #include "err.c" > > +#include "jump_label.c" > > #include "kunit.c" > > #include "mutex.c" > > #include "page.c" > > diff --git a/rust/helpers/jump_label.c b/rust/helpers/jump_label.c > > new file mode 100644 > > index 000000000000..0e9ed15903f6 > > --- /dev/null > > +++ b/rust/helpers/jump_label.c > > @@ -0,0 +1,15 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * Copyright (C) 2024 Google LLC. > > + */ > > + > > +#include <linux/jump_label.h> > > + > > Nit, the above line has a spurious space. Thanks. Looks like the function body also uses 7 spaces instead of a tab. I'll fix it. Alice
© 2016 - 2024 Red Hat, Inc.