* [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 10:13 [PATCH v10 0/5] Tracepoints and static branch in Rust Alice Ryhl
@ 2024-10-11 10:13 ` Alice Ryhl
2024-10-11 12:13 ` Gary Guo
2024-10-11 10:13 ` [PATCH v10 2/5] rust: add tracepoint support Alice Ryhl
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-10-11 10:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch, Alice Ryhl
Add just enough support for static key so that we can use it from
tracepoints. Tracepoints rely on `static_branch_unlikely` with a `struct
static_key_false`, 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 | 30 ++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
5 files changed, 48 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..6948cae5738f
--- /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..4b7655b2a022
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,30 @@
+// 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_branch_unlikely {
+ ($key:path, $keytyp:ty, $field:ident) => {{
+ let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+ let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
+ let _key: *const $crate::bindings::static_key = _key.cast();
+
+ $crate::bindings::static_key_count(_key.cast_mut()) > 0
+ }};
+}
+pub use static_branch_unlikely;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b5f4b3ce6b48..708ff817ccc3 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.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 10:13 ` [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false Alice Ryhl
@ 2024-10-11 12:13 ` Gary Guo
2024-10-11 15:23 ` Sami Tolvanen
0 siblings, 1 reply; 20+ messages in thread
From: Gary Guo @ 2024-10-11 12:13 UTC (permalink / raw)
To: Alice Ryhl
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, 11 Oct 2024 10:13:34 +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_branch_unlikely` with a `struct
> static_key_false`, 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 | 30 ++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 1 +
> 5 files changed, 48 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..6948cae5738f
> --- /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);
^ Explicit export should be removed. This only works because we didn't
remove export.h from all helpers.c yet, but there's a patch to do
that and this will stop working.
You can add a Reviewed-by from me with this fixed.
> +#endif
> diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
> new file mode 100644
> index 000000000000..4b7655b2a022
> --- /dev/null
> +++ b/rust/kernel/jump_label.rs
> @@ -0,0 +1,30 @@
> +// 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_branch_unlikely {
> + ($key:path, $keytyp:ty, $field:ident) => {{
> + let _key: *const $keytyp = ::core::ptr::addr_of!($key);
> + let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
> + let _key: *const $crate::bindings::static_key = _key.cast();
> +
> + $crate::bindings::static_key_count(_key.cast_mut()) > 0
> + }};
> +}
> +pub use static_branch_unlikely;
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index b5f4b3ce6b48..708ff817ccc3 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;
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 12:13 ` Gary Guo
@ 2024-10-11 15:23 ` Sami Tolvanen
2024-10-11 16:12 ` Gary Guo
0 siblings, 1 reply; 20+ messages in thread
From: Sami Tolvanen @ 2024-10-11 15:23 UTC (permalink / raw)
To: Gary Guo
Cc: Alice Ryhl, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, Oct 11, 2024 at 5:13 AM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri, 11 Oct 2024 10:13:34 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
> > +#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);
>
> ^ Explicit export should be removed. This only works because we didn't
> remove export.h from all helpers.c yet, but there's a patch to do
> that and this will stop working.
What's the benefit of removing explicit exports from the Rust helper C
code? It requires special casing things like modversions for these
files, so I assume there's a reason for this. I asked about it here,
but never got a response:
https://lore.kernel.org/rust-for-linux/CABCJKudqAEvLcdqTqyfE2+iW+jeqBpnTGgYJvrZ0by6hGdfevQ@mail.gmail.com/
Sami
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 15:23 ` Sami Tolvanen
@ 2024-10-11 16:12 ` Gary Guo
2024-10-11 17:51 ` Sami Tolvanen
0 siblings, 1 reply; 20+ messages in thread
From: Gary Guo @ 2024-10-11 16:12 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Alice Ryhl, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, 11 Oct 2024 08:23:18 -0700
Sami Tolvanen <samitolvanen@google.com> wrote:
> On Fri, Oct 11, 2024 at 5:13 AM Gary Guo <gary@garyguo.net> wrote:
> >
> > On Fri, 11 Oct 2024 10:13:34 +0000
> > Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > > +#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);
> >
> > ^ Explicit export should be removed. This only works because we didn't
> > remove export.h from all helpers.c yet, but there's a patch to do
> > that and this will stop working.
>
> What's the benefit of removing explicit exports from the Rust helper C
> code? It requires special casing things like modversions for these
> files, so I assume there's a reason for this. I asked about it here,
> but never got a response:
>
> https://lore.kernel.org/rust-for-linux/CABCJKudqAEvLcdqTqyfE2+iW+jeqBpnTGgYJvrZ0by6hGdfevQ@mail.gmail.com/
>
> Sami
Ah, I didn't saw that email, probably because I archived the mails after
the patch is applied.
We're working towards having an option that enables inlining these
helpers into Rust; when that option is enabled, the helpers must not be
exported. See
https://lore.kernel.org/rust-for-linux/20240529202817.3641974-1-gary@garyguo.net/
and https://lwn.net/Articles/993163/.
It's also quite tedious for every helper to carry this export.
Best,
Gary
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 16:12 ` Gary Guo
@ 2024-10-11 17:51 ` Sami Tolvanen
2024-10-11 19:48 ` Miguel Ojeda
0 siblings, 1 reply; 20+ messages in thread
From: Sami Tolvanen @ 2024-10-11 17:51 UTC (permalink / raw)
To: Gary Guo
Cc: Alice Ryhl, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
Hi Gary,
On Fri, Oct 11, 2024 at 9:12 AM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri, 11 Oct 2024 08:23:18 -0700
> Sami Tolvanen <samitolvanen@google.com> wrote:
>
> > On Fri, Oct 11, 2024 at 5:13 AM Gary Guo <gary@garyguo.net> wrote:
> > >
> > > On Fri, 11 Oct 2024 10:13:34 +0000
> > > Alice Ryhl <aliceryhl@google.com> wrote:
> > >
> > > > +#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);
> > >
> > > ^ Explicit export should be removed. This only works because we didn't
> > > remove export.h from all helpers.c yet, but there's a patch to do
> > > that and this will stop working.
> >
> > What's the benefit of removing explicit exports from the Rust helper C
> > code? It requires special casing things like modversions for these
> > files, so I assume there's a reason for this. I asked about it here,
> > but never got a response:
> >
> > https://lore.kernel.org/rust-for-linux/CABCJKudqAEvLcdqTqyfE2+iW+jeqBpnTGgYJvrZ0by6hGdfevQ@mail.gmail.com/
> >
> > Sami
>
> Ah, I didn't saw that email, probably because I archived the mails after
> the patch is applied.
Sometimes you might get pings about patches that are already applied too. :)
> We're working towards having an option that enables inlining these
> helpers into Rust; when that option is enabled, the helpers must not be
> exported. See
> https://lore.kernel.org/rust-for-linux/20240529202817.3641974-1-gary@garyguo.net/
> and https://lwn.net/Articles/993163/.
Interesting, thanks for the links. It would have been helpful to
explain the motivation for the change also in the patch that was
applied.
Did you consider using the preprocessor to simply skip exporting the
helpers when cross-language LTO inlining is used? This would allow us
to use the existing C build rules for the code instead of adding a
separate rule to handle Rust-style exports, like I'm doing here:
https://github.com/samitolvanen/linux/commit/545277e4d0432dafc530b1618f0152aed82af2f5
> It's also quite tedious for every helper to carry this export.
It's just one line per helper, but sure, I do see your point.
Sami
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 17:51 ` Sami Tolvanen
@ 2024-10-11 19:48 ` Miguel Ojeda
2024-10-11 21:25 ` Sami Tolvanen
0 siblings, 1 reply; 20+ messages in thread
From: Miguel Ojeda @ 2024-10-11 19:48 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Gary Guo, Alice Ryhl, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, Oct 11, 2024 at 7:52 PM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> It's just one line per helper, but sure, I do see your point.
I guess we will have a lot of helpers added over time, so even if it
is one line, it may end up being a lot of lines in total. The rules
should stay constant, which would be better. Having said that, it is
true the extra complexity of the rules isn't great either.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false
2024-10-11 19:48 ` Miguel Ojeda
@ 2024-10-11 21:25 ` Sami Tolvanen
0 siblings, 0 replies; 20+ messages in thread
From: Sami Tolvanen @ 2024-10-11 21:25 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Gary Guo, Alice Ryhl, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, Oct 11, 2024 at 12:48 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Oct 11, 2024 at 7:52 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > It's just one line per helper, but sure, I do see your point.
>
> I guess we will have a lot of helpers added over time, so even if it
> is one line, it may end up being a lot of lines in total. The rules
> should stay constant, which would be better. Having said that, it is
> true the extra complexity of the rules isn't great either.
My only concern is that custom C build rules must be kept in sync with
Makefile.build changes, while EXPORT_SYMBOL lines should be basically
maintenance-free. However, perhaps this isn't really an issue since
most of the complexity is already contained in rule_cc_o_c that we can
conveniently reuse. Anyway, I think this is something we can worry
about later if we actually run into problems. I was mostly interested
in the reasoning behind these changes.
Sami
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v10 2/5] rust: add tracepoint support
2024-10-11 10:13 [PATCH v10 0/5] Tracepoints and static branch in Rust Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false Alice Ryhl
@ 2024-10-11 10:13 ` Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 3/5] rust: samples: add tracepoint to Rust sample Alice Ryhl
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-10-11 10:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch, Alice Ryhl,
Carlos Llamas
Make it possible to have Rust code call into tracepoints defined by C
code. It is still required that the tracepoint is declared in a C
header, and that this header is included in the input to bindgen.
Instead of calling __DO_TRACE directly, the exported rust_do_trace_
function calls an inline helper function. This is because the `cond`
argument does not exist at the callsite of DEFINE_RUST_DO_TRACE.
__DECLARE_TRACE always emits an inline static and an extern declaration
that is only used when CREATE_RUST_TRACE_POINTS is set. These should not
end up in the final binary so it is not a problem that they sometimes
are emitted without a user.
Reviewed-by: Carlos Llamas <cmllamas@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
include/linux/tracepoint.h | 28 ++++++++++++++++++++++-
include/trace/define_trace.h | 12 ++++++++++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 49 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 0dc67fad706c..84c4924e499f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -225,6 +225,18 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
preempt_enable_notrace(); \
} while (0)
+/*
+ * Declare an exported function that Rust code can call to trigger this
+ * tracepoint. This function does not include the static branch; that is done
+ * in Rust to avoid a function call when the tracepoint is disabled.
+ */
+#define DEFINE_RUST_DO_TRACE(name, proto, args)
+#define __DEFINE_RUST_DO_TRACE(name, proto, args) \
+ notrace void rust_do_trace_##name(proto) \
+ { \
+ __rust_do_trace_##name(args); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -240,6 +252,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
extern int __traceiter_##name(data_proto); \
DECLARE_STATIC_CALL(tp_func_##name, __traceiter_##name); \
extern struct tracepoint __tracepoint_##name; \
+ extern void rust_do_trace_##name(proto); \
static inline int \
register_trace_##name(void (*probe)(data_proto), void *data) \
{ \
@@ -271,6 +284,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE(name, proto, args, cond, data_proto) \
__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), cond, PARAMS(data_proto)) \
+ static inline void __rust_do_trace_##name(proto) \
+ { \
+ __DO_TRACE(name, \
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 0); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_branch_unlikely(&__tracepoint_##name.key)) \
@@ -285,6 +304,12 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE_SYSCALL(name, proto, args, cond, data_proto) \
__DECLARE_TRACE_COMMON(name, PARAMS(proto), PARAMS(args), cond, PARAMS(data_proto)) \
+ static inline void __rust_do_trace_##name(proto) \
+ { \
+ __DO_TRACE(name, \
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 1); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_branch_unlikely(&__tracepoint_##name.key)) \
@@ -339,7 +364,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
void __probestub_##_name(void *__data, proto) \
{ \
} \
- DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
+ DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name); \
+ DEFINE_RUST_DO_TRACE(_name, TP_PROTO(proto), TP_ARGS(args))
#define DEFINE_TRACE(name, proto, args) \
DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index ff5fa17a6259..0557626b6f6a 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -76,6 +76,13 @@
#define DECLARE_TRACE(name, proto, args) \
DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
+/* If requested, create helpers for calling these tracepoints from Rust. */
+#ifdef CREATE_RUST_TRACE_POINTS
+#undef DEFINE_RUST_DO_TRACE
+#define DEFINE_RUST_DO_TRACE(name, proto, args) \
+ __DEFINE_RUST_DO_TRACE(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -134,6 +141,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index e0846e7e93e6..752572e638a6 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -20,6 +20,7 @@
#include <linux/refcount.h>
#include <linux/sched.h>
#include <linux/slab.h>
+#include <linux/tracepoint.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 708ff817ccc3..55f81f49024e 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -54,6 +54,7 @@
pub mod sync;
pub mod task;
pub mod time;
+pub mod tracepoint;
pub mod types;
pub mod uaccess;
pub mod workqueue;
diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
new file mode 100644
index 000000000000..c6e80aa99e8e
--- /dev/null
+++ b/rust/kernel/tracepoint.rs
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for tracepoints.
+
+/// Declare the Rust entry point for a tracepoint.
+///
+/// This macro generates an unsafe function that calls into C, and its safety requirements will be
+/// whatever the relevant C code requires. To document these safety requirements, you may add
+/// doc-comments when invoking the macro.
+#[macro_export]
+macro_rules! declare_trace {
+ ($($(#[$attr:meta])* $pub:vis unsafe fn $name:ident($($argname:ident : $argtyp:ty),* $(,)?);)*) => {$(
+ $( #[$attr] )*
+ #[inline(always)]
+ $pub unsafe fn $name($($argname : $argtyp),*) {
+ #[cfg(CONFIG_TRACEPOINTS)]
+ {
+ // SAFETY: It's always okay to query the static key for a tracepoint.
+ let should_trace = unsafe {
+ $crate::macros::paste! {
+ $crate::jump_label::static_branch_unlikely!(
+ $crate::bindings::[< __tracepoint_ $name >],
+ $crate::bindings::tracepoint,
+ key
+ )
+ }
+ };
+
+ if should_trace {
+ $crate::macros::paste! {
+ // SAFETY: The caller guarantees that it is okay to call this tracepoint.
+ unsafe { $crate::bindings::[< rust_do_trace_ $name >]($($argname),*) };
+ }
+ }
+ }
+
+ #[cfg(not(CONFIG_TRACEPOINTS))]
+ {
+ // If tracepoints are disabled, insert a trivial use of each argument
+ // to avoid unused argument warnings.
+ $( let _unused = $argname; )*
+ }
+ }
+ )*}
+}
+
+pub use declare_trace;
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v10 3/5] rust: samples: add tracepoint to Rust sample
2024-10-11 10:13 [PATCH v10 0/5] Tracepoints and static branch in Rust Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 1/5] rust: add static_branch_unlikely for static_key_false Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 2/5] rust: add tracepoint support Alice Ryhl
@ 2024-10-11 10:13 ` Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 4/5] jump_label: adjust inline asm to be consistent Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 5/5] rust: add arch_static_branch Alice Ryhl
4 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-10-11 10:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch, Alice Ryhl
This updates the Rust printing sample to invoke a tracepoint. This
ensures that we have a user in-tree from the get-go even though the
patch is being merged before its real user.
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
MAINTAINERS | 1 +
include/trace/events/rust_sample.h | 31 +++++++++++++++++++++++++++++++
rust/bindings/bindings_helper.h | 1 +
samples/rust/Makefile | 3 ++-
samples/rust/rust_print.rs | 18 ++++++++++++++++++
samples/rust/rust_print_events.c | 8 ++++++++
6 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index a097afd76ded..a9b71411d77a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20223,6 +20223,7 @@ C: zulip://rust-for-linux.zulipchat.com
P: https://rust-for-linux.com/contributing
T: git https://github.com/Rust-for-Linux/linux.git rust-next
F: Documentation/rust/
+F: include/trace/events/rust_sample.h
F: rust/
F: samples/rust/
F: scripts/*rust*
diff --git a/include/trace/events/rust_sample.h b/include/trace/events/rust_sample.h
new file mode 100644
index 000000000000..dbc80ca2e465
--- /dev/null
+++ b/include/trace/events/rust_sample.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tracepoints for `samples/rust/rust_print.rs`.
+ *
+ * Copyright (C) 2024 Google, Inc.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rust_sample
+
+#if !defined(_RUST_SAMPLE_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _RUST_SAMPLE_TRACE_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(rust_sample_loaded,
+ TP_PROTO(int magic_number),
+ TP_ARGS(magic_number),
+ TP_STRUCT__entry(
+ __field(int, magic_number)
+ ),
+ TP_fast_assign(
+ __entry->magic_number = magic_number;
+ ),
+ TP_printk("magic=%d", __entry->magic_number)
+);
+
+#endif /* _RUST_SAMPLE_TRACE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 752572e638a6..b072c197ce9e 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -23,6 +23,7 @@
#include <linux/tracepoint.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include <trace/events/rust_sample.h>
/* `bindgen` gets confused at certain things. */
const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..f29280ec4820 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+ccflags-y += -I$(src) # needed for trace events
obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o
-obj-$(CONFIG_SAMPLE_RUST_PRINT) += rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_PRINT) += rust_print.o rust_print_events.o
subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 6eabb0d79ea3..6d14b08cac1c 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -69,6 +69,8 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
arc_print()?;
+ trace::trace_rust_sample_loaded(42);
+
Ok(RustPrint)
}
}
@@ -78,3 +80,19 @@ fn drop(&mut self) {
pr_info!("Rust printing macros sample (exit)\n");
}
}
+
+mod trace {
+ use core::ffi::c_int;
+
+ kernel::declare_trace! {
+ /// # Safety
+ ///
+ /// Always safe to call.
+ unsafe fn rust_sample_loaded(magic: c_int);
+ }
+
+ pub(crate) fn trace_rust_sample_loaded(magic: i32) {
+ // SAFETY: Always safe to call.
+ unsafe { rust_sample_loaded(magic as c_int) }
+ }
+}
diff --git a/samples/rust/rust_print_events.c b/samples/rust/rust_print_events.c
new file mode 100644
index 000000000000..a9169ff0edf1
--- /dev/null
+++ b/samples/rust/rust_print_events.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ */
+
+#define CREATE_TRACE_POINTS
+#define CREATE_RUST_TRACE_POINTS
+#include <trace/events/rust_sample.h>
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v10 4/5] jump_label: adjust inline asm to be consistent
2024-10-11 10:13 [PATCH v10 0/5] Tracepoints and static branch in Rust Alice Ryhl
` (2 preceding siblings ...)
2024-10-11 10:13 ` [PATCH v10 3/5] rust: samples: add tracepoint to Rust sample Alice Ryhl
@ 2024-10-11 10:13 ` Alice Ryhl
2024-10-11 10:13 ` [PATCH v10 5/5] rust: add arch_static_branch Alice Ryhl
4 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-10-11 10:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch, Alice Ryhl
To avoid duplication of inline asm between C and Rust, we need to
import the inline asm from the relevant `jump_label.h` header into Rust.
To make that easier, this patch updates the header files to expose the
inline asm via a new ARCH_STATIC_BRANCH_ASM macro.
The header files are all updated to define a ARCH_STATIC_BRANCH_ASM that
takes the same arguments in a consistent order so that Rust can use the
same logic for every architecture.
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
arch/arm/include/asm/jump_label.h | 14 +++++----
arch/arm64/include/asm/jump_label.h | 20 ++++++++-----
arch/loongarch/include/asm/jump_label.h | 16 +++++++----
arch/riscv/include/asm/jump_label.h | 50 ++++++++++++++++++---------------
arch/x86/include/asm/jump_label.h | 39 +++++++++++--------------
5 files changed, 76 insertions(+), 63 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
index e4eb54f6cd9f..a35aba7f548c 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -9,13 +9,17 @@
#define JUMP_LABEL_NOP_SIZE 4
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1:\n\t" \
+ WASM(nop) "\n\t" \
+ ".pushsection __jump_table, \"aw\"\n\t" \
+ ".word 1b, " label ", " key "\n\t" \
+ ".popsection\n\t" \
+
static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
{
- asm goto("1:\n\t"
- WASM(nop) "\n\t"
- ".pushsection __jump_table, \"aw\"\n\t"
- ".word 1b, %l[l_yes], %c0\n\t"
- ".popsection\n\t"
+ asm goto(ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
: : "i" (&((char *)key)[branch]) : : l_yes);
return false;
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index a0a5bbae7229..424ed421cd97 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -19,10 +19,14 @@
#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
".align 3\n\t" \
- ".long 1b - ., %l["#label"] - .\n\t" \
- ".quad %c0 - .\n\t" \
- ".popsection\n\t" \
- : : "i"(key) : : label
+ ".long 1b - ., " label " - .\n\t" \
+ ".quad " key " - .\n\t" \
+ ".popsection\n\t"
+
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: nop\n\t" \
+ JUMP_TABLE_ENTRY(key, label)
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
@@ -30,8 +34,8 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
char *k = &((char *)key)[branch];
asm goto(
- "1: nop \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
@@ -43,9 +47,11 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
const bool branch)
{
char *k = &((char *)key)[branch];
+
asm goto(
"1: b %l[l_yes] \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ JUMP_TABLE_ENTRY("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
l_yes:
diff --git a/arch/loongarch/include/asm/jump_label.h b/arch/loongarch/include/asm/jump_label.h
index 29acfe3de3fa..8a924bd69d19 100644
--- a/arch/loongarch/include/asm/jump_label.h
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -13,18 +13,22 @@
#define JUMP_LABEL_NOP_SIZE 4
-#define JUMP_TABLE_ENTRY \
+/* This macro is also expanded on the Rust side. */
+#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\" \n\t" \
".align 3 \n\t" \
- ".long 1b - ., %l[l_yes] - . \n\t" \
- ".quad %0 - . \n\t" \
+ ".long 1b - ., " label " - . \n\t" \
+ ".quad " key " - . \n\t" \
".popsection \n\t"
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: nop \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+
static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
{
asm goto(
- "1: nop \n\t"
- JUMP_TABLE_ENTRY
+ ARCH_STATIC_BRANCH_ASM("%0", "%l[l_yes]")
: : "i"(&((char *)key)[branch]) : : l_yes);
return false;
@@ -37,7 +41,7 @@ static __always_inline bool arch_static_branch_jump(struct static_key * const ke
{
asm goto(
"1: b %l[l_yes] \n\t"
- JUMP_TABLE_ENTRY
+ JUMP_TABLE_ENTRY("%0", "%l[l_yes]")
: : "i"(&((char *)key)[branch]) : : l_yes);
return false;
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
index 1c768d02bd0c..87a71cc6d146 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -16,21 +16,28 @@
#define JUMP_LABEL_NOP_SIZE 4
+#define JUMP_TABLE_ENTRY(key, label) \
+ ".pushsection __jump_table, \"aw\" \n\t" \
+ ".align " RISCV_LGPTR " \n\t" \
+ ".long 1b - ., " label " - . \n\t" \
+ "" RISCV_PTR " " key " - . \n\t" \
+ ".popsection \n\t"
+
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ " .align 2 \n\t" \
+ " .option push \n\t" \
+ " .option norelax \n\t" \
+ " .option norvc \n\t" \
+ "1: nop \n\t" \
+ " .option pop \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
asm goto(
- " .align 2 \n\t"
- " .option push \n\t"
- " .option norelax \n\t"
- " .option norvc \n\t"
- "1: nop \n\t"
- " .option pop \n\t"
- " .pushsection __jump_table, \"aw\" \n\t"
- " .align " RISCV_LGPTR " \n\t"
- " .long 1b - ., %l[label] - . \n\t"
- " " RISCV_PTR " %0 - . \n\t"
- " .popsection \n\t"
+ ARCH_STATIC_BRANCH_ASM("%0", "%l[label]")
: : "i"(&((char *)key)[branch]) : : label);
return false;
@@ -38,21 +45,20 @@ static __always_inline bool arch_static_branch(struct static_key * const key,
return true;
}
+#define ARCH_STATIC_BRANCH_JUMP_ASM(key, label) \
+ " .align 2 \n\t" \
+ " .option push \n\t" \
+ " .option norelax \n\t" \
+ " .option norvc \n\t" \
+ "1: j " label " \n\t" \
+ " .option pop \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+
static __always_inline bool arch_static_branch_jump(struct static_key * const key,
const bool branch)
{
asm goto(
- " .align 2 \n\t"
- " .option push \n\t"
- " .option norelax \n\t"
- " .option norvc \n\t"
- "1: j %l[label] \n\t"
- " .option pop \n\t"
- " .pushsection __jump_table, \"aw\" \n\t"
- " .align " RISCV_LGPTR " \n\t"
- " .long 1b - ., %l[label] - . \n\t"
- " " RISCV_PTR " %0 - . \n\t"
- " .popsection \n\t"
+ ARCH_STATIC_BRANCH_JUMP_ASM("%0", "%l[label]")
: : "i"(&((char *)key)[branch]) : : label);
return false;
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index cbbef32517f0..ffd0d1a1a4af 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -12,49 +12,42 @@
#include <linux/stringify.h>
#include <linux/types.h>
-#define JUMP_TABLE_ENTRY \
+#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\" \n\t" \
_ASM_ALIGN "\n\t" \
".long 1b - . \n\t" \
- ".long %l[l_yes] - . \n\t" \
- _ASM_PTR "%c0 + %c1 - .\n\t" \
+ ".long " label " - . \n\t" \
+ _ASM_PTR " " key " - . \n\t" \
".popsection \n\t"
+/* This macro is also expanded on the Rust side. */
#ifdef CONFIG_HAVE_JUMP_LABEL_HACK
-
-static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
-{
- asm goto("1:"
- "jmp %l[l_yes] # objtool NOPs this \n\t"
- JUMP_TABLE_ENTRY
- : : "i" (key), "i" (2 | branch) : : l_yes);
-
- return false;
-l_yes:
- return true;
-}
-
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: jmp " label " # objtool NOPs this \n\t" \
+ JUMP_TABLE_ENTRY(key, label)
#else /* !CONFIG_HAVE_JUMP_LABEL_HACK */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: .byte " __stringify(BYTES_NOP5) "\n\t" \
+ JUMP_TABLE_ENTRY(key, label)
+#endif /* CONFIG_HAVE_JUMP_LABEL_HACK */
static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
{
- asm goto("1:"
- ".byte " __stringify(BYTES_NOP5) "\n\t"
- JUMP_TABLE_ENTRY
- : : "i" (key), "i" (branch) : : l_yes);
+ int hack_bit = IS_ENABLED(CONFIG_HAVE_JUMP_LABEL_HACK) ? 2 : 0;
+
+ asm goto(ARCH_STATIC_BRANCH_ASM("%c0 + %c1", "%l[l_yes]")
+ : : "i" (key), "i" (hack_bit | branch) : : l_yes);
return false;
l_yes:
return true;
}
-#endif /* CONFIG_HAVE_JUMP_LABEL_HACK */
-
static __always_inline bool arch_static_branch_jump(struct static_key * const key, const bool branch)
{
asm goto("1:"
"jmp %l[l_yes]\n\t"
- JUMP_TABLE_ENTRY
+ JUMP_TABLE_ENTRY("%c0 + %c1", "%l[l_yes]")
: : "i" (key), "i" (branch) : : l_yes);
return false;
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v10 5/5] rust: add arch_static_branch
2024-10-11 10:13 [PATCH v10 0/5] Tracepoints and static branch in Rust Alice Ryhl
` (3 preceding siblings ...)
2024-10-11 10:13 ` [PATCH v10 4/5] jump_label: adjust inline asm to be consistent Alice Ryhl
@ 2024-10-11 10:13 ` Alice Ryhl
2024-10-11 15:23 ` Josh Poimboeuf
2024-10-15 11:05 ` kernel test robot
4 siblings, 2 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-10-11 10:13 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch, Alice Ryhl
To allow the Rust implementation of static_key_false to use runtime code
patching instead of the generic implementation, pull in the relevant
inline assembly from the jump_label.h header by running the C
preprocessor on a .rs.S file. Build rules are added for .rs.S files.
Since the relevant inline asm has been adjusted to export the inline asm
via the ARCH_STATIC_BRANCH_ASM macro in a consistent way, the Rust side
does not need architecture specific code to pull in the asm.
It is not possible to use the existing C implementation of
arch_static_branch via a Rust helper because it passes the argument
`key` to inline assembly as an 'i' parameter. Any attempt to add a C
helper for this function will fail to compile because the value of `key`
must be known at compile-time.
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/Makefile | 5 ++-
rust/kernel/.gitignore | 3 ++
rust/kernel/arch_static_branch_asm.rs.S | 7 ++++
rust/kernel/jump_label.rs | 64 ++++++++++++++++++++++++++++++++-
rust/kernel/lib.rs | 35 ++++++++++++++++++
scripts/Makefile.build | 9 ++++-
6 files changed, 120 insertions(+), 3 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index b5e0a73b78f3..09ea07cc4001 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -36,6 +36,8 @@ always-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.c
obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
+always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/arch_static_branch_asm.rs
+
# Avoids running `$(RUSTC)` for the sysroot when it may not be available.
ifdef CONFIG_RUST
@@ -421,7 +423,8 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
- $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+ $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
+ $(obj)/kernel/arch_static_branch_asm.rs FORCE
+$(call if_changed_rule,rustc_library)
endif # CONFIG_RUST
diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
new file mode 100644
index 000000000000..d082731007c6
--- /dev/null
+++ b/rust/kernel/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+/arch_static_branch_asm.rs
diff --git a/rust/kernel/arch_static_branch_asm.rs.S b/rust/kernel/arch_static_branch_asm.rs.S
new file mode 100644
index 000000000000..2afb638708db
--- /dev/null
+++ b/rust/kernel/arch_static_branch_asm.rs.S
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/jump_label.h>
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}", "{l_yes}"))
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
index 4b7655b2a022..cbd0ec00f0c5 100644
--- a/rust/kernel/jump_label.rs
+++ b/rust/kernel/jump_label.rs
@@ -24,7 +24,69 @@ macro_rules! static_branch_unlikely {
let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
let _key: *const $crate::bindings::static_key = _key.cast();
- $crate::bindings::static_key_count(_key.cast_mut()) > 0
+ #[cfg(not(CONFIG_JUMP_LABEL))]
+ {
+ $crate::bindings::static_key_count(_key) > 0
+ }
+
+ #[cfg(CONFIG_JUMP_LABEL)]
+ $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
}};
}
pub use static_branch_unlikely;
+
+/// Assert that the assembly block evaluates to a string literal.
+#[cfg(CONFIG_JUMP_LABEL)]
+const _: &str = include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
+macro_rules! arch_static_branch {
+ ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+ $crate::asm!(
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
+ l_yes = label {
+ break 'my_label true;
+ },
+ symb = sym $key,
+ off = const ::core::mem::offset_of!($keytyp, $field),
+ branch = const $crate::jump_label::bool_to_int($branch),
+ );
+
+ break 'my_label false;
+ }};
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
+macro_rules! arch_static_branch {
+ ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+ $crate::asm!(
+ include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
+ l_yes = label {
+ break 'my_label true;
+ },
+ symb = sym $key,
+ off = const ::core::mem::offset_of!($keytyp, $field),
+ branch = const 2 | $crate::jump_label::bool_to_int($branch),
+ );
+
+ break 'my_label false;
+ }};
+}
+
+#[cfg(CONFIG_JUMP_LABEL)]
+pub use arch_static_branch;
+
+/// A helper used by inline assembly to pass a boolean to as a `const` parameter.
+///
+/// Using this function instead of a cast lets you assert that the input is a boolean, and not some
+/// other type that can also be cast to an integer.
+#[doc(hidden)]
+pub const fn bool_to_int(b: bool) -> i32 {
+ b as i32
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 55f81f49024e..c0ae9ddd9468 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -148,3 +148,38 @@ macro_rules! container_of {
ptr.sub(offset) as *const $type
}}
}
+
+/// Helper for `.rs.S` files.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! concat_literals {
+ ($( $asm:literal )* ) => {
+ ::core::concat!($($asm),*)
+ };
+}
+
+/// Wrapper around `asm!` configured for use in the kernel.
+///
+/// Uses a semicolon to avoid parsing ambiguities, even though this does not match native `asm!`
+/// syntax.
+// For x86, `asm!` uses intel syntax by default, but we want to use at&t syntax in the kernel.
+#[cfg(target_arch = "x86_64")]
+#[macro_export]
+macro_rules! asm {
+ ($($asm:expr),* ; $($rest:tt)*) => {
+ ::core::arch::asm!( $($asm)*, options(att_syntax), $($rest)* )
+ };
+}
+
+/// Wrapper around `asm!` configured for use in the kernel.
+///
+/// Uses a semicolon to avoid parsing ambiguities, even though this does not match native `asm!`
+/// syntax.
+// For non-x86 arches we just pass through to `asm!`.
+#[cfg(not(target_arch = "x86_64"))]
+#[macro_export]
+macro_rules! asm {
+ ($($asm:expr),* ; $($rest:tt)*) => {
+ ::core::arch::asm!( $($asm)*, $($rest)* )
+ };
+}
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 8f423a1faf50..03ee558fcd4d 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -248,12 +248,13 @@ $(obj)/%.lst: $(obj)/%.c FORCE
# Compile Rust sources (.rs)
# ---------------------------------------------------------------------------
-rust_allowed_features := new_uninit
+rust_allowed_features := asm_const,asm_goto,new_uninit
# `--out-dir` is required to avoid temporaries being created by `rustc` in the
# current working directory, which may be not accessible in the out-of-tree
# modules case.
rust_common_cmd = \
+ OBJTREE=$(abspath $(objtree)) \
RUST_MODFILE=$(modfile) $(RUSTC_OR_CLIPPY) $(rust_flags) \
-Zallow-features=$(rust_allowed_features) \
-Zcrate-attr=no_std \
@@ -303,6 +304,12 @@ quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
$(obj)/%.ll: $(obj)/%.rs FORCE
+$(call if_changed_dep,rustc_ll_rs)
+quiet_cmd_rustc_rs_rs_S = RSCPP $(quiet_modtag) $@
+ cmd_rustc_rs_rs_S = $(CPP) $(c_flags) -xc -C -P $< | sed '1,/^\/\/ Cut here.$$/d' >$@
+
+$(obj)/%.rs: $(obj)/%.rs.S FORCE
+ +$(call if_changed_dep,rustc_rs_rs_S)
+
# Compile assembler sources (.S)
# ---------------------------------------------------------------------------
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-11 10:13 ` [PATCH v10 5/5] rust: add arch_static_branch Alice Ryhl
@ 2024-10-11 15:23 ` Josh Poimboeuf
2024-10-15 13:05 ` Alice Ryhl
2024-10-15 11:05 ` kernel test robot
1 sibling, 1 reply; 20+ messages in thread
From: Josh Poimboeuf @ 2024-10-11 15:23 UTC (permalink / raw)
To: Alice Ryhl
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Jason Baron, Ard Biesheuvel, Miguel Ojeda,
Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, Oct 11, 2024 at 10:13:38AM +0000, Alice Ryhl wrote:
> +#[cfg(CONFIG_JUMP_LABEL)]
> +#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
> +macro_rules! arch_static_branch {
> + ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> + $crate::asm!(
> + include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
> + l_yes = label {
> + break 'my_label true;
> + },
> + symb = sym $key,
> + off = const ::core::mem::offset_of!($keytyp, $field),
> + branch = const $crate::jump_label::bool_to_int($branch),
> + );
> +
> + break 'my_label false;
> + }};
> +}
> +
> +#[macro_export]
> +#[doc(hidden)]
> +#[cfg(CONFIG_JUMP_LABEL)]
> +#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
> +macro_rules! arch_static_branch {
> + ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> + $crate::asm!(
> + include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
> + l_yes = label {
> + break 'my_label true;
> + },
> + symb = sym $key,
> + off = const ::core::mem::offset_of!($keytyp, $field),
> + branch = const 2 | $crate::jump_label::bool_to_int($branch),
> + );
> +
> + break 'my_label false;
> + }};
Ouch... could we get rid of all this duplication by containing the hack
bit to ARCH_STATIC_BRANCH_ASM() like so?
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index ffd0d1a1a4af..3f1c1d6c0da1 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -24,7 +24,7 @@
#ifdef CONFIG_HAVE_JUMP_LABEL_HACK
#define ARCH_STATIC_BRANCH_ASM(key, label) \
"1: jmp " label " # objtool NOPs this \n\t" \
- JUMP_TABLE_ENTRY(key, label)
+ JUMP_TABLE_ENTRY(key " + 2", label)
#else /* !CONFIG_HAVE_JUMP_LABEL_HACK */
#define ARCH_STATIC_BRANCH_ASM(key, label) \
"1: .byte " __stringify(BYTES_NOP5) "\n\t" \
@@ -33,10 +33,8 @@
static __always_inline bool arch_static_branch(struct static_key * const key, const bool branch)
{
- int hack_bit = IS_ENABLED(CONFIG_HAVE_JUMP_LABEL_HACK) ? 2 : 0;
-
asm goto(ARCH_STATIC_BRANCH_ASM("%c0 + %c1", "%l[l_yes]")
- : : "i" (key), "i" (hack_bit | branch) : : l_yes);
+ : : "i" (key), "i" (branch) : : l_yes);
return false;
l_yes:
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-11 15:23 ` Josh Poimboeuf
@ 2024-10-15 13:05 ` Alice Ryhl
0 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-10-15 13:05 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Jason Baron, Ard Biesheuvel, Miguel Ojeda,
Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
linux-trace-kernel, rust-for-linux, linux-kernel, Arnd Bergmann,
linux-arch, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Sean Christopherson,
Uros Bizjak, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mark Rutland, Ryan Roberts, Fuad Tabba,
linux-arm-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Anup Patel, Andrew Jones, Alexandre Ghiti, Conor Dooley,
Samuel Holland, linux-riscv, Huacai Chen, WANG Xuerui, Bibo Mao,
Tiezhu Yang, Andrew Morton, Tianrui Zhao, loongarch
On Fri, Oct 11, 2024 at 5:24 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Fri, Oct 11, 2024 at 10:13:38AM +0000, Alice Ryhl wrote:
> > +#[cfg(CONFIG_JUMP_LABEL)]
> > +#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
> > +macro_rules! arch_static_branch {
> > + ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> > + $crate::asm!(
> > + include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
> > + l_yes = label {
> > + break 'my_label true;
> > + },
> > + symb = sym $key,
> > + off = const ::core::mem::offset_of!($keytyp, $field),
> > + branch = const $crate::jump_label::bool_to_int($branch),
> > + );
> > +
> > + break 'my_label false;
> > + }};
> > +}
> > +
> > +#[macro_export]
> > +#[doc(hidden)]
> > +#[cfg(CONFIG_JUMP_LABEL)]
> > +#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
> > +macro_rules! arch_static_branch {
> > + ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
> > + $crate::asm!(
> > + include!(concat!(env!("OBJTREE"), "/rust/kernel/arch_static_branch_asm.rs"));
> > + l_yes = label {
> > + break 'my_label true;
> > + },
> > + symb = sym $key,
> > + off = const ::core::mem::offset_of!($keytyp, $field),
> > + branch = const 2 | $crate::jump_label::bool_to_int($branch),
> > + );
> > +
> > + break 'my_label false;
> > + }};
>
> Ouch... could we get rid of all this duplication by containing the hack
> bit to ARCH_STATIC_BRANCH_ASM() like so?
Good idea, thanks.
Alice
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-11 10:13 ` [PATCH v10 5/5] rust: add arch_static_branch Alice Ryhl
2024-10-11 15:23 ` Josh Poimboeuf
@ 2024-10-15 11:05 ` kernel test robot
2024-10-15 12:11 ` Miguel Ojeda
1 sibling, 1 reply; 20+ messages in thread
From: kernel test robot @ 2024-10-15 11:05 UTC (permalink / raw)
To: Alice Ryhl, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg
Cc: oe-kbuild-all, linux-trace-kernel, rust-for-linux, linux-kernel,
Arnd Bergmann, linux-arch, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Sean Christopherson, Uros Bizjak, Catalin Marinas, Will Deacon
Hi Alice,
kernel test robot noticed the following build errors:
[auto build test ERROR on eb887c4567d1b0e7684c026fe7df44afa96589e6]
url: https://github.com/intel-lab-lkp/linux/commits/Alice-Ryhl/rust-add-static_branch_unlikely-for-static_key_false/20241011-181703
base: eb887c4567d1b0e7684c026fe7df44afa96589e6
patch link: https://lore.kernel.org/r/20241011-tracepoint-v10-5-7fbde4d6b525%40google.com
patch subject: [PATCH v10 5/5] rust: add arch_static_branch
config: riscv-randconfig-r062-20241015 (https://download.01.org/0day-ci/archive/20241015/202410151814.WmLlAkCq-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241015/202410151814.WmLlAkCq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410151814.WmLlAkCq-lkp@intel.com/
All errors (new ones prefixed by >>):
>> /bin/sh: 1: cannot create rust/kernel/arch_static_branch_asm.rs: Directory nonexistent
make[3]: *** [scripts/Makefile.build:311: rust/kernel/arch_static_branch_asm.rs] Error 2
make[3]: Target 'rust/' not remade because of errors.
make[2]: *** [Makefile:1209: prepare] Error 2
make[1]: *** [Makefile:224: __sub-make] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:224: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-15 11:05 ` kernel test robot
@ 2024-10-15 12:11 ` Miguel Ojeda
2024-10-15 13:06 ` Alice Ryhl
0 siblings, 1 reply; 20+ messages in thread
From: Miguel Ojeda @ 2024-10-15 12:11 UTC (permalink / raw)
To: kernel test robot
Cc: Alice Ryhl, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Ard Biesheuvel,
Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
oe-kbuild-all, linux-trace-kernel, rust-for-linux, linux-kernel,
Arnd Bergmann, linux-arch, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Sean Christopherson, Uros Bizjak, Catalin Marinas, Will Deacon
On Tue, Oct 15, 2024 at 1:06 PM kernel test robot <lkp@intel.com> wrote:
>
> >> /bin/sh: 1: cannot create rust/kernel/arch_static_branch_asm.rs: Directory nonexistent
This happens because we run `RSCPP` even when it is not a target. We
can add the dependency conditionally:
@@ -423,8 +425,10 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
- $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
- $(obj)/kernel/arch_static_branch_asm.rs FORCE
+ $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+$(call if_changed_rule,rustc_library)
+ifneq ($(CONFIG_JUMP_LABEL),)
+$(obj)/kernel.o: $(obj)/kernel/arch_static_branch_asm.rs
+endif
Cheers,
Miguel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-15 12:11 ` Miguel Ojeda
@ 2024-10-15 13:06 ` Alice Ryhl
2024-10-15 14:43 ` Miguel Ojeda
0 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-10-15 13:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: kernel test robot, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, oe-kbuild-all, linux-trace-kernel,
rust-for-linux, linux-kernel, Arnd Bergmann, linux-arch,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Sean Christopherson, Uros Bizjak, Catalin Marinas,
Will Deacon
On Tue, Oct 15, 2024 at 2:11 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Oct 15, 2024 at 1:06 PM kernel test robot <lkp@intel.com> wrote:
> >
> > >> /bin/sh: 1: cannot create rust/kernel/arch_static_branch_asm.rs: Directory nonexistent
>
> This happens because we run `RSCPP` even when it is not a target. We
> can add the dependency conditionally:
>
> @@ -423,8 +425,10 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
> $(obj)/kernel.o: private rustc_target_flags = --extern alloc \
> --extern build_error --extern macros --extern bindings --extern uapi
> $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
> - $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
> - $(obj)/kernel/arch_static_branch_asm.rs FORCE
> + $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
> +$(call if_changed_rule,rustc_library)
> +ifneq ($(CONFIG_JUMP_LABEL),)
> +$(obj)/kernel.o: $(obj)/kernel/arch_static_branch_asm.rs
> +endif
Thank you. I was able to reproduce the error locally. It only happens
when CONFIG_JUMP_LABEL is disabled. I've verified that this fixes it.
Alice
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-15 13:06 ` Alice Ryhl
@ 2024-10-15 14:43 ` Miguel Ojeda
2024-10-15 14:45 ` Alice Ryhl
0 siblings, 1 reply; 20+ messages in thread
From: Miguel Ojeda @ 2024-10-15 14:43 UTC (permalink / raw)
To: Alice Ryhl
Cc: kernel test robot, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, oe-kbuild-all, linux-trace-kernel,
rust-for-linux, linux-kernel, Arnd Bergmann, linux-arch,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Sean Christopherson, Uros Bizjak, Catalin Marinas,
Will Deacon
On Tue, Oct 15, 2024 at 3:07 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Thank you. I was able to reproduce the error locally. It only happens
> when CONFIG_JUMP_LABEL is disabled. I've verified that this fixes it.
You're welcome!
By the way, if you end up sending a new version, then you could
simplify to `ifdef CONFIG_JUMP_LABEL`.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-15 14:43 ` Miguel Ojeda
@ 2024-10-15 14:45 ` Alice Ryhl
2024-10-15 15:31 ` Miguel Ojeda
0 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-10-15 14:45 UTC (permalink / raw)
To: Miguel Ojeda
Cc: kernel test robot, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, oe-kbuild-all, linux-trace-kernel,
rust-for-linux, linux-kernel, Arnd Bergmann, linux-arch,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Sean Christopherson, Uros Bizjak, Catalin Marinas,
Will Deacon
On Tue, Oct 15, 2024 at 4:44 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Oct 15, 2024 at 3:07 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Thank you. I was able to reproduce the error locally. It only happens
> > when CONFIG_JUMP_LABEL is disabled. I've verified that this fixes it.
>
> You're welcome!
>
> By the way, if you end up sending a new version, then you could
> simplify to `ifdef CONFIG_JUMP_LABEL`.
Too late! But I can do that if I send yet another version.
Alice
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v10 5/5] rust: add arch_static_branch
2024-10-15 14:45 ` Alice Ryhl
@ 2024-10-15 15:31 ` Miguel Ojeda
0 siblings, 0 replies; 20+ messages in thread
From: Miguel Ojeda @ 2024-10-15 15:31 UTC (permalink / raw)
To: Alice Ryhl
Cc: kernel test robot, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Ard Biesheuvel, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, oe-kbuild-all, linux-trace-kernel,
rust-for-linux, linux-kernel, Arnd Bergmann, linux-arch,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Sean Christopherson, Uros Bizjak, Catalin Marinas,
Will Deacon
On Tue, Oct 15, 2024 at 4:46 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Too late! But I can do that if I send yet another version.
Yeah, I meant after the one you already sent :)
Cheers,
Miguel
^ permalink raw reply [flat|nested] 20+ messages in thread