Re: [PATCH v8 1/5 alt] rust: add generic static_key_false

Boqun Feng
2024-08-22T13:43:18Z

On Thu, Aug 22, 2024 at 12:08:07PM +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.
>
> 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.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> ---
> This is an alternate version of patch 1 that resolves the conflict with
> https://lore.kernel.org/all/20240725183325.122827-7-ojeda@kernel.org/
>
> rust/bindings/bindings_helper.h | 1 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/tracepoint.c | 18 ++++++++++++++++++
> rust/kernel/jump_label.rs | 29 +++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 50 insertions(+)
> create mode 100644 rust/helpers/tracepoint.c
> create mode 100644 rust/kernel/jump_label.rs
>
> 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 173533616c91..5b17839de43a 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -20,6 +20,7 @@
> #include "slab.c"
> #include "spinlock.c"
> #include "task.c"
> +#include "tracepoint.c"
> #include "uaccess.c"
> #include "wait.c"
> #include "workqueue.c"
> diff --git a/rust/helpers/tracepoint.c b/rust/helpers/tracepoint.c
> new file mode 100644
> index 000000000000..02aafb2b226f
> --- /dev/null
> +++ b/rust/helpers/tracepoint.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Helpers for tracepoints. At the moment, helpers are only needed when
> + * CONFIG_JUMP_LABEL is disabled, as `static_key_count` is only marked inline
> + * in that case.
> + *
> + * 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);
> +}
> +#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 274bdc1b0a82..91af9f75d121 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;
> #[cfg(CONFIG_NET)]
> --
> 2.46.0.184.g6999bdac58-goog
>