* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Benno Lossin @ 2025-07-09 18:22 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
In-Reply-To: <87ple9lkjr.fsf@kernel.org>
On Wed Jul 9, 2025 at 12:34 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
>>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>>> >
>>>>> > Introduce the `SetOnce` type, a container that can only be written once.
>>>>> > The container uses an internal atomic to synchronize writes to the internal
>>>>> > value.
>>>>> >
>>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>>>
>>>>> LGTM:
>>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>>>>
>>>>> > +impl<T> Drop for SetOnce<T> {
>>>>> > + fn drop(&mut self) {
>>>>> > + if self.init.load(Acquire) == 2 {
>>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>>>>> > + // `self`.
>>>>> > + unsafe { drop_in_place(self.value.get()) };
>>>>>
>>>>> This load does not need to be Acquire. It can be a Relaxed load or
>>>>> even an unsynchronized one since the access is exclusive.
>>>>
>>>> Right, I think we can do the similar as Revocable here:
>>>>
>>>> if *self.init.get_mut() == 2 { }
>
> Ok, now I got it. You are saying I don't need to use the atomic load
> method, because I have mutable access. Sounds good.
>
> But I guess a relaxed load and access through a mutable reference should
> result in the same code generation on most (all?) platforms?
AFAIK it is not the same on arm.
---
Cheers,
Benno
^ permalink raw reply
* [PATCH v16 6/7] rust: samples: add a module parameter to the rust_minimal sample
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
Showcase the rust module parameter support by adding a module parameter to
the `rust_minimal` sample.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
samples/rust/rust_minimal.rs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 1fc7a1be6b6d7..8eb9583571d72 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -10,6 +10,12 @@
authors: ["Rust for Linux Contributors"],
description: "Rust minimal sample",
license: "GPL",
+ params: {
+ test_parameter: i64 {
+ default: 1,
+ description: "This parameter has a default of 1",
+ },
+ },
}
struct RustMinimal {
@@ -20,6 +26,10 @@ impl kernel::Module for RustMinimal {
fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Rust minimal sample (init)\n");
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+ pr_info!(
+ "test_parameter: {}\n",
+ *module_parameters::test_parameter.value()
+ );
let mut numbers = KVec::new();
numbers.push(72, GFP_KERNEL)?;
--
2.47.2
^ permalink raw reply related
* [PATCH v16 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
Introduce the `SetOnce` type, a container that can only be written once.
The container uses an internal atomic to synchronize writes to the internal
value.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/sync.rs | 2 +
rust/kernel/sync/set_once.rs | 122 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 124 insertions(+)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 81e3a806e57e2..13e6bc7fa87ac 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -18,6 +18,7 @@
mod locked_by;
pub mod poll;
pub mod rcu;
+mod set_once;
pub use arc::{Arc, ArcBorrow, UniqueArc};
pub use completion::Completion;
@@ -26,6 +27,7 @@
pub use lock::mutex::{new_mutex, Mutex, MutexGuard};
pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
pub use locked_by::LockedBy;
+pub use set_once::SetOnce;
/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
#[repr(transparent)]
diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
new file mode 100644
index 0000000000000..73706abfe9991
--- /dev/null
+++ b/rust/kernel/sync/set_once.rs
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A container that can be initialized at most once.
+
+use super::atomic::{
+ ordering::{Acquire, Relaxed, Release},
+ Atomic,
+};
+use core::{cell::UnsafeCell, mem::MaybeUninit, ptr::drop_in_place};
+
+/// A container that can be populated at most once. Thread safe.
+///
+/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
+/// lifetime `Self`.
+///
+/// # Invariants
+///
+/// - `init` may only increase in value.
+/// - `init` may only assume values in the range `0..=2`.
+/// - `init == 0` if and only if the container is empty.
+/// - `init == 1` if and only if being initialized.
+/// - `init == 2` if and only if the container is populated and valid for shared access.
+///
+/// # Example
+///
+/// ```
+/// # use kernel::sync::SetOnce;
+/// let value = SetOnce::new();
+/// assert_eq!(None, value.as_ref());
+///
+/// let status = value.populate(42u8);
+/// assert_eq!(true, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+///
+/// let status = value.populate(101u8);
+/// assert_eq!(false, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+/// ```
+pub struct SetOnce<T> {
+ init: Atomic<u32>,
+ value: UnsafeCell<MaybeUninit<T>>,
+}
+
+impl<T> Default for SetOnce<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl<T> SetOnce<T> {
+ /// Create a new [`SetOnce`].
+ ///
+ /// The returned instance will be empty.
+ pub const fn new() -> Self {
+ // INVARIANT: The container is empty and we initialize `init` to `0`.
+ Self {
+ value: UnsafeCell::new(MaybeUninit::uninit()),
+ init: Atomic::new(0),
+ }
+ }
+
+ /// Get a reference to the contained object.
+ ///
+ /// Returns [`None`] if this [`SetOnce`] is empty.
+ pub fn as_ref(&self) -> Option<&T> {
+ if self.init.load(Acquire) == 2 {
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // contains a valid value.
+ Some(unsafe { &*self.value.get().cast() })
+ } else {
+ None
+ }
+ }
+
+ /// Populate the [`SetOnce`].
+ ///
+ /// Returns `true` if the [`SetOnce`] was successfully populated.
+ pub fn populate(&self, value: T) -> bool {
+ // INVARIANT: If the swap succeeds:
+ // - We increase `init`.
+ // - We write the valid value `1` to `init`.
+ // - Only one thread can succeed in this write, so we have exclusive access after the
+ // write.
+ if let Ok(0) = self.init.cmpxchg(0, 1, Relaxed) {
+ // SAFETY: By the type invariants of `Self`, the fact that we succeeded in writing `1`
+ // to `self.init` means we obtained exclusive access to the contained object.
+ unsafe { core::ptr::write(self.value.get().cast(), value) };
+ // INVARIANT:
+ // - We increase `init`.
+ // - We write the valid value `2` to `init`.
+ // - We release our exclusive access to the contained object and the object is now
+ // valid for shared access.
+ self.init.store(2, Release);
+ true
+ } else {
+ false
+ }
+ }
+
+ /// Get a copy of the contained object.
+ ///
+ /// Returns [`None`] if the [`SetOnce`] is empty.
+ pub fn copy(&self) -> Option<T>
+ where
+ T: Copy,
+ {
+ self.as_ref().copied()
+ }
+}
+
+impl<T> Drop for SetOnce<T> {
+ fn drop(&mut self) {
+ if *self.init.get_mut() == 2 {
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // contains a valid value. We have exclusive access, as we hold a `mut` reference to
+ // `self`.
+ unsafe { drop_in_place(self.value.get()) };
+ }
+ }
+}
--
2.47.2
^ permalink raw reply related
* [PATCH v16 4/7] rust: module: use a reference in macros::module::module
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
When we add parameter support to the module macro, we want to be able to
pass a reference to `ModuleInfo` to a helper function. That is not possible
when we move out of the local `modinfo`. So change the function to access
the local via reference rather than value.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/module.rs | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 2ddd2eeb28521..1a867a1e787ed 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -179,26 +179,26 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Rust does not allow hyphens in identifiers, use underscore instead.
let ident = info.name.replace('-', "_");
let mut modinfo = ModInfoBuilder::new(ident.as_ref());
- if let Some(author) = info.author {
- modinfo.emit("author", &author);
+ if let Some(author) = &info.author {
+ modinfo.emit("author", author);
}
- if let Some(authors) = info.authors {
+ if let Some(authors) = &info.authors {
for author in authors {
- modinfo.emit("author", &author);
+ modinfo.emit("author", author);
}
}
- if let Some(description) = info.description {
- modinfo.emit("description", &description);
+ if let Some(description) = &info.description {
+ modinfo.emit("description", description);
}
modinfo.emit("license", &info.license);
- if let Some(aliases) = info.alias {
+ if let Some(aliases) = &info.alias {
for alias in aliases {
- modinfo.emit("alias", &alias);
+ modinfo.emit("alias", alias);
}
}
- if let Some(firmware) = info.firmware {
+ if let Some(firmware) = &info.firmware {
for fw in firmware {
- modinfo.emit("firmware", &fw);
+ modinfo.emit("firmware", fw);
}
}
--
2.47.2
^ permalink raw reply related
* [PATCH v16 2/7] rust: str: add radix prefixed integer parsing functions
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Tested-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 150 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index a927db8e079c3..2b6c8b4a0ae4b 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -8,6 +8,8 @@
use crate::prelude::*;
+pub mod parse_int;
+
/// Byte string without UTF-8 validity guarantee.
#[repr(transparent)]
pub struct BStr([u8]);
diff --git a/rust/kernel/str/parse_int.rs b/rust/kernel/str/parse_int.rs
new file mode 100644
index 0000000000000..48eb4c202984c
--- /dev/null
+++ b/rust/kernel/str/parse_int.rs
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Integer parsing functions.
+//!
+//! Integer parsing functions for parsing signed and unsigned integers
+//! potentially prefixed with `0x`, `0o`, or `0b`.
+
+use crate::prelude::*;
+use crate::str::BStr;
+use core::ops::Deref;
+
+// Make `FromStrRadix` a public type with a private name. This seals
+// `ParseInt`, that is, prevents downstream users from implementing the
+// trait.
+mod private {
+ use crate::prelude::*;
+ use crate::str::BStr;
+
+ /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+ pub trait FromStrRadix: Sized {
+ /// Parse `src` to [`Self`] using radix `radix`.
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self>;
+
+ /// Tries to convert `value` into [`Self`] and negates the resulting value.
+ fn from_u64_negated(value: u64) -> Result<Self>;
+ }
+}
+
+/// Extract the radix from an integer literal optionally prefixed with
+/// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+fn strip_radix(src: &BStr) -> (u32, &BStr) {
+ match src.deref() {
+ [b'0', b'x' | b'X', rest @ ..] => (16, rest.as_ref()),
+ [b'0', b'o' | b'O', rest @ ..] => (8, rest.as_ref()),
+ [b'0', b'b' | b'B', rest @ ..] => (2, rest.as_ref()),
+ // NOTE: We are including the leading zero to be able to parse
+ // literal `0` here. If we removed it as a radix prefix, we would
+ // not be able to parse `0`.
+ [b'0', ..] => (8, src),
+ _ => (10, src),
+ }
+}
+
+/// Trait for parsing string representations of integers.
+///
+/// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+/// binary respectively. Strings beginning with `0` otherwise are parsed as
+/// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+/// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+/// successfully parsed.
+///
+/// [`kstrtol()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtol
+/// [`kstrtoul()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtoul
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::str::parse_int::ParseInt;
+/// # use kernel::b_str;
+///
+/// assert_eq!(Ok(0u8), u8::from_str(b_str!("0")));
+///
+/// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+/// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+///
+/// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+/// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+///
+/// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+/// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+///
+/// assert_eq!(Ok(127i8), i8::from_str(b_str!("127")));
+/// assert!(i8::from_str(b_str!("128")).is_err());
+/// assert_eq!(Ok(-128i8), i8::from_str(b_str!("-128")));
+/// assert!(i8::from_str(b_str!("-129")).is_err());
+/// assert_eq!(Ok(255u8), u8::from_str(b_str!("255")));
+/// assert!(u8::from_str(b_str!("256")).is_err());
+/// ```
+pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
+ /// Parse a string according to the description in [`Self`].
+ fn from_str(src: &BStr) -> Result<Self> {
+ match src.deref() {
+ [b'-', rest @ ..] => {
+ let (radix, digits) = strip_radix(rest.as_ref());
+ // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
+ // So if we want to parse negative numbers as positive and
+ // later multiply by -1, we have to parse into a larger
+ // integer. We choose `u64` as sufficiently large.
+ //
+ // NOTE: 128 bit integers are not available on all
+ // platforms, hence the choice of 64 bits.
+ let val =
+ u64::from_str_radix(core::str::from_utf8(digits).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)?;
+ Self::from_u64_negated(val)
+ }
+ _ => {
+ let (radix, digits) = strip_radix(src);
+ Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+ }
+ }
+ }
+}
+
+macro_rules! impl_parse_int {
+ ($($ty:ty),*) => {
+ $(
+ impl private::FromStrRadix for $ty {
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self> {
+ <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)
+ }
+
+ fn from_u64_negated(value: u64) -> Result<Self> {
+ const ABS_MIN: u64 = {
+ #[allow(unused_comparisons)]
+ if <$ty>::MIN < 0 {
+ 1u64 << (<$ty>::BITS - 1)
+ } else {
+ 0
+ }
+ };
+
+ if value > ABS_MIN {
+ return Err(EINVAL);
+ }
+
+ if value == ABS_MIN {
+ return Ok(<$ty>::MIN);
+ }
+
+ // SAFETY: The above checks guarantee that `value` fits into `Self`:
+ // - if `Self` is unsigned, then `ABS_MIN == 0` and thus we have returned above
+ // (either `EINVAL` or `MIN`).
+ // - if `Self` is signed, then we have that `0 <= value < ABS_MIN`. And since
+ // `ABS_MIN - 1` fits into `Self` by construction, `value` also does.
+ let value: Self = unsafe { value.try_into().unwrap_unchecked() };
+
+ Ok((!value).wrapping_add(1))
+ }
+ }
+
+ impl ParseInt for $ty {}
+ )*
+ };
+}
+
+impl_parse_int![i8, u8, i16, u16, i32, u32, i64, u64, isize, usize];
--
2.47.2
^ permalink raw reply related
* [PATCH v16 7/7] modules: add rust modules files to MAINTAINERS
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
The module subsystem people agreed to maintain rust support for modules
[1]. Thus, add entries for relevant files to modules entry in MAINTAINERS.
Link: https://lore.kernel.org/all/0d9e596a-5316-4e00-862b-fd77552ae4b5@suse.com/ [1]
Acked-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d431320ed3b2a..afa385ecc5c4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16835,6 +16835,8 @@ F: include/linux/module*.h
F: kernel/module/
F: lib/test_kmod.c
F: lib/tests/module/
+F: rust/kernel/module_param.rs
+F: rust/macros/module.rs
F: scripts/module*
F: tools/testing/selftests/kmod/
F: tools/testing/selftests/module/
--
2.47.2
^ permalink raw reply related
* [PATCH v16 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
Add types and traits for interfacing the C moduleparam API.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 181 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c37..2b439ea061850 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -87,6 +87,7 @@
pub mod list;
pub mod miscdevice;
pub mod mm;
+pub mod module_param;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod of;
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
new file mode 100644
index 0000000000000..9b187ed1d3513
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for module parameters.
+//!
+//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
+
+use crate::prelude::*;
+use crate::str::BStr;
+use bindings;
+use kernel::sync::SetOnce;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct KernelParam(bindings::kernel_param);
+
+impl KernelParam {
+ #[doc(hidden)]
+ pub const fn new(val: bindings::kernel_param) -> Self {
+ Self(val)
+ }
+}
+
+// SAFETY: C kernel handles serializing access to this type. We never access it
+// from Rust module.
+unsafe impl Sync for KernelParam {}
+
+/// Types that can be used for module parameters.
+// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
+pub trait ModuleParam: Sized + Copy {
+ /// Parse a parameter argument into the parameter value.
+ fn try_from_param_arg(arg: &BStr) -> Result<Self>;
+}
+
+/// Set the module parameter from a string.
+///
+/// Used to set the parameter value at kernel initialization, when loading
+/// the module or when set through `sysfs`.
+///
+/// See `struct kernel_param_ops.set`.
+///
+/// # Safety
+///
+/// - If `val` is non-null then it must point to a valid null-terminated string that must be valid
+/// for reads for the duration of the call.
+/// - `param` must be a pointer to a `bindings::kernel_param` initialized by the rust module macro.
+/// The pointee must be valid for reads for the duration of the call.
+///
+/// # Note
+///
+/// - The safety requirements are satisfied by C API contract when this function is invoked by the
+/// module subsystem C code.
+/// - Currently, we only support read-only parameters that are not readable from `sysfs`. Thus, this
+/// function is only called at kernel initialization time, or at module load time, and we have
+/// exclusive access to the parameter for the duration of the function.
+///
+/// [`module!`]: macros::module
+unsafe extern "C" fn set_param<T>(val: *const c_char, param: *const bindings::kernel_param) -> c_int
+where
+ T: ModuleParam,
+{
+ // NOTE: If we start supporting arguments without values, val _is_ allowed
+ // to be null here.
+ if val.is_null() {
+ // TODO: Use pr_warn_once available.
+ crate::pr_warn!("Null pointer passed to `module_param::set_param`");
+ return EINVAL.to_errno();
+ }
+
+ // SAFETY: By function safety requirement, val is non-null, null-terminated
+ // and valid for reads for the duration of this function.
+ let arg = unsafe { CStr::from_char_ptr(val) };
+
+ crate::error::from_result(|| {
+ let new_value = T::try_from_param_arg(arg)?;
+
+ // SAFETY: By function safety requirements, this access is safe.
+ let container = unsafe { &*((*param).__bindgen_anon_1.arg as *mut SetOnce<T>) };
+
+ container
+ .populate(new_value)
+ .then_some(0)
+ .ok_or(kernel::error::code::EEXIST)
+ })
+}
+
+macro_rules! impl_int_module_param {
+ ($ty:ident) => {
+ impl ModuleParam for $ty {
+ fn try_from_param_arg(arg: &BStr) -> Result<Self> {
+ <$ty as crate::str::parse_int::ParseInt>::from_str(arg)
+ }
+ }
+ };
+}
+
+impl_int_module_param!(i8);
+impl_int_module_param!(u8);
+impl_int_module_param!(i16);
+impl_int_module_param!(u16);
+impl_int_module_param!(i32);
+impl_int_module_param!(u32);
+impl_int_module_param!(i64);
+impl_int_module_param!(u64);
+impl_int_module_param!(isize);
+impl_int_module_param!(usize);
+
+/// A wrapper for kernel parameters.
+///
+/// This type is instantiated by the [`module!`] macro when module parameters are
+/// defined. You should never need to instantiate this type directly.
+///
+/// Note: This type is `pub` because it is used by module crates to access
+/// parameter values.
+pub struct ModuleParamAccess<T> {
+ value: SetOnce<T>,
+ default: T,
+}
+
+// SAFETY: We only create shared references to the contents of this container,
+// so if `T` is `Sync`, so is `ModuleParamAccess`.
+unsafe impl<T: Sync> Sync for ModuleParamAccess<T> {}
+
+impl<T> ModuleParamAccess<T> {
+ #[doc(hidden)]
+ pub const fn new(default: T) -> Self {
+ Self {
+ value: SetOnce::new(),
+ default,
+ }
+ }
+
+ /// Get a shared reference to the parameter value.
+ // Note: When sysfs access to parameters are enabled, we have to pass in a
+ // held lock guard here.
+ pub fn value(&self) -> &T {
+ self.value.as_ref().unwrap_or(&self.default)
+ }
+
+ /// Get a mutable pointer to `self`.
+ ///
+ /// NOTE: In most cases it is not safe deref the returned pointer.
+ pub const fn as_void_ptr(&self) -> *mut c_void {
+ core::ptr::from_ref(self).cast_mut().cast()
+ }
+}
+
+#[doc(hidden)]
+/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
+///
+/// # Examples
+///
+/// ```ignore
+/// make_param_ops!(
+/// /// Documentation for new param ops.
+/// PARAM_OPS_MYTYPE, // Name for the static.
+/// MyType // A type which implements [`ModuleParam`].
+/// );
+/// ```
+macro_rules! make_param_ops {
+ ($ops:ident, $ty:ty) => {
+ #[doc(hidden)]
+ pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
+ flags: 0,
+ set: Some(set_param::<$ty>),
+ get: None,
+ free: None,
+ };
+ };
+}
+
+make_param_ops!(PARAM_OPS_I8, i8);
+make_param_ops!(PARAM_OPS_U8, u8);
+make_param_ops!(PARAM_OPS_I16, i16);
+make_param_ops!(PARAM_OPS_U16, u16);
+make_param_ops!(PARAM_OPS_I32, i32);
+make_param_ops!(PARAM_OPS_U32, u32);
+make_param_ops!(PARAM_OPS_I64, i64);
+make_param_ops!(PARAM_OPS_U64, u64);
+make_param_ops!(PARAM_OPS_ISIZE, isize);
+make_param_ops!(PARAM_OPS_USIZE, usize);
--
2.47.2
^ permalink raw reply related
* [PATCH v16 5/7] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org>
Allow module parameters to be declared in the rust `module!` macro.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/helpers.rs | 25 +++++++
rust/macros/lib.rs | 31 +++++++++
rust/macros/module.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 224 insertions(+), 10 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c10..365d7eb499c08 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -10,6 +10,17 @@ pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
}
}
+pub(crate) fn try_sign(it: &mut token_stream::IntoIter) -> Option<char> {
+ let peek = it.clone().next();
+ match peek {
+ Some(TokenTree::Punct(punct)) if punct.as_char() == '-' => {
+ let _ = it.next();
+ Some(punct.as_char())
+ }
+ _ => None,
+ }
+}
+
pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
if let Some(TokenTree::Literal(literal)) = it.next() {
Some(literal.to_string())
@@ -103,3 +114,17 @@ pub(crate) fn file() -> String {
proc_macro::Span::call_site().file()
}
}
+
+/// Parse a token stream of the form `expected_name: "value",` and return the
+/// string in the position of "value".
+///
+/// # Panics
+///
+/// - On parse error.
+pub(crate) fn expect_string_field(it: &mut token_stream::IntoIter, expected_name: &str) -> String {
+ assert_eq!(expect_ident(it), expected_name);
+ assert_eq!(expect_punct(it), ':');
+ let string = expect_string(it);
+ assert_eq!(expect_punct(it), ',');
+ string
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index fa847cf3a9b5f..2fb520dc930af 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -28,6 +28,30 @@
/// The `type` argument should be a type which implements the [`Module`]
/// trait. Also accepts various forms of kernel metadata.
///
+/// The `params` field describe module parameters. Each entry has the form
+///
+/// ```ignore
+/// parameter_name: type {
+/// default: default_value,
+/// description: "Description",
+/// }
+/// ```
+///
+/// `type` may be one of
+///
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i16`]
+/// - [`u16`]
+/// - [`i32`]
+/// - [`u32`]
+/// - [`i64`]
+/// - [`u64`]
+/// - [`isize`]
+/// - [`usize`]
+///
/// C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
///
/// [`Module`]: ../kernel/trait.Module.html
@@ -44,6 +68,12 @@
/// description: "My very own kernel module!",
/// license: "GPL",
/// alias: ["alternate_module_name"],
+/// params: {
+/// my_parameter: i64 {
+/// default: 1,
+/// description: "This parameter has a default of 1",
+/// },
+/// },
/// }
///
/// struct MyModule(i32);
@@ -52,6 +82,7 @@
/// fn init(_module: &'static ThisModule) -> Result<Self> {
/// let foo: i32 = 42;
/// pr_info!("I contain: {}\n", foo);
+/// pr_info!("i32 param is: {}\n", module_parameters::my_parameter.read());
/// Ok(Self(foo))
/// }
/// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 1a867a1e787ed..c1400597774a5 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -26,6 +26,7 @@ struct ModInfoBuilder<'a> {
module: &'a str,
counter: usize,
buffer: String,
+ param_buffer: String,
}
impl<'a> ModInfoBuilder<'a> {
@@ -34,10 +35,11 @@ fn new(module: &'a str) -> Self {
module,
counter: 0,
buffer: String::new(),
+ param_buffer: String::new(),
}
}
- fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
+ fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool) {
let string = if builtin {
// Built-in modules prefix their modinfo strings by `module.`.
format!(
@@ -51,8 +53,14 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
format!("{field}={content}\0")
};
+ let buffer = if param {
+ &mut self.param_buffer
+ } else {
+ &mut self.buffer
+ };
+
write!(
- &mut self.buffer,
+ buffer,
"
{cfg}
#[doc(hidden)]
@@ -75,20 +83,119 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
self.counter += 1;
}
- fn emit_only_builtin(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, true)
+ fn emit_only_builtin(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, true, param)
}
- fn emit_only_loadable(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, false)
+ fn emit_only_loadable(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, false, param)
}
fn emit(&mut self, field: &str, content: &str) {
- self.emit_only_builtin(field, content);
- self.emit_only_loadable(field, content);
+ self.emit_internal(field, content, false);
+ }
+
+ fn emit_internal(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_only_builtin(field, content, param);
+ self.emit_only_loadable(field, content, param);
+ }
+
+ fn emit_param(&mut self, field: &str, param: &str, content: &str) {
+ let content = format!("{param}:{content}", param = param, content = content);
+ self.emit_internal(field, &content, true);
+ }
+
+ fn emit_params(&mut self, info: &ModuleInfo) {
+ let Some(params) = &info.params else {
+ return;
+ };
+
+ for param in params {
+ let ops = param_ops_path(¶m.ptype);
+
+ // Note: The spelling of these fields is dictated by the user space
+ // tool `modinfo`.
+ self.emit_param("parmtype", ¶m.name, ¶m.ptype);
+ self.emit_param("parm", ¶m.name, ¶m.description);
+
+ write!(
+ self.param_buffer,
+ "
+ pub(crate) static {param_name}:
+ ::kernel::module_param::ModuleParamAccess<{param_type}> =
+ ::kernel::module_param::ModuleParamAccess::new({param_default});
+
+ const _: () = {{
+ #[link_section = \"__param\"]
+ #[used]
+ static __{module_name}_{param_name}_struct:
+ ::kernel::module_param::KernelParam =
+ ::kernel::module_param::KernelParam::new(
+ ::kernel::bindings::kernel_param {{
+ name: if ::core::cfg!(MODULE) {{
+ ::kernel::c_str!(\"{param_name}\").as_bytes_with_nul()
+ }} else {{
+ ::kernel::c_str!(\"{module_name}.{param_name}\")
+ .as_bytes_with_nul()
+ }}.as_ptr(),
+ // SAFETY: `__this_module` is constructed by the kernel at load
+ // time and will not be freed until the module is unloaded.
+ #[cfg(MODULE)]
+ mod_: unsafe {{
+ core::ptr::from_ref(&::kernel::bindings::__this_module)
+ .cast_mut()
+ }},
+ #[cfg(not(MODULE))]
+ mod_: ::core::ptr::null_mut(),
+ ops: core::ptr::from_ref(&{ops}),
+ perm: 0, // Will not appear in sysfs
+ level: -1,
+ flags: 0,
+ __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
+ arg: {param_name}.as_void_ptr()
+ }},
+ }}
+ );
+ }};
+ ",
+ module_name = info.name,
+ param_type = param.ptype,
+ param_default = param.default,
+ param_name = param.name,
+ ops = ops,
+ )
+ .unwrap();
+ }
+ }
+}
+
+fn param_ops_path(param_type: &str) -> &'static str {
+ match param_type {
+ "i8" => "::kernel::module_param::PARAM_OPS_I8",
+ "u8" => "::kernel::module_param::PARAM_OPS_U8",
+ "i16" => "::kernel::module_param::PARAM_OPS_I16",
+ "u16" => "::kernel::module_param::PARAM_OPS_U16",
+ "i32" => "::kernel::module_param::PARAM_OPS_I32",
+ "u32" => "::kernel::module_param::PARAM_OPS_U32",
+ "i64" => "::kernel::module_param::PARAM_OPS_I64",
+ "u64" => "::kernel::module_param::PARAM_OPS_U64",
+ "isize" => "::kernel::module_param::PARAM_OPS_ISIZE",
+ "usize" => "::kernel::module_param::PARAM_OPS_USIZE",
+ t => panic!("Unsupported parameter type {}", t),
}
}
+fn expect_param_default(param_it: &mut token_stream::IntoIter) -> String {
+ assert_eq!(expect_ident(param_it), "default");
+ assert_eq!(expect_punct(param_it), ':');
+ let sign = try_sign(param_it);
+ let default = try_literal(param_it).expect("Expected default param value");
+ assert_eq!(expect_punct(param_it), ',');
+ let mut value = sign.map(String::from).unwrap_or_default();
+ value.push_str(&default);
+ value
+}
+
#[derive(Debug, Default)]
struct ModuleInfo {
type_: String,
@@ -99,6 +206,50 @@ struct ModuleInfo {
description: Option<String>,
alias: Option<Vec<String>>,
firmware: Option<Vec<String>>,
+ params: Option<Vec<Parameter>>,
+}
+
+#[derive(Debug)]
+struct Parameter {
+ name: String,
+ ptype: String,
+ default: String,
+ description: String,
+}
+
+fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
+ let params = expect_group(it);
+ assert_eq!(params.delimiter(), Delimiter::Brace);
+ let mut it = params.stream().into_iter();
+ let mut parsed = Vec::new();
+
+ loop {
+ let param_name = match it.next() {
+ Some(TokenTree::Ident(ident)) => ident.to_string(),
+ Some(_) => panic!("Expected Ident or end"),
+ None => break,
+ };
+
+ assert_eq!(expect_punct(&mut it), ':');
+ let param_type = expect_ident(&mut it);
+ let group = expect_group(&mut it);
+ assert_eq!(group.delimiter(), Delimiter::Brace);
+ assert_eq!(expect_punct(&mut it), ',');
+
+ let mut param_it = group.stream().into_iter();
+ let param_default = expect_param_default(&mut param_it);
+ let param_description = expect_string_field(&mut param_it, "description");
+ expect_end(&mut param_it);
+
+ parsed.push(Parameter {
+ name: param_name,
+ ptype: param_type,
+ default: param_default,
+ description: param_description,
+ })
+ }
+
+ parsed
}
impl ModuleInfo {
@@ -114,6 +265,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license",
"alias",
"firmware",
+ "params",
];
const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
let mut seen_keys = Vec::new();
@@ -140,6 +292,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license" => info.license = expect_string_ascii(it),
"alias" => info.alias = Some(expect_string_array(it)),
"firmware" => info.firmware = Some(expect_string_array(it)),
+ "params" => info.params = Some(expect_params(it)),
_ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
}
@@ -205,7 +358,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Built-in modules also export the `file` modinfo string.
let file =
std::env::var("RUST_MODFILE").expect("Unable to fetch RUST_MODFILE environmental variable");
- modinfo.emit_only_builtin("file", &file);
+ modinfo.emit_only_builtin("file", &file, false);
+
+ modinfo.emit_params(&info);
format!(
"
@@ -369,15 +524,18 @@ unsafe fn __exit() {{
__MOD.assume_init_drop();
}}
}}
-
{modinfo}
}}
}}
+ mod module_parameters {{
+ {params}
+ }}
",
type_ = info.type_,
name = info.name,
ident = ident,
modinfo = modinfo.buffer,
+ params = modinfo.param_buffer,
initcall_section = ".initcall6.init"
)
.parse()
--
2.47.2
^ permalink raw reply related
* [PATCH v16 0/7] rust: extend `module!` macro with integer parameter support
From: Andreas Hindborg @ 2025-07-09 17:52 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Extend the `module!` macro with support module parameters. Also add some
string to integer parsing functions.
Based on the original module parameter support by Miguel [1],
later extended and generalized by Adam for more types [2][3].
Originally tracked at [4].
Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v16:
- Normalize imports in `set_once.rs`.
- Use `UnsafeCell<MaybeUninit<T>>` rather than `Opaque<T>` for `SetOnce`.
- Use regular load in drop of `SetOnce`.
- Update attribution paragraph in cover letter with details from Miguel.
- Remove stray TODO in `set_once.rs`
- Link to v15: https://lore.kernel.org/r/20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org
Changes in v15:
- Rebase on v6.16-rc5.
- Dedent code in module macro for better formatting.
- Rename `OnceLock` to `SetOnce`.
- Use "being initialized" rather than "being mutably accessed" when
describing initialization state of `OnceLock`.
- Use `Relaxed` ordering when transitioning to exclusive access in
`OnceLock`.
- Add drop implementation for `OnceLock`.
- Re-export `OnceLock` from `kernel::sync` module.
- Improve indentation of in macro code. Prefix `cfg` to `::core::cfg` in
macro code.
- Use `core::ptr::from_ref` rather than `as` casts.
- Hide `KernelParam` instances behind `const _: ()` blocks.
- Rename `ModuleParamAccess::get` to `ModuleParamAccess::value`.
- Rename `RacyKernelParam` to `KernelParam`.
- Remove `ModuleParam::Value`.
- Move `copy` implementation of `OnceLock`.
- Update safety comments and invariants of `OnceLock`.
- Link to v14: https://lore.kernel.org/r/20250702-module-params-v3-v14-0-5b1cc32311af@kernel.org
Note: This series now depends on the atomics series [1].
[1] https://lore.kernel.org/all/20250618164934.19817-1-boqun.feng@gmail.com
Changes in v14:
- Remove unnecessary `crate::` prefix from `module_param::set_param`.
- Make `FromStrRadix` safe again by moving unsafe blocks to macro implementation (thanks Benno).
- Use `core::ptr::write` in `set_param` and drop safety requirement regarding initialization.
- Add a TODO to use `SyncUnsafeCell` for `ModuleParamAccess` when available.
- Add a NOTE regarding `Copy` bound on `ModuleParam`.
- Remove `'static` lifetime qualifier from `ModuleParam::try_from_param_arg` argument.
- Fix a typo in the safety requirements for `set_param`.
- Remove unused `#[macro_export]` attribute.
- Remove obsolete documentation for `ModuleParam::try_from_param_arg`.
- Make `RacyKernelParam` tuple field private.
- Introduce `OnceLock` and use that to synchronize population of parameter values.
- Link to v13: https://lore.kernel.org/r/20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org
Changes in v13:
- remove absolute path for `ffi` types.
- Split patch 2 into 4 separate patches.
- Overhaul safety framework for `set_param`.
- Remove generated docs for `kernel_param_ops`.
- Move `parse_int` to separate file.
- Rebase on v6.16-rc1
- Link to v12: https://lore.kernel.org/r/20250506-module-params-v3-v12-0-c04d80c8a2b1@kernel.org
Changes in v12:
- Assign through pointer rather than using `core::ptr::replace`.
- Prevent a potential use-after-free during module teardown.
- Link to v11: https://lore.kernel.org/r/20250502-module-params-v3-v11-0-6096875a2b78@kernel.org
Changes in v11:
- Apply a few nits from Miguel.
- Link to v10: https://lore.kernel.org/r/20250501-module-params-v3-v10-0-4da485d343d5@kernel.org
Changes in v10:
- Apply fixups from Miguel:
- Add integer type suffixes to `assert!` in tests.
- Fix links to docs.kernel.org.
- Applyy markdown and intra-doc links where possible.
- Change to `///` for `mod` docs.
- Slightly reword a comment.
- Pluralize "Examples" section name.
- Hide `use`s in example.
- Removed `#[expect]` for the `rusttest` target.
- Link to v9: https://lore.kernel.org/r/20250321-module-params-v3-v9-0-28b905f2e345@kernel.org
Changes in v9:
- Remove UB when parsing the minimum integer values.
- Make `FromStr` trait unsafe, since wrong implementations can cause UB.
- Drop patches that were applied to rust-next.
- Link to v8: https://lore.kernel.org/r/20250227-module-params-v3-v8-0-ceeee85d9347@kernel.org
Changes in v8:
- Change print statement in sample to better communicate parameter name.
- Use imperative mode in commit messages.
- Remove prefix path from `EINVAL`.
- Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
- Parse integers without 128 bit integer types.
- Seal trait `FromStrRadix`.
- Strengthen safety requirement of `set_param`.
- Remove comment about Display and `PAGE_SIZE`.
- Add note describing why `ModuleParamAccess` is pub.
- Typo and grammar fixes for documentation.
- Update MAINTAINERS with rust module files.
- Link to v7: https://lore.kernel.org/r/20250218-module-params-v3-v7-0-5e1afabcac1b@kernel.org
Changes in v7:
- Remove dependency on `pr_warn_once` patches, replace with TODO.
- Rework `ParseInt::from_str` to avoid allocating.
- Add a comment explaining how we parse "0".
- Change trait bound on `Index` impl for `BStr` to match std library approach.
- Link to v6: https://lore.kernel.org/r/20250211-module-params-v3-v6-0-24b297ddc43d@kernel.org
Changes in v6:
- Fix a bug that prevented parsing of negative default values for
parameters in the `module!` macro.
- Fix a bug that prevented parsing zero in `strip_radix`. Also add a
test case for this.
- Add `AsRef<BStr>` for `[u8]` and `BStr`.
- Use `impl AsRef<BStr>` as type of prefix in `BStr::strip_prefix`.
- Link to v5: https://lore.kernel.org/r/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org
Changes in v5:
- Fix a typo in a safety comment in `set_param`.
- Use a match statement in `parse_int::strip_radix`.
- Add an implementation of `Index` for `BStr`.
- Fix a logic inversion bug where parameters would not be parsed.
- Use `kernel::ffi::c_char` in `set_param` rather than the one in `core`.
- Use `kernel::c_str!` rather than `c"..."` literal in module macro.
- Rebase on v6.14-rc1.
- Link to v4: https://lore.kernel.org/r/20250109-module-params-v3-v4-0-c208bcfbe11f@kernel.org
Changes in v4:
- Add module maintainers to Cc list (sorry)
- Add a few missing [`doc_links`]
- Add panic section to `expect_string_field`
- Fix a typo in safety requirement of `module_params::free`
- Change `assert!` to `pr_warn_once!` in `module_params::set_param`
- Remove `module_params::get_param` and install null pointer instead
- Remove use of the unstable feature `sync_unsafe_cell`
- Link to v3: https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org
Changes in v3:
- use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
- remove `Display` bound from `ModuleParam`
- automatically generate documentation for `PARAM_OPS_.*`
- remove `as *const _ as *mut_` phrasing
- inline parameter name in struct instantiation in `emit_params`
- move `RacyKernelParam` out of macro template
- use C string literals rather than byte string literals with explicit null
- template out `__{name}_{param_name}` in `emit_param`
- indent template in `emit_params`
- use let-else expression in `emit_params` to get rid of an indentation level
- document `expect_string_field`
- move invication of `impl_int_module_param` to be closer to macro def
- move attributes after docs in `make_param_ops`
- rename `impl_module_param` to impl_int_module_param`
- use `ty` instead of `ident` in `impl_parse_int`
- use `BStr` instead of `&str` for string manipulation
- move string parsing functions to seperate patch and add examples, fix bugs
- degrade comment about future support from doc comment to regular comment
- remove std lib path from `Sized` marker
- update documentation for `trait ModuleParam`
- Link to v2: https://lore.kernel.org/all/20240819133345.3438739-1-nmi@metaspace.dk/
Changes in v2:
- Remove support for params without values (`NOARG_ALLOWED`).
- Improve documentation for `try_from_param_arg`.
- Use prelude import.
- Refactor `try_from_param_arg` to return `Result`.
- Refactor `ParseInt::from_str` to return `Result`.
- Move C callable functions out of `ModuleParam` trait.
- Rename literal string field parser to `expect_string_field`.
- Move parameter parsing from generation to parsing stage.
- Use absolute type paths in macro code.
- Inline `kparam`and `read_func` values.
- Resolve TODO regarding alignment attributes.
- Remove unnecessary unsafe blocks in macro code.
- Improve error message for unrecognized parameter types.
- Do not use `self` receiver when reading parameter value.
- Add parameter documentation to `module!` macro.
- Use empty `enum` for parameter type.
- Use `addr_of_mut` to get address of parameter value variable.
- Enabled building of docs for for `module_param` module.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-nmi@metaspace.dk/
---
Andreas Hindborg (7):
rust: sync: add `SetOnce`
rust: str: add radix prefixed integer parsing functions
rust: introduce module_param module
rust: module: use a reference in macros::module::module
rust: module: update the module macro with module parameter support
rust: samples: add a module parameter to the rust_minimal sample
modules: add rust modules files to MAINTAINERS
MAINTAINERS | 2 +
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 181 +++++++++++++++++++++++++++++++++++++++
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 148 ++++++++++++++++++++++++++++++++
rust/kernel/sync.rs | 2 +
rust/kernel/sync/set_once.rs | 122 ++++++++++++++++++++++++++
rust/macros/helpers.rs | 25 ++++++
rust/macros/lib.rs | 31 +++++++
rust/macros/module.rs | 198 ++++++++++++++++++++++++++++++++++++++-----
samples/rust/rust_minimal.rs | 10 +++
11 files changed, 702 insertions(+), 20 deletions(-)
---
base-commit: 47633099a672fc7bfe604ef454e4f116e2c954b1
change-id: 20241211-module-params-v3-ae7e5c8d8b5a
prerequisite-message-id: 20250618164934.19817-1-boqun.feng@gmail.com
prerequisite-patch-id: 997f74bd63d81dc66725d7f84ffcf5a3517da1eb
prerequisite-patch-id: 3e3f8bdada027406a356a2b35aebe13edbda0a94
prerequisite-patch-id: 468b093532aa503c23302142b8cfe7989e3cb8f1
prerequisite-patch-id: 96ca162a1e95a29d3aca2e748d600a31417b83e5
prerequisite-patch-id: 8f78abede975c0c9690ee0f4cf4e7af3cb145191
prerequisite-patch-id: a716ee7a51184d224ef60812ecfffadb51980153
prerequisite-patch-id: e54f74896a75147b66e509c1281804047d41f08b
prerequisite-patch-id: 3f77e2179901ae664a2d2e58965a9725ad13189e
prerequisite-patch-id: e964a9576f9c3970bb7a34b4d2891ffb92939d0e
prerequisite-patch-id: bba35ecdf783653e0fa46015bc289706508fac55
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply
* [PATCH v2 8/8] x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
For the most part ftrace uses text poking and can handle ROX memory.
The only place that requires writable memory is create_trampoline() that
updates the allocated memory and in the end makes it ROX.
Use execmem_alloc_rw() in x86::ftrace::alloc_tramp() and enable ROX cache
for EXECMEM_FTRACE when configuration and CPU features allow that.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/ftrace.c | 2 +-
arch/x86/mm/init.c | 9 ++++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 252e82bcfd2f..4450acec9390 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -263,7 +263,7 @@ void arch_ftrace_update_code(int command)
static inline void *alloc_tramp(unsigned long size)
{
- return execmem_alloc(EXECMEM_FTRACE, size);
+ return execmem_alloc_rw(EXECMEM_FTRACE, size);
}
static inline void tramp_free(void *tramp)
{
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 442fafd8ff52..bb57e93b4caf 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1105,7 +1105,14 @@ struct execmem_info __init *execmem_arch_setup(void)
.pgprot = PAGE_KERNEL_ROX,
.alignment = MODULE_ALIGN,
},
- [EXECMEM_FTRACE ... EXECMEM_BPF] = {
+ [EXECMEM_FTRACE] = {
+ .flags = flags,
+ .start = start,
+ .end = MODULES_END,
+ .pgprot = pgprot,
+ .alignment = MODULE_ALIGN,
+ },
+ [EXECMEM_BPF] = {
.flags = EXECMEM_KASAN_SHADOW,
.start = start,
.end = MODULES_END,
--
2.47.2
^ permalink raw reply related
* [PATCH v2 7/8] x86/kprobes: enable EXECMEM_ROX_CACHE for kprobes allocations
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
x86::alloc_insn_page() always allocates ROX memory.
Instead of overriding this method, add EXECMEM_KPROBES entry in
execmem_info with pgprot set to PAGE_KERNEL_ROX and use ROX cache when
configuration and CPU features allow it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/kprobes/core.c | 18 ------------------
arch/x86/mm/init.c | 9 ++++++++-
2 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 47cb8eb138ba..6079d15dab8c 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -481,24 +481,6 @@ static int prepare_singlestep(kprobe_opcode_t *buf, struct kprobe *p,
return len;
}
-/* Make page to RO mode when allocate it */
-void *alloc_insn_page(void)
-{
- void *page;
-
- page = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
- if (!page)
- return NULL;
-
- /*
- * TODO: Once additional kernel code protection mechanisms are set, ensure
- * that the page was not maliciously altered and it is still zeroed.
- */
- set_memory_rox((unsigned long)page, 1);
-
- return page;
-}
-
/* Kprobe x86 instruction emulation - only regs->ip or IF flag modifiers */
static void kprobe_emulate_ifmodifiers(struct kprobe *p, struct pt_regs *regs)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index dbc63f0d538f..442fafd8ff52 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1098,7 +1098,14 @@ struct execmem_info __init *execmem_arch_setup(void)
.pgprot = pgprot,
.alignment = MODULE_ALIGN,
},
- [EXECMEM_KPROBES ... EXECMEM_BPF] = {
+ [EXECMEM_KPROBES] = {
+ .flags = flags,
+ .start = start,
+ .end = MODULES_END,
+ .pgprot = PAGE_KERNEL_ROX,
+ .alignment = MODULE_ALIGN,
+ },
+ [EXECMEM_FTRACE ... EXECMEM_BPF] = {
.flags = EXECMEM_KASAN_SHADOW,
.start = start,
.end = MODULES_END,
--
2.47.2
^ permalink raw reply related
* [PATCH v2 6/8] execmem: drop writable parameter from execmem_fill_trapping_insns()
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
After update of execmem_cache_free() that made memory writable before
updating it, there is no need to update read only memory, so the writable
parameter to execmem_fill_trapping_insns() is not needed. Drop it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/mm/init.c | 8 ++------
include/linux/execmem.h | 3 +--
mm/execmem.c | 4 ++--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 7456df985d96..dbc63f0d538f 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -1063,13 +1063,9 @@ unsigned long arch_max_swapfile_size(void)
static struct execmem_info execmem_info __ro_after_init;
#ifdef CONFIG_ARCH_HAS_EXECMEM_ROX
-void execmem_fill_trapping_insns(void *ptr, size_t size, bool writeable)
+void execmem_fill_trapping_insns(void *ptr, size_t size)
{
- /* fill memory with INT3 instructions */
- if (writeable)
- memset(ptr, INT3_INSN_OPCODE, size);
- else
- text_poke_set(ptr, INT3_INSN_OPCODE, size);
+ memset(ptr, INT3_INSN_OPCODE, size);
}
#endif
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 4e510d1c609c..fe367bdadc3e 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -60,12 +60,11 @@ enum execmem_range_flags {
* will trap
* @ptr: pointer to memory to fill
* @size: size of the range to fill
- * @writable: is the memory poited by @ptr is writable or ROX
*
* A hook for architecures to fill execmem ranges with invalid instructions.
* Architectures that use EXECMEM_ROX_CACHE must implement this.
*/
-void execmem_fill_trapping_insns(void *ptr, size_t size, bool writable);
+void execmem_fill_trapping_insns(void *ptr, size_t size);
/**
* execmem_restore_rox - restore read-only-execute permissions
diff --git a/mm/execmem.c b/mm/execmem.c
index 04c35c3a9361..0822305413ec 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -304,7 +304,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
goto err_free_mem;
/* fill memory with instructions that will trap */
- execmem_fill_trapping_insns(p, alloc_size, /* writable = */ true);
+ execmem_fill_trapping_insns(p, alloc_size);
err = set_memory_rox((unsigned long)p, vm->nr_pages);
if (err)
@@ -363,7 +363,7 @@ static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
if (err)
return err;
- execmem_fill_trapping_insns(ptr, size, /* writable = */ true);
+ execmem_fill_trapping_insns(ptr, size);
execmem_restore_rox(ptr, size);
err = execmem_cache_add_locked(ptr, size, gfp_mask);
--
2.47.2
^ permalink raw reply related
* [PATCH v2 5/8] execmem: add fallback for failures in vmalloc(VM_ALLOW_HUGE_VMAP)
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
When execmem populates ROX cache it uses vmalloc(VM_ALLOW_HUGE_VMAP).
Although vmalloc falls back to allocating base pages if high order
allocation fails, it may happen that it still cannot allocate enough
memory.
Right now ROX cache is only used by modules and in majority of cases the
allocations happen at boot time when there's plenty of free memory, but
upcoming enabling ROX cache for ftrace and kprobes would mean that execmem
allocations can happen when the system is under memory pressure and a
failure to allocate large page worth of memory becomes more likely.
Fallback to regular vmalloc() if vmalloc(VM_ALLOW_HUGE_VMAP) fails.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 056d3caaf4a1..04c35c3a9361 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -291,6 +291,11 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
alloc_size = round_up(size, PMD_SIZE);
p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
+ if (!p) {
+ alloc_size = size;
+ p = execmem_vmalloc(range, alloc_size, PAGE_KERNEL, vm_flags);
+ }
+
if (!p)
return err;
@@ -462,7 +467,7 @@ void *execmem_alloc(enum execmem_type type, size_t size)
bool use_cache = range->flags & EXECMEM_ROX_CACHE;
unsigned long vm_flags = VM_FLUSH_RESET_PERMS;
pgprot_t pgprot = range->pgprot;
- void *p;
+ void *p = NULL;
size = PAGE_ALIGN(size);
--
2.47.2
^ permalink raw reply related
* [PATCH v2 4/8] execmem: move execmem_force_rw() and execmem_restore_rox() before use
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
to avoid static declarations.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 44 +++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 4670e97f8e4e..056d3caaf4a1 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -137,6 +137,27 @@ static int execmem_set_direct_map_valid(struct vm_struct *vm, bool valid)
return err;
}
+static int execmem_force_rw(void *ptr, size_t size)
+{
+ unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long addr = (unsigned long)ptr;
+ int ret;
+
+ ret = set_memory_nx(addr, nr);
+ if (ret)
+ return ret;
+
+ return set_memory_rw(addr, nr);
+}
+
+int execmem_restore_rox(void *ptr, size_t size)
+{
+ unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long addr = (unsigned long)ptr;
+
+ return set_memory_rox(addr, nr);
+}
+
static void execmem_cache_clean(struct work_struct *work)
{
struct maple_tree *free_areas = &execmem_cache.free_areas;
@@ -328,8 +349,6 @@ static inline void *pending_free_clear(void *ptr)
return (void *)((unsigned long)ptr & ~PENDING_FREE_MASK);
}
-static int execmem_force_rw(void *ptr, size_t size);
-
static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
{
size_t size = mas_range_len(mas);
@@ -415,27 +434,6 @@ static bool execmem_cache_free(void *ptr)
return true;
}
-static int execmem_force_rw(void *ptr, size_t size)
-{
- unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
- unsigned long addr = (unsigned long)ptr;
- int ret;
-
- ret = set_memory_nx(addr, nr);
- if (ret)
- return ret;
-
- return set_memory_rw(addr, nr);
-}
-
-int execmem_restore_rox(void *ptr, size_t size)
-{
- unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
- unsigned long addr = (unsigned long)ptr;
-
- return set_memory_rox(addr, nr);
-}
-
#else /* CONFIG_ARCH_HAS_EXECMEM_ROX */
/*
* when ROX cache is not used the permissions defined by architectures for
--
2.47.2
^ permalink raw reply related
* [PATCH v2 3/8] execmem: rework execmem_cache_free()
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Currently execmem_cache_free() ignores potential allocation failures that
may happen in execmem_cache_add(). Besides, it uses text poking to fill the
memory with trapping instructions before returning it to cache although it
would be more efficient to make that memory writable, update it using
memcpy and then restore ROX protection.
Rework execmem_cache_free() so that in case of an error it will defer
freeing of the memory to a delayed work.
With this the happy fast path will now change permissions to RW, fill the
memory with trapping instructions using memcpy, restore ROX permissions,
add the memory back to the free cache and clear the relevant entry in
busy_areas.
If any step in the fast path fails, the entry in busy_areas will be marked
as pending_free. These entries will be handled by a delayed work and freed
asynchronously.
To make the fast path faster, use __GFP_NORETRY for memory allocations and
let asynchronous handler try harder with GFP_KERNEL.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
mm/execmem.c | 125 +++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 102 insertions(+), 23 deletions(-)
diff --git a/mm/execmem.c b/mm/execmem.c
index 6b040fbc5f4f..4670e97f8e4e 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -93,8 +93,15 @@ struct execmem_cache {
struct mutex mutex;
struct maple_tree busy_areas;
struct maple_tree free_areas;
+ unsigned int pending_free_cnt; /* protected by mutex */
};
+/* delay to schedule asynchronous free if fast path free fails */
+#define FREE_DELAY (msecs_to_jiffies(10))
+
+/* mark entries in busy_areas that should be freed asynchronously */
+#define PENDING_FREE_MASK (1 << (PAGE_SHIFT - 1))
+
static struct execmem_cache execmem_cache = {
.mutex = __MUTEX_INITIALIZER(execmem_cache.mutex),
.busy_areas = MTREE_INIT_EXT(busy_areas, MT_FLAGS_LOCK_EXTERN,
@@ -155,20 +162,17 @@ static void execmem_cache_clean(struct work_struct *work)
static DECLARE_WORK(execmem_cache_clean_work, execmem_cache_clean);
-static int execmem_cache_add(void *ptr, size_t size)
+static int execmem_cache_add_locked(void *ptr, size_t size, gfp_t gfp_mask)
{
struct maple_tree *free_areas = &execmem_cache.free_areas;
- struct mutex *mutex = &execmem_cache.mutex;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, free_areas, addr - 1, addr + 1);
unsigned long lower, upper;
void *area = NULL;
- int err;
lower = addr;
upper = addr + size - 1;
- mutex_lock(mutex);
area = mas_walk(&mas);
if (area && mas.last == addr - 1)
lower = mas.index;
@@ -178,12 +182,14 @@ static int execmem_cache_add(void *ptr, size_t size)
upper = mas.last;
mas_set_range(&mas, lower, upper);
- err = mas_store_gfp(&mas, (void *)lower, GFP_KERNEL);
- mutex_unlock(mutex);
- if (err)
- return err;
+ return mas_store_gfp(&mas, (void *)lower, gfp_mask);
+}
- return 0;
+static int execmem_cache_add(void *ptr, size_t size, gfp_t gfp_mask)
+{
+ guard(mutex)(&execmem_cache.mutex);
+
+ return execmem_cache_add_locked(ptr, size, gfp_mask);
}
static bool within_range(struct execmem_range *range, struct ma_state *mas,
@@ -278,7 +284,7 @@ static int execmem_cache_populate(struct execmem_range *range, size_t size)
if (err)
goto err_free_mem;
- err = execmem_cache_add(p, alloc_size);
+ err = execmem_cache_add(p, alloc_size, GFP_KERNEL);
if (err)
goto err_reset_direct_map;
@@ -307,29 +313,102 @@ static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
return __execmem_cache_alloc(range, size);
}
+static inline bool is_pending_free(void *ptr)
+{
+ return ((unsigned long)ptr & PENDING_FREE_MASK);
+}
+
+static inline void *pending_free_set(void *ptr)
+{
+ return (void *)((unsigned long)ptr | PENDING_FREE_MASK);
+}
+
+static inline void *pending_free_clear(void *ptr)
+{
+ return (void *)((unsigned long)ptr & ~PENDING_FREE_MASK);
+}
+
+static int execmem_force_rw(void *ptr, size_t size);
+
+static int __execmem_cache_free(struct ma_state *mas, void *ptr, gfp_t gfp_mask)
+{
+ size_t size = mas_range_len(mas);
+ int err;
+
+ err = execmem_force_rw(ptr, size);
+ if (err)
+ return err;
+
+ execmem_fill_trapping_insns(ptr, size, /* writable = */ true);
+ execmem_restore_rox(ptr, size);
+
+ err = execmem_cache_add_locked(ptr, size, gfp_mask);
+ if (err)
+ return err;
+
+ mas_store_gfp(mas, NULL, gfp_mask);
+ return 0;
+}
+
+static void execmem_cache_free_slow(struct work_struct *work);
+static DECLARE_DELAYED_WORK(execmem_cache_free_work, execmem_cache_free_slow);
+
+static void execmem_cache_free_slow(struct work_struct *work)
+{
+ struct maple_tree *busy_areas = &execmem_cache.busy_areas;
+ MA_STATE(mas, busy_areas, 0, ULONG_MAX);
+ void *area;
+
+ guard(mutex)(&execmem_cache.mutex);
+
+ if (!execmem_cache.pending_free_cnt)
+ return;
+
+ mas_for_each(&mas, area, ULONG_MAX) {
+ if (!is_pending_free(area))
+ continue;
+
+ area = pending_free_clear(area);
+ if (__execmem_cache_free(&mas, area, GFP_KERNEL))
+ continue;
+
+ execmem_cache.pending_free_cnt--;
+ }
+
+ if (execmem_cache.pending_free_cnt)
+ schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
+ else
+ schedule_work(&execmem_cache_clean_work);
+}
+
static bool execmem_cache_free(void *ptr)
{
struct maple_tree *busy_areas = &execmem_cache.busy_areas;
- struct mutex *mutex = &execmem_cache.mutex;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, busy_areas, addr, addr);
- size_t size;
void *area;
+ int err;
+
+ guard(mutex)(&execmem_cache.mutex);
- mutex_lock(mutex);
area = mas_walk(&mas);
- if (!area) {
- mutex_unlock(mutex);
+ if (!area)
return false;
- }
- size = mas_range_len(&mas);
- mas_store_gfp(&mas, NULL, GFP_KERNEL);
- mutex_unlock(mutex);
-
- execmem_fill_trapping_insns(ptr, size, /* writable = */ false);
-
- execmem_cache_add(ptr, size);
+ err = __execmem_cache_free(&mas, area, GFP_KERNEL | __GFP_NORETRY);
+ if (err) {
+ /*
+ * mas points to exact slot we've got the area from, nothing
+ * else can modify the tree because of the mutex, so there
+ * won't be any allocations in mas_store_gfp() and it will just
+ * change the pointer.
+ */
+ area = pending_free_set(area);
+ mas_store_gfp(&mas, area, GFP_KERNEL);
+ execmem_cache.pending_free_cnt++;
+ schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
+ return true;
+ }
schedule_work(&execmem_cache_clean_work);
--
2.47.2
^ permalink raw reply related
* [PATCH v2 2/8] execmem: introduce execmem_alloc_rw()
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Some callers of execmem_alloc() require the memory to be temporarily
writable even when it is allocated from ROX cache. These callers use
execemem_make_temp_rw() right after the call to execmem_alloc().
Wrap this sequence in execmem_alloc_rw() API.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/kernel/alternative.c | 3 +--
include/linux/execmem.h | 38 ++++++++++++++++++++---------------
kernel/module/main.c | 13 ++----------
mm/execmem.c | 27 ++++++++++++++++++++++++-
4 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ea1d984166cd..526a5fef93ab 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -120,7 +120,7 @@ struct its_array its_pages;
static void *__its_alloc(struct its_array *pages)
{
- void *page __free(execmem) = execmem_alloc(EXECMEM_MODULE_TEXT, PAGE_SIZE);
+ void *page __free(execmem) = execmem_alloc_rw(EXECMEM_MODULE_TEXT, PAGE_SIZE);
if (!page)
return NULL;
@@ -237,7 +237,6 @@ static void *its_alloc(void)
if (!page)
return NULL;
- execmem_make_temp_rw(page, PAGE_SIZE);
if (pages == &its_pages)
set_memory_x((unsigned long)page, 1);
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 734fbe83d98e..4e510d1c609c 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -67,21 +67,6 @@ enum execmem_range_flags {
*/
void execmem_fill_trapping_insns(void *ptr, size_t size, bool writable);
-/**
- * execmem_make_temp_rw - temporarily remap region with read-write
- * permissions
- * @ptr: address of the region to remap
- * @size: size of the region to remap
- *
- * Remaps a part of the cached large page in the ROX cache in the range
- * [@ptr, @ptr + @size) as writable and not executable. The caller must
- * have exclusive ownership of this range and ensure nothing will try to
- * execute code in this range.
- *
- * Return: 0 on success or negative error code on failure.
- */
-int execmem_make_temp_rw(void *ptr, size_t size);
-
/**
* execmem_restore_rox - restore read-only-execute permissions
* @ptr: address of the region to remap
@@ -95,7 +80,6 @@ int execmem_make_temp_rw(void *ptr, size_t size);
*/
int execmem_restore_rox(void *ptr, size_t size);
#else
-static inline int execmem_make_temp_rw(void *ptr, size_t size) { return 0; }
static inline int execmem_restore_rox(void *ptr, size_t size) { return 0; }
#endif
@@ -165,6 +149,28 @@ struct execmem_info *execmem_arch_setup(void);
*/
void *execmem_alloc(enum execmem_type type, size_t size);
+/**
+ * execmem_alloc_rw - allocate writatble executable memory
+ * @type: type of the allocation
+ * @size: how many bytes of memory are required
+ *
+ * Allocates memory that will contain executable code, either generated or
+ * loaded from kernel modules.
+ *
+ * Allocates memory that will contain data coupled with executable code,
+ * like data sections in kernel modules.
+ *
+ * Forces writable permissions on the allocated memory and the caller is
+ * responsible to manage the permissions afterwards.
+ *
+ * For architectures that use ROX cache the permissions will be set to R+W.
+ * For architectures that don't use ROX cache the default permissions for @type
+ * will be used as they must be writable.
+ *
+ * Return: a pointer to the allocated memory or %NULL
+ */
+void *execmem_alloc_rw(enum execmem_type type, size_t size);
+
/**
* execmem_free - free executable memory
* @ptr: pointer to the memory that should be freed
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 413ac6ea3702..d009326ef7bb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1292,20 +1292,11 @@ static int module_memory_alloc(struct module *mod, enum mod_mem_type type)
else
execmem_type = EXECMEM_MODULE_TEXT;
- ptr = execmem_alloc(execmem_type, size);
+ ptr = execmem_alloc_rw(execmem_type, size);
if (!ptr)
return -ENOMEM;
- if (execmem_is_rox(execmem_type)) {
- int err = execmem_make_temp_rw(ptr, size);
-
- if (err) {
- execmem_free(ptr);
- return -ENOMEM;
- }
-
- mod->mem[type].is_rox = true;
- }
+ mod->mem[type].is_rox = execmem_is_rox(execmem_type);
/*
* The pointer to these blocks of memory are stored on the module
diff --git a/mm/execmem.c b/mm/execmem.c
index 0712ebb4eb77..6b040fbc5f4f 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -336,7 +336,7 @@ static bool execmem_cache_free(void *ptr)
return true;
}
-int execmem_make_temp_rw(void *ptr, size_t size)
+static int execmem_force_rw(void *ptr, size_t size)
{
unsigned int nr = PAGE_ALIGN(size) >> PAGE_SHIFT;
unsigned long addr = (unsigned long)ptr;
@@ -358,6 +358,16 @@ int execmem_restore_rox(void *ptr, size_t size)
}
#else /* CONFIG_ARCH_HAS_EXECMEM_ROX */
+/*
+ * when ROX cache is not used the permissions defined by architectures for
+ * execmem ranges that are updated before use (e.g. EXECMEM_MODULE_TEXT) must
+ * be writable anyway
+ */
+static inline int execmem_force_rw(void *ptr, size_t size)
+{
+ return 0;
+}
+
static void *execmem_cache_alloc(struct execmem_range *range, size_t size)
{
return NULL;
@@ -387,6 +397,21 @@ void *execmem_alloc(enum execmem_type type, size_t size)
return kasan_reset_tag(p);
}
+void *execmem_alloc_rw(enum execmem_type type, size_t size)
+{
+ void *p __free(execmem) = execmem_alloc(type, size);
+ int err;
+
+ if (!p)
+ return NULL;
+
+ err = execmem_force_rw(p, size);
+ if (err)
+ return NULL;
+
+ return no_free_ptr(p);
+}
+
void execmem_free(void *ptr)
{
/*
--
2.47.2
^ permalink raw reply related
* [PATCH v2 1/8] execmem: drop unused execmem_update_copy()
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250709134933.3848895-1-rppt@kernel.org>
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
The execmem_update_copy() that used text poking was required when memory
allocated from ROX cache was always read-only. Since now its permissions
can be switched to read-write there is no need in a function that updates
memory with text poking.
Remove it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
include/linux/execmem.h | 13 -------------
mm/execmem.c | 5 -----
2 files changed, 18 deletions(-)
diff --git a/include/linux/execmem.h b/include/linux/execmem.h
index 3be35680a54f..734fbe83d98e 100644
--- a/include/linux/execmem.h
+++ b/include/linux/execmem.h
@@ -185,19 +185,6 @@ DEFINE_FREE(execmem, void *, if (_T) execmem_free(_T));
struct vm_struct *execmem_vmap(size_t size);
#endif
-/**
- * execmem_update_copy - copy an update to executable memory
- * @dst: destination address to update
- * @src: source address containing the data
- * @size: how many bytes of memory shold be copied
- *
- * Copy @size bytes from @src to @dst using text poking if the memory at
- * @dst is read-only.
- *
- * Return: a pointer to @dst or NULL on error
- */
-void *execmem_update_copy(void *dst, const void *src, size_t size);
-
/**
* execmem_is_rox - check if execmem is read-only
* @type - the execmem type to check
diff --git a/mm/execmem.c b/mm/execmem.c
index 2b683e7d864d..0712ebb4eb77 100644
--- a/mm/execmem.c
+++ b/mm/execmem.c
@@ -399,11 +399,6 @@ void execmem_free(void *ptr)
vfree(ptr);
}
-void *execmem_update_copy(void *dst, const void *src, size_t size)
-{
- return text_poke_copy(dst, src, size);
-}
-
bool execmem_is_rox(enum execmem_type type)
{
return !!(execmem_info->ranges[type].flags & EXECMEM_ROX_CACHE);
--
2.47.2
^ permalink raw reply related
* [PATCH v2 0/8] x86: enable EXECMEM_ROX_CACHE for ftrace and kprobes
From: Mike Rapoport @ 2025-07-09 13:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Andy Lutomirski, Borislav Petkov, Christophe Leroy, Daniel Gomez,
Dave Hansen, Ingo Molnar, Liam R. Howlett, Luis Chamberlain,
Mark Rutland, Masami Hiramatsu, Mike Rapoport, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
Hi,
These patches enable use of EXECMEM_ROX_CACHE for ftrace and kprobes
allocations on x86.
They also include some ground work in execmem.
Since the execmem model for caching large ROX pages changed from the
initial assumption that the memory that is allocated from ROX cache is
always ROX to the current state where memory can be temporarily made RW and
then restored to ROX, we can stop using text poking to update it. This also
saves the hassle of trying lock text_mutex in execmem_cache_free() when
kprobes already hold that mutex.
The patches 1-6 update and cleanup execmem ROX cache management,
patch 7 enables EXECMEM_ROX_CACHE for kprobes and
patch 8 enables EXECMEM_ROX_CACHE for frace.
The patches are also available at git:
https://git.kernel.org/rppt/h/execmem/x86-rox/ftrace%2bkprobes/v2
v2 changes:
* Fix setting and clearing pending_free for an area (Yann)
* Reorder execmem_cache_free() to avoid error goto (Peter)
* Add comment why mas_store_gfp() cannot fail in execmem_cache_free() (Peter)
Mike Rapoport (Microsoft) (8):
execmem: drop unused execmem_update_copy()
execmem: introduce execmem_alloc_rw()
execmem: rework execmem_cache_free()
execmem: move execmem_force_rw() and execmem_restore_rox() before use
execmem: add fallback for failures in vmalloc(VM_ALLOW_HUGE_VMAP)
execmem: drop writable parameter from execmem_fill_trapping_insns()
x86/kprobes: enable EXECMEM_ROX_CACHE for kprobes allocations
x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
arch/x86/kernel/alternative.c | 3 +-
arch/x86/kernel/ftrace.c | 2 +-
arch/x86/kernel/kprobes/core.c | 18 ---
arch/x86/mm/init.c | 24 ++--
include/linux/execmem.h | 54 ++++-----
kernel/module/main.c | 13 +--
mm/execmem.c | 198 +++++++++++++++++++++++++--------
7 files changed, 194 insertions(+), 118 deletions(-)
base-commit: 86731a2a651e58953fc949573895f2fa6d456841
--
2.47.2
^ permalink raw reply
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-07-09 10:34 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
In-Reply-To: <DB6JZBUSWGKX.3M3M5TSWPLLFN@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>
>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>> >
>>>> > Introduce the `SetOnce` type, a container that can only be written once.
>>>> > The container uses an internal atomic to synchronize writes to the internal
>>>> > value.
>>>> >
>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>>
>>>> LGTM:
>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>>>
>>>> > +impl<T> Drop for SetOnce<T> {
>>>> > + fn drop(&mut self) {
>>>> > + if self.init.load(Acquire) == 2 {
>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>>>> > + // `self`.
>>>> > + unsafe { drop_in_place(self.value.get()) };
>>>>
>>>> This load does not need to be Acquire. It can be a Relaxed load or
>>>> even an unsynchronized one since the access is exclusive.
>>>
>>> Right, I think we can do the similar as Revocable here:
>>>
>>> if *self.init.get_mut() == 2 { }
Ok, now I got it. You are saying I don't need to use the atomic load
method, because I have mutable access. Sounds good.
But I guess a relaxed load and access through a mutable reference should
result in the same code generation on most (all?) platforms?
>>>
>>> Further, with my following Benno's suggestion and making `Atomic<T>` an
>>> `UnsafeCell<T>:
>>>
>>> https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/
>>>
>>> compiler can generate a noalias reference here, which allows further
>>> optimization.
>>>
>>
>> You would like to remove `PhantomPinned` to enable noalias? I guess that
>> makes sense in this case. I'll fix that for next spin.
>
> I think you two are talking about different things. Boqun is saying that
> the `Atomic<T>` will use `UnsafeCell` rather than `Opaque`, which will
> potentially allow more optimizations.
>
> But you are talking about `SetOnce`, right? I think it makes more sense
> for `SetOnce` to use `UnsafeCell<MaybeUninit<T>>` rather than `Opaque`
> too. So feel free to change it in the next version.
Exactly. We don't need `UnsafePinned` mechanics here.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Benno Lossin @ 2025-07-09 9:10 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <87v7o1lp2g.fsf@kernel.org>
On Wed Jul 9, 2025 at 10:56 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>
>> On Tue Jul 8, 2025 at 3:06 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@kernel.org> writes:
>>>> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>>>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>>>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>>>>> --- a/rust/kernel/sync.rs
>>>>> +++ b/rust/kernel/sync.rs
>>>>> @@ -18,6 +18,7 @@
>>>>> mod locked_by;
>>>>> pub mod poll;
>>>>> pub mod rcu;
>>>>> +mod set_once;
>>>>
>>>> I would have named this `once`.
>>>
>>> So module `once` and struct `SetOnce`? Struct name `Once` would lead
>>> thoughts to `std::sync::Once`, which is a different thing.
>>
>> Hmm I thought that `Once` and `SetOnce` would live in the same module,
>> but if they don't then I think it's better to keep the `set_once`
>> module as-is.
>
> I guess they could live together. I was thinking a module for each. We
> can always move it, the module name is not part of a public API.
>
> Let's go with `set_once` for now and we can change it later, if we
> decide it is for the better?
Sure.
---
Cheers,
Benno
^ permalink raw reply
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-07-09 8:56 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
In-Reply-To: <DB6QM8WOX9FS.2W36R3YF2VS0F@kernel.org>
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Jul 8, 2025 at 3:06 PM CEST, Andreas Hindborg wrote:
>> "Benno Lossin" <lossin@kernel.org> writes:
>>> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>>>> --- a/rust/kernel/sync.rs
>>>> +++ b/rust/kernel/sync.rs
>>>> @@ -18,6 +18,7 @@
>>>> mod locked_by;
>>>> pub mod poll;
>>>> pub mod rcu;
>>>> +mod set_once;
>>>
>>> I would have named this `once`.
>>
>> So module `once` and struct `SetOnce`? Struct name `Once` would lead
>> thoughts to `std::sync::Once`, which is a different thing.
>
> Hmm I thought that `Once` and `SetOnce` would live in the same module,
> but if they don't then I think it's better to keep the `set_once`
> module as-is.
I guess they could live together. I was thinking a module for each. We
can always move it, the module name is not part of a public API.
Let's go with `set_once` for now and we can change it later, if we
decide it is for the better?
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [GIT PULL] Modules fixes for v6.16-rc6
From: pr-tracker-bot @ 2025-07-08 20:40 UTC (permalink / raw)
To: Daniel Gomez
Cc: Linus Torvalds, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, linux-modules, Sebastian Andrzej Siewior,
linux-kernel
In-Reply-To: <20250708193822.45168-1-da.gomez@kernel.org>
The pull request you sent on Tue, 8 Jul 2025 21:38:20 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-6.16-rc6.fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/72782127388d96e971f0186996a5bd44e64a1665
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: Re: [GIT PULL] Modules fixes for v6.16-rc6
From: pr-tracker-bot @ 2025-07-08 20:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Daniel Gomez, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, linux-modules, Sebastian Andrzej Siewior,
linux-kernel
In-Reply-To: <CAHk-=whHRv28HBL47x-L5okG+fZDJyX432uxG3AQqbqRAgkAYQ@mail.gmail.com>
The pull request you sent on Tue, 8 Jul 2025 13:21:09 -0700:
> https://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux-fullhistory.git/ refs/heads/master
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/046cc01be6b9d139b49dfc396b7201c633ff1a26
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] Modules fixes for v6.16-rc6
From: Linus Torvalds @ 2025-07-08 20:21 UTC (permalink / raw)
To: Daniel Gomez
Cc: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
linux-modules, Sebastian Andrzej Siewior, linux-kernel
In-Reply-To: <20250708193822.45168-1-da.gomez@kernel.org>
On Tue, 8 Jul 2025 at 12:38, Daniel Gomez <da.gomez@kernel.org> wrote:
>
> Petr and Sebastian went down to the archive history (before Git) and
> found the commit that broke it b3b91325f3c7 ("Don't relocate
> non-allocated regions in modules.").
When mentioning git commits in other trees, you really do need to
mention *which* other tree it is.
In this case, it seems to be
https://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux-fullhistory.git/
that has that commit, but that's actually a kind of odd tree.
The typical bitkeeper conversion tree (which is where I suspect the
full-history conversion then got it from, unless somebody did a
separate conversion) that contains that commit is actually
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/
and there the commit has commit ID 1a6100caae ("Don't relocate
non-allocated regions in modules")
Linus
^ permalink raw reply
* [GIT PULL] Modules fixes for v6.16-rc6
From: Daniel Gomez @ 2025-07-08 19:38 UTC (permalink / raw)
To: Linus Torvalds, Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, linux-modules
Cc: Sebastian Andrzej Siewior, linux-kernel
The following changes since commit d7b8f8e20813f0179d8ef519541a3527e7661d3a:
Linux 6.16-rc5 (2025-07-06 14:10:26 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git tags/modules-6.16-rc6.fixes
for you to fetch changes up to af1ccf546e5f2915fbbde26841db43a971d81cf3:
MAINTAINERS: update Daniel Gomez's role and email address (2025-07-08 20:52:30 +0200)
----------------------------------------------------------------
Modules fixes for 6.16-rc6
This includes 2 fixes: one introduced in the current release cycle (v6.16-rc1)
and another introduced back in v6.4-rc1. Both have been in linux-next since
next-250701 and have passed the modules ci tests using kdevops. Finally, as Petr
and Luis mentioned in previous pull requests, add myself (Daniel Gomez) to the
list of modules maintainers.
The first was reported by Intel's kernel test robot, and it addresses a crash
exposed by Sebastian's commit c50d295c37f2 ("rds: Use nested-BH locking for
rds_page_remainder") by allowing relocations for the per-CPU section even if it
lacks the SHF_ALLOC flag. Petr and Sebastian went down to the archive history
(before Git) and found the commit that broke it b3b91325f3c7 ("Don't relocate
non-allocated regions in modules.").
The second fix, reported and fixed by Petr (with additional cleanup), resolves a
memory leak by ensuring proper deallocation if module loading fails. We couldn't
find a reproducer other than forcing it manually or leveraging eBPF. So, I
tested it by enabling error injection in the codetag functions through the error
path that produces the leak and made it fail until execmem is unable to allocate
more memory.
Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE73Ua4R8Pc+G5xjxTQJ6jxB8ZUfsFAmhta7AACgkQQJ6jxB8Z
UftulhAAiK78CFOdIcKbGKMD8tBGCHUT6IVCskY0E3aqDmhZiNEXhpmwKnOq56zo
ztbwg81Acc41t0Y283vgmZuEpJbzmS47qRYIZEhk5BuLdKl3Hn91Iyi3sdZLB9gD
VaYkpY0y4V3pWL5AM/9rtnLvY7PSLZG0bnAeS2fsOzxN5DIn2IIdD3WsU++96O+k
LHIik7dXcBQQKOukRClAI+cZiU2IGGG/FcDuMmANcsw+E4GKTBQunZiixTZOjdgi
e//NMYw1+Y5DJQ/i7C3Fl6+W+r/8/HPkRxyZ3Wt79WBZxwV/Hp5iTG9652vHKE1i
ybBDwlJkQC/EQsup4NKzDby38bPW1tdYwwK0dNbMRGxNICqNe7rq5fEG4WjTjXAA
H2O5VZ84Ys1SRx+uXeu83aog6fdiZ9DezGuaxFgrTRHUken2QvxJ/HTXe1BmMNY5
9BbHPrG22dp+dEDptSVaDWWMQZ6OP73gXrPYowQC8/FOm9NdBXVIH9hvvI7KRSf5
OC4A7nyZCDNW/Dwzc6yi3qAWMkKCR6IGMqN9vFiR3iIkaHJN1KABFzaVmh3JpR+B
6TsuO8/EIu9Ue25VgMehSj9LuGFgbhcfg40N2Fj/7+vgAM4Rd5sOw/NyiaD8xUPC
j0Ql4ehZiOQof8wAhdxpHFQQjO30UdpQgOmUO1yNUdmuPIpdSN0=
=dQvQ
-----END PGP SIGNATURE-----
----------------------------------------------------------------
Daniel Gomez (1):
MAINTAINERS: update Daniel Gomez's role and email address
Petr Pavlu (2):
module: Fix memory deallocation on error path in move_module()
module: Avoid unnecessary return value initialization in move_module()
Sebastian Andrzej Siewior (1):
module: Make sure relocations are applied to the per-CPU section
MAINTAINERS | 2 +-
kernel/module/main.c | 17 +++++++++++------
2 files changed, 12 insertions(+), 7 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox