* [PATCH v3 1/6] rust: retain pointer mut-ness in `container_of!`
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 2/6] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
Avoid casting the input pointer to `*const _`, allowing the output
pointer to be `*mut` if the input is `*mut`. This allows a number of
`*const` to `*mut` conversions to be removed at the cost of slightly
worse ergonomics when the macro is used with a reference rather than a
pointer; the only example of this was in the macro's own doctest.
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/lib.rs | 5 ++---
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 2 +-
rust/kernel/rbtree.rs | 23 ++++++++++-------------
4 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7697c60b2d1a..9cd6b6864739 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -187,7 +187,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
/// }
///
/// let test = Test { a: 10, b: 20 };
-/// let b_ptr = &test.b;
+/// let b_ptr: *const _ = &test.b;
/// // SAFETY: The pointer points at the `b` field of a `Test`, so the resulting pointer will be
/// // in-bounds of the same allocation as `b_ptr`.
/// let test_alias = unsafe { container_of!(b_ptr, Test, b) };
@@ -196,9 +196,8 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
#[macro_export]
macro_rules! container_of {
($ptr:expr, $type:ty, $($f:tt)*) => {{
- let ptr = $ptr as *const _ as *const u8;
let offset: usize = ::core::mem::offset_of!($type, $($f)*);
- ptr.sub(offset) as *const $type
+ $ptr.byte_sub(offset).cast::<$type>()
}}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4c98b5b9aa1e..c37476576f02 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -364,7 +364,7 @@ pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::pci_dev {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct pci_dev`.
- unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+ unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) }
}
/// Returns the PCI vendor ID.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 50e6b0421813..c51617569c01 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -189,7 +189,7 @@ unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::platform_device {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct platform_device`.
- unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut()
+ unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }
}
}
diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 0d1e75810664..1fdea2806cfa 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -424,7 +424,7 @@ pub fn cursor_lower_bound(&mut self, key: &K) -> Option<Cursor<'_, K, V>>
while !node.is_null() {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node, Node<K, V>, links) };
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
let this_key = unsafe { &(*this).key };
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
@@ -496,7 +496,7 @@ fn drop(&mut self) {
// but it is not observable. The loop invariant is still maintained.
// SAFETY: `this` is valid per the loop invariant.
- unsafe { drop(KBox::from_raw(this.cast_mut())) };
+ unsafe { drop(KBox::from_raw(this)) };
}
}
}
@@ -761,7 +761,7 @@ pub fn remove_current(self) -> (Option<Self>, RBTreeNode<K, V>) {
let next = self.get_neighbor_raw(Direction::Next);
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
let node = RBTreeNode { node };
@@ -806,7 +806,7 @@ fn remove_neighbor(&mut self, direction: Direction) -> Option<RBTreeNode<K, V>>
unsafe { bindings::rb_erase(neighbor, addr_of_mut!(self.tree.root)) };
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(neighbor, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(neighbor, Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
return Some(RBTreeNode { node });
@@ -912,7 +912,7 @@ unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b
unsafe fn to_key_value_raw<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, *mut V) {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) };
// SAFETY: The passed `node` is the current node or a non-null neighbor,
// thus `this` is valid by the type invariants.
let k = unsafe { &(*this).key };
@@ -1021,7 +1021,7 @@ fn next(&mut self) -> Option<Self::Item> {
// SAFETY: By the type invariant of `IterRaw`, `self.next` is a valid node in an `RBTree`,
// and by the type invariant of `RBTree`, all nodes point to the links field of `Node<K, V>` objects.
- let cur = unsafe { container_of!(self.next, Node<K, V>, links) }.cast_mut();
+ let cur = unsafe { container_of!(self.next, Node<K, V>, links) };
// SAFETY: `self.next` is a valid tree node by the type invariants.
self.next = unsafe { bindings::rb_next(self.next) };
@@ -1216,7 +1216,7 @@ pub fn get_mut(&mut self) -> &mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - We have exclusive access to the underlying tree, and can thus give out a mutable reference.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Converts the entry into a mutable reference to its value.
@@ -1226,7 +1226,7 @@ pub fn into_mut(self) -> &'a mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - This consumes the `&'a mut RBTree<K, V>`, therefore it can give out a mutable reference that lives for `'a`.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Remove this entry from the [`RBTree`].
@@ -1239,9 +1239,7 @@ pub fn remove_node(self) -> RBTreeNode<K, V> {
RBTreeNode {
// SAFETY: The node was a node in the tree, but we removed it, so we can convert it
// back into a box.
- node: unsafe {
- KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut())
- },
+ node: unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) },
}
}
@@ -1272,8 +1270,7 @@ fn replace(self, node: RBTreeNode<K, V>) -> RBTreeNode<K, V> {
// SAFETY:
// - `self.node_ptr` produces a valid pointer to a node in the tree.
// - Now that we removed this entry from the tree, we can convert the node to a box.
- let old_node =
- unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut()) };
+ let old_node = unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) };
RBTreeNode { node: old_node }
}
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 2/6] rust: enable `clippy::ptr_as_ptr` lint
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 1/6] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 3/6] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast` is safer because it cannot accidentally change the
> pointer's mutability, nor cast the pointer to other types like `usize`.
There are a few classes of changes required:
- Modules generated by bindgen are marked
`#[allow(clippy::ptr_as_ptr)]`.
- Inferred casts (` as _`) are replaced with `.cast()`.
- Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
- Multistep casts from references (` as *const _ as *const T`) are
replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
according to the previous rules. The intermediate `let` binding is
required because `(x as *const _).cast::<T>()` results in inference
failure.
- Native literal C strings are replaced with `c_str!().as_char_ptr()`.
- `*mut *mut T as _` is replaced with `let *mut *const T = (*mut *mut
T)`.cast();` since pointer to pointer can be confusing.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/bindings/lib.rs | 1 +
rust/kernel/alloc/allocator_test.rs | 2 +-
rust/kernel/alloc/kvec.rs | 4 ++--
rust/kernel/device.rs | 5 +++--
rust/kernel/devres.rs | 2 +-
rust/kernel/error.rs | 2 +-
rust/kernel/firmware.rs | 3 ++-
rust/kernel/fs/file.rs | 2 +-
rust/kernel/kunit.rs | 15 +++++++--------
rust/kernel/list/impl_list_item_mod.rs | 2 +-
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 4 +++-
rust/kernel/print.rs | 11 +++++------
rust/kernel/seq_file.rs | 3 ++-
rust/kernel/str.rs | 2 +-
rust/kernel/sync/poll.rs | 2 +-
rust/kernel/workqueue.rs | 10 +++++-----
rust/uapi/lib.rs | 1 +
19 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/Makefile b/Makefile
index 70bdbf2218fc..ec8efc8e23ba 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,7 @@ export rust_common_flags := --edition=2021 \
-Wclippy::needless_continue \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
+ -Wclippy::ptr_as_ptr \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 014af0d1fc70..0486a32ed314 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -25,6 +25,7 @@
)]
#[allow(dead_code)]
+#[allow(clippy::ptr_as_ptr)]
#[allow(clippy::undocumented_unsafe_blocks)]
mod bindings_raw {
// Manual definition for blocklisted types.
diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs
index c37d4c0c64e9..8017aa9d5213 100644
--- a/rust/kernel/alloc/allocator_test.rs
+++ b/rust/kernel/alloc/allocator_test.rs
@@ -82,7 +82,7 @@ unsafe fn realloc(
// SAFETY: Returns either NULL or a pointer to a memory allocation that satisfies or
// exceeds the given size and alignment requirements.
- let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) } as *mut u8;
+ let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) }.cast::<u8>();
let dst = NonNull::new(dst).ok_or(AllocError)?;
if flags.contains(__GFP_ZERO) {
diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index ae9d072741ce..c12844764671 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -262,7 +262,7 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
// - `self.len` is smaller than `self.capacity` and hence, the resulting pointer is
// guaranteed to be part of the same allocated object.
// - `self.len` can not overflow `isize`.
- let ptr = unsafe { self.as_mut_ptr().add(self.len) } as *mut MaybeUninit<T>;
+ let ptr = unsafe { self.as_mut_ptr().add(self.len) }.cast::<MaybeUninit<T>>();
// SAFETY: The memory between `self.len` and `self.capacity` is guaranteed to be allocated
// and valid, but uninitialized.
@@ -554,7 +554,7 @@ fn drop(&mut self) {
// - `ptr` points to memory with at least a size of `size_of::<T>() * len`,
// - all elements within `b` are initialized values of `T`,
// - `len` does not exceed `isize::MAX`.
- unsafe { Vec::from_raw_parts(ptr as _, len, len) }
+ unsafe { Vec::from_raw_parts(ptr.cast(), len, len) }
}
}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..9e500498835d 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -168,16 +168,17 @@ pub fn pr_dbg(&self, args: fmt::Arguments<'_>) {
/// `KERN_*`constants, for example, `KERN_CRIT`, `KERN_ALERT`, etc.
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
+ let msg: *const _ = &msg;
// SAFETY: `klevel` is null-terminated and one of the kernel constants. `self.as_raw`
// is valid because `self` is valid. The "%pA" format string expects a pointer to
// `fmt::Arguments`, which is what we're passing as the last argument.
#[cfg(CONFIG_PRINTK)]
unsafe {
bindings::_dev_printk(
- klevel as *const _ as *const crate::ffi::c_char,
+ klevel.as_ptr().cast::<crate::ffi::c_char>(),
self.as_raw(),
c_str!("%pA").as_char_ptr(),
- &msg as *const _ as *const crate::ffi::c_void,
+ msg.cast::<crate::ffi::c_void>(),
)
};
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 942376f6f3af..3a9d998ec371 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -157,7 +157,7 @@ fn remove_action(this: &Arc<Self>) {
#[allow(clippy::missing_safety_doc)]
unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
- let ptr = ptr as *mut DevresInner<T>;
+ let ptr = ptr.cast::<DevresInner<T>>();
// Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
// reference.
// SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index a194d83e6835..8c62cff8742e 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _) as *mut _ }
+ unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..74df815d2f4e 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -58,10 +58,11 @@ impl Firmware {
fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
let mut fw: *mut bindings::firmware = core::ptr::null_mut();
let pfw: *mut *mut bindings::firmware = &mut fw;
+ let pfw: *mut *const bindings::firmware = pfw.cast();
// SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
// `name` and `dev` are valid as by their type invariants.
- let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
+ let ret = unsafe { func.0(pfw, name.as_char_ptr(), dev.as_raw()) };
if ret != 0 {
return Err(Error::from_errno(ret));
}
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index e03dbe14d62a..8936afc234a4 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -364,7 +364,7 @@ fn deref(&self) -> &LocalFile {
//
// By the type invariants, there are no `fdget_pos` calls that did not take the
// `f_pos_lock` mutex.
- unsafe { LocalFile::from_raw_file(self as *const File as *const bindings::file) }
+ unsafe { LocalFile::from_raw_file((self as *const Self).cast()) }
}
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 824da0e9738a..7ed2063c1af0 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -8,19 +8,20 @@
use core::{ffi::c_void, fmt};
+#[cfg(CONFIG_PRINTK)]
+use crate::c_str;
+
/// Prints a KUnit error-level message.
///
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn err(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x013%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x013%pA").as_char_ptr(), args.cast::<c_void>());
}
}
@@ -29,14 +30,12 @@ pub fn err(args: fmt::Arguments<'_>) {
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn info(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x016%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x016%pA").as_char_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee1..1f9498c1458f 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -34,7 +34,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
// SAFETY: The caller promises that the pointer is valid. The implementer promises that the
// `OFFSET` constant is correct.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast() }
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c37476576f02..63218abb7a25 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -75,7 +75,7 @@ extern "C" fn probe_callback(
// Let the `struct pci_dev` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct pci_dev`.
- unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign().cast()) };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index c51617569c01..d609119e8ce8 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -66,7 +66,9 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
// Let the `struct platform_device` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct platform_device`.
- unsafe { bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe {
+ bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign().cast())
+ };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index b19ee490be58..0245c145ea32 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -25,7 +25,7 @@
// SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
// SAFETY: TODO.
- let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
+ let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
w.pos().cast()
}
@@ -102,6 +102,7 @@ pub unsafe fn call_printk(
module_name: &[u8],
args: fmt::Arguments<'_>,
) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
#[cfg(CONFIG_PRINTK)]
// SAFETY: TODO.
@@ -109,7 +110,7 @@ pub unsafe fn call_printk(
bindings::_printk(
format_string.as_ptr(),
module_name.as_ptr(),
- &args as *const _ as *const c_void,
+ args.cast::<c_void>(),
);
}
}
@@ -122,15 +123,13 @@ pub unsafe fn call_printk(
#[doc(hidden)]
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
pub fn call_printk_cont(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
//
// SAFETY: The format string is fixed.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- format_strings::CONT.as_ptr(),
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(format_strings::CONT.as_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 04947c672979..90545d28e6b7 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -31,12 +31,13 @@ pub unsafe fn from_raw<'a>(ptr: *mut bindings::seq_file) -> &'a SeqFile {
/// Used by the [`seq_print`] macro.
pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: Passing a void pointer to `Arguments` is valid for `%pA`.
unsafe {
bindings::seq_printf(
self.inner.get(),
c_str!("%pA").as_char_ptr(),
- &args as *const _ as *const crate::ffi::c_void,
+ args.cast::<crate::ffi::c_void>(),
);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..6a1a982b946d 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -191,7 +191,7 @@ pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
// to a `NUL`-terminated C string.
let len = unsafe { bindings::strlen(ptr) } + 1;
// SAFETY: Lifetime guaranteed by the safety precondition.
- let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len) };
+ let bytes = unsafe { core::slice::from_raw_parts(ptr.cast(), len) };
// SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`.
// As we have added 1 to `len`, the last byte is known to be `NUL`.
unsafe { Self::from_bytes_with_nul_unchecked(bytes) }
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index d5f17153b424..a151f54cde91 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -73,7 +73,7 @@ pub fn register_wait(&mut self, file: &File, cv: &PollCondVar) {
// be destroyed, the destructor must run. That destructor first removes all waiters,
// and then waits for an rcu grace period. Therefore, `cv.wait_queue_head` is valid for
// long enough.
- unsafe { qproc(file.as_ptr() as _, cv.wait_queue_head.get(), self.0.get()) };
+ unsafe { qproc(file.as_ptr().cast(), cv.wait_queue_head.get(), self.0.get()) };
}
}
}
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index b7be224cdf4b..51eb40d0abaa 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -170,7 +170,7 @@ impl Queue {
pub unsafe fn from_raw<'a>(ptr: *const bindings::workqueue_struct) -> &'a Queue {
// SAFETY: The `Queue` type is `#[repr(transparent)]`, so the pointer cast is valid. The
// caller promises that the pointer is not dangling.
- unsafe { &*(ptr as *const Queue) }
+ unsafe { &*ptr.cast::<Queue>() }
}
/// Enqueues a work item.
@@ -457,7 +457,7 @@ fn get_work_offset(&self) -> usize {
#[inline]
unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work<T, ID> {
// SAFETY: The caller promises that the pointer is valid.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut Work<T, ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast::<Work<T, ID>>() }
}
/// Returns a pointer to the struct containing the [`Work<T, ID>`] field.
@@ -472,7 +472,7 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
{
// SAFETY: The caller promises that the pointer points at a field of the right type in the
// right kind of struct.
- unsafe { (ptr as *mut u8).sub(Self::OFFSET) as *mut Self }
+ unsafe { ptr.cast::<u8>().sub(Self::OFFSET).cast::<Self>() }
}
}
@@ -538,7 +538,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
@@ -591,7 +591,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 13495910271f..fe9bf7b5a306 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -15,6 +15,7 @@
#![allow(
clippy::all,
clippy::undocumented_unsafe_blocks,
+ clippy::ptr_as_ptr,
dead_code,
missing_docs,
non_camel_case_types,
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 3/6] rust: enable `clippy::ptr_cast_constness` lint
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 1/6] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 2/6] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 4/6] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.72.0, Clippy introduced the `ptr_cast_constness` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast_mut` and `pointer::cast_const` are safer because they
> cannot accidentally cast the pointer to another type.
There are only 2 affected sites:
- `*mut T as *const U as *mut U` becomes `(*mut T).cast()`
- `&self as *const Self as *mut Self` becomes a reference-to-pointer
coercion + `(*const Self).cast()`.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/request.rs | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index ec8efc8e23ba..c62bae2b107b 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,7 @@ export rust_common_flags := --edition=2021 \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
-Wclippy::ptr_as_ptr \
+ -Wclippy::ptr_cast_constness \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 7943f43b9575..10c6d69be7f3 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -69,7 +69,7 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef<Self> {
// INVARIANT: By the safety requirements of this function, invariants are upheld.
// SAFETY: By the safety requirement of this function, we own a
// reference count that we can pass to `ARef`.
- unsafe { ARef::from_raw(NonNull::new_unchecked(ptr as *const Self as *mut Self)) }
+ unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) }
}
/// Notify the block layer that a request is going to be processed now.
@@ -151,11 +151,12 @@ pub(crate) unsafe fn wrapper_ptr(this: *mut Self) -> NonNull<RequestDataWrapper>
/// Return a reference to the [`RequestDataWrapper`] stored in the private
/// area of the request structure.
pub(crate) fn wrapper_ref(&self) -> &RequestDataWrapper {
+ let this: *const _ = self;
// SAFETY: By type invariant, `self.0` is a valid allocation. Further,
// the private data associated with this request is initialized and
// valid. The existence of `&self` guarantees that the private data is
// valid as a shared reference.
- unsafe { Self::wrapper_ptr(self as *const Self as *mut Self).as_ref() }
+ unsafe { Self::wrapper_ptr(this.cast_mut()).as_ref() }
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 4/6] rust: enable `clippy::as_ptr_cast_mut` lint
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (2 preceding siblings ...)
2025-03-14 12:28 ` [PATCH v3 3/6] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 5/6] rust: enable `clippy::as_underscore` lint Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 6/6] rust: use strict provenance APIs Tamir Duberstein
5 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.66.0, Clippy introduced the `as_ptr_cast_mut` lint [1]:
> Since `as_ptr` takes a `&self`, the pointer won’t have write
> permissions unless interior mutability is used, making it unlikely
> that having it as a mutable pointer is correct.
There is only one affected callsite, and the change amounts to replacing
`as _` with `.cast_mut().cast()`. This doesn't change the semantics, but
is more descriptive of what's going on.
Apply this change and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/devres.rs | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c62bae2b107b..bb15b86182a3 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ export rust_common_flags := --edition=2021 \
-Wrust_2018_idioms \
-Wunreachable_pub \
-Wclippy::all \
+ -Wclippy::as_ptr_cast_mut \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 3a9d998ec371..598001157293 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -143,7 +143,7 @@ fn remove_action(this: &Arc<Self>) {
bindings::devm_remove_action_nowarn(
this.dev.as_raw(),
Some(this.callback),
- this.as_ptr() as _,
+ this.as_ptr().cast_mut().cast(),
)
};
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 5/6] rust: enable `clippy::as_underscore` lint
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (3 preceding siblings ...)
2025-03-14 12:28 ` [PATCH v3 4/6] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 12:28 ` [PATCH v3 6/6] rust: use strict provenance APIs Tamir Duberstein
5 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.63.0, Clippy introduced the `as_underscore` lint [1]:
> The conversion might include lossy conversion or a dangerous cast that
> might go undetected due to the type being inferred.
>
> The lint is allowed by default as using `_` is less wordy than always
> specifying the type.
Always specifying the type is especially helpful in function call
contexts where the inferred type may change at a distance. Specifying
the type also allows Clippy to spot more cases of `useless_conversion`.
The primary downside is the need to specify the type in trivial getters.
There are 4 such functions: 3 have become slightly less ergonomic, 1 was
revealed to be a `useless_conversion`.
While this doesn't eliminate unchecked `as` conversions, it makes such
conversions easier to scrutinize. It also has the slight benefit of
removing a degree of freedom on which to bikeshed. Thus apply the
changes and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/operations.rs | 2 +-
rust/kernel/block/mq/request.rs | 2 +-
rust/kernel/device_id.rs | 2 +-
rust/kernel/devres.rs | 15 ++++++++-------
rust/kernel/error.rs | 2 +-
rust/kernel/io.rs | 18 +++++++++---------
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/of.rs | 6 +++---
rust/kernel/pci.rs | 9 ++++++---
rust/kernel/str.rs | 8 ++++----
rust/kernel/workqueue.rs | 2 +-
12 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/Makefile b/Makefile
index bb15b86182a3..2af40bfed9ce 100644
--- a/Makefile
+++ b/Makefile
@@ -478,6 +478,7 @@ export rust_common_flags := --edition=2021 \
-Wunreachable_pub \
-Wclippy::all \
-Wclippy::as_ptr_cast_mut \
+ -Wclippy::as_underscore \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 864ff379dc91..d18ef55490da 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -101,7 +101,7 @@ impl<T: Operations> OperationsVTable<T> {
if let Err(e) = ret {
e.to_blk_status()
} else {
- bindings::BLK_STS_OK as _
+ bindings::BLK_STS_OK as u8
}
}
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 10c6d69be7f3..bcf2b73d9189 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -125,7 +125,7 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
// success of the call to `try_set_end` guarantees that there are no
// `ARef`s pointing to this request. Therefore it is safe to hand it
// back to the block layer.
- unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as _) };
+ unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as u8) };
Ok(())
}
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index e5859217a579..4063f09d76d9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -82,7 +82,7 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
unsafe {
raw_ids[i]
.as_mut_ptr()
- .byte_offset(T::DRIVER_DATA_OFFSET as _)
+ .byte_add(T::DRIVER_DATA_OFFSET)
.cast::<usize>()
.write(i);
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 598001157293..34571f992f0d 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -45,7 +45,7 @@ struct DevresInner<T> {
/// # Example
///
/// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -59,19 +59,19 @@ struct DevresInner<T> {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -115,8 +115,9 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
// SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
// detached.
- let ret =
- unsafe { bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data as _) };
+ let ret = unsafe {
+ bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data.cast_mut().cast())
+ };
if ret != 0 {
// SAFETY: We just created another reference to `inner` in order to pass it to
@@ -130,7 +131,7 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
}
fn as_ptr(&self) -> *const Self {
- self as _
+ self
}
fn remove_action(this: &Arc<Self>) {
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 8c62cff8742e..f4482c9a8d82 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
+ unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d4a73e52e3ee..9d2aadf40edf 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -5,7 +5,7 @@
//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
use crate::error::{code::EINVAL, Result};
-use crate::{bindings, build_assert};
+use crate::{bindings, build_assert, ffi::c_void};
/// Raw representation of an MMIO region.
///
@@ -56,7 +56,7 @@ pub fn maxsize(&self) -> usize {
/// # Examples
///
/// ```no_run
-/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -70,19 +70,19 @@ pub fn maxsize(&self) -> usize {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(addr as _) }
+ unsafe { bindings::$name(addr as *const c_void) }
}
/// Read IO data from a given offset.
@@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$name(addr as _) })
+ Ok(unsafe { bindings::$name(addr as *const c_void) })
}
};
}
@@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _, ) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
}
/// Write IO data from a given offset.
@@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
Ok(())
}
};
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d..2c66e926bffb 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -33,7 +33,7 @@ impl MiscDeviceOptions {
pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
// SAFETY: All zeros is valid for this C type.
let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
- result.minor = bindings::MISC_DYNAMIC_MINOR as _;
+ result.minor = bindings::MISC_DYNAMIC_MINOR as i32;
result.name = self.name.as_char_ptr();
result.fops = create_vtable::<T>();
result
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 04f2d8ef29cb..40d1bd13682c 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- self.0.data as _
+ self.0.data as usize
}
}
@@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
// SAFETY: FFI type is valid to be zero-initialized.
let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
- // TODO: Use `clone_from_slice` once the corresponding types do match.
+ // TODO: Use `copy_from_slice` once stabilized for `const`.
let mut i = 0;
while i < src.len() {
- of.compatible[i] = src[i] as _;
+ of.compatible[i] = src[i];
i += 1;
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 63218abb7a25..a925732f6c7a 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -166,7 +166,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
fn index(&self) -> usize {
- self.0.driver_data as _
+ self.0.driver_data
}
}
@@ -201,7 +201,10 @@ macro_rules! pci_device_table {
/// MODULE_PCI_TABLE,
/// <MyDriver as pci::Driver>::IdInfo,
/// [
-/// (pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as _), ())
+/// (
+/// pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as u32),
+/// (),
+/// )
/// ]
/// );
///
@@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
// `ioptr` is valid by the safety requirements.
// `num` is valid by the safety requirements.
unsafe {
- bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
+ bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
bindings::pci_release_region(pdev.as_raw(), num);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6a1a982b946d..0b80a119d5f0 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -692,9 +692,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: pos as _,
- pos: pos as _,
- end: end as _,
+ beg: pos as usize,
+ pos: pos as usize,
+ end: end as usize,
}
}
@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
///
/// N.B. It may point to invalid memory.
pub(crate) fn pos(&self) -> *mut u8 {
- self.pos as _
+ self.pos as *mut u8
}
/// Returns the number of bytes written to the formatter.
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 51eb40d0abaa..954ab4d88f3d 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -198,7 +198,7 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
unsafe {
w.__enqueue(move |work_ptr| {
bindings::queue_work_on(
- bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
+ bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32,
queue_ptr,
work_ptr,
)
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 12:28 [PATCH v3 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (4 preceding siblings ...)
2025-03-14 12:28 ` [PATCH v3 5/6] rust: enable `clippy::as_underscore` lint Tamir Duberstein
@ 2025-03-14 12:28 ` Tamir Duberstein
2025-03-14 20:18 ` Benno Lossin
2025-03-14 21:54 ` Boqun Feng
5 siblings, 2 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 12:28 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
Rust 1.84.0 stabilized the strict provenance APIs[1].
This patch enables the (unstable) lints `fuzzy_provenance_casts` and
`lossy_provenance_casts` (available since Rust 1.61.0[2]) and uses
strict provenance APIs where these lints triggered. The `kernel` crate
is kept backwards-compatible by introducing forwarding functions at the
root which are marked `#[allow(clippy::incompatible_msrv)]` to avoid
warnings on rustc < 1.84.0.
The discussion in the tracking Issue for strict_provenance_lints[3]
seems to be nearing resolution with the only open question being:
> do we really want two separate lints for the two directions?
which seems minor enough that this is unlikely to cause significant
churn when stabilized.
This is limited to the `kernel` crate because adding these lints in the
root `Makefile` causes `core` itself to be compiled with them, which in
turn causes warnings on the implementations of the strict provenance
APIs themselves.
Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
Link: https://github.com/rust-lang/rust/blob/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/compiler/rustc_feature/src/unstable.rs#L605 [2]
Link: https://github.com/rust-lang/rust/issues/130351 [3]
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
---
Makefile | 9 ++++++++-
init/Kconfig | 3 +++
rust/Makefile | 26 ++++++++++++++++++++------
rust/kernel/alloc.rs | 2 +-
rust/kernel/devres.rs | 4 ++--
rust/kernel/io.rs | 14 +++++++-------
rust/kernel/lib.rs | 20 ++++++++++++++++++++
rust/kernel/of.rs | 2 +-
rust/kernel/pci.rs | 4 ++--
rust/kernel/str.rs | 16 ++++++----------
rust/kernel/uaccess.rs | 12 ++++++++----
scripts/Makefile.build | 2 +-
scripts/Makefile.host | 4 ++++
13 files changed, 83 insertions(+), 35 deletions(-)
diff --git a/Makefile b/Makefile
index 2af40bfed9ce..bc12650783f1 100644
--- a/Makefile
+++ b/Makefile
@@ -473,6 +473,8 @@ export rust_common_flags := --edition=2021 \
-Astable_features \
-Dnon_ascii_idents \
-Dunsafe_op_in_unsafe_fn \
+ -Wfuzzy_provenance_casts \
+ -Wlossy_provenance_casts \
-Wmissing_docs \
-Wrust_2018_idioms \
-Wunreachable_pub \
@@ -498,7 +500,7 @@ KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) \
-I $(srctree)/scripts/include
KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
- -Zallow-features= $(HOSTRUSTFLAGS)
+ $(HOSTRUSTFLAGS)
KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
KBUILD_PROCMACROLDFLAGS := $(or $(PROCMACROLDFLAGS),$(KBUILD_HOSTLDFLAGS))
@@ -870,6 +872,11 @@ KBUILD_CFLAGS += -Os
KBUILD_RUSTFLAGS += -Copt-level=s
endif
+# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
+#
+# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
+export rustc_strict_provenance_feature := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
+
# Always set `debug-assertions` and `overflow-checks` because their default
# depends on `opt-level` and `debug-assertions`, respectively.
KBUILD_RUSTFLAGS += -Cdebug-assertions=$(if $(CONFIG_RUST_DEBUG_ASSERTIONS),y,n)
diff --git a/init/Kconfig b/init/Kconfig
index 324c2886b2ea..04df2893348c 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
config RUSTC_HAS_COERCE_POINTEE
def_bool RUSTC_VERSION >= 108400
+config RUSTC_HAS_STABLE_STRICT_PROVENANCE
+ def_bool RUSTC_VERSION >= 108400
+
config PAHOLE_VERSION
int
default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/Makefile b/rust/Makefile
index ea3849eb78f6..dad47bea19f3 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -57,10 +57,12 @@ endif
core-cfgs = \
--cfg no_fp_fmt_parse
+rustc_strict_provenance_flags = -Zcrate-attr='feature($(rustc_strict_provenance_feature))'
+
quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
cmd_rustdoc = \
OBJTREE=$(abspath $(objtree)) \
- $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags))) \
+ $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) $(rustc_strict_provenance_flags)) \
$(rustc_target_flags) -L$(objtree)/$(obj) \
-Zunstable-options --generate-link-to-definition \
--output $(rustdoc_output) \
@@ -99,7 +101,7 @@ rustdoc-macros: $(src)/macros/lib.rs FORCE
# Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should
# not be needed -- see https://github.com/rust-lang/rust/pull/128307.
-rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks
+rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks $(rustc_strict_provenance_flags)
rustdoc-core: private rustc_target_flags = $(core-cfgs)
rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
+$(call if_changed,rustdoc)
@@ -122,6 +124,7 @@ quiet_cmd_rustc_test_library = $(RUSTC_OR_CLIPPY_QUIET) TL $<
cmd_rustc_test_library = \
OBJTREE=$(abspath $(objtree)) \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
+ $(rustc_strict_provenance_flags) \
@$(objtree)/include/generated/rustc_cfg $(rustc_target_flags) \
--crate-type $(if $(rustc_test_library_proc),proc-macro,rlib) \
--out-dir $(objtree)/$(obj)/test --cfg testlib \
@@ -155,11 +158,19 @@ rusttestlib-uapi: private rustc_target_flags = --extern ffi
rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi FORCE
+$(call if_changed,rustc_test_library)
+# `rustdoc --test` doesn't respect `-Zcrate-attr`, which means we can't use
+# `rustc_strict_provenance_flags` below. Instead we filter out those lints to avoid unknown lint
+# warnings.
+#
+# See https://github.com/rust-lang/rust/issues/138491.
+rustc_strict_provenance_lints = -Wfuzzy_provenance_casts -Wlossy_provenance_casts
+
quiet_cmd_rustdoc_test = RUSTDOC T $<
cmd_rustdoc_test = \
RUST_MODFILE=test.rs \
OBJTREE=$(abspath $(objtree)) \
- $(RUSTDOC) --test $(rust_common_flags) \
+ $(RUSTDOC) --test \
+ $(filter-out $(rustc_strict_provenance_lints),$(rust_common_flags)) \
@$(objtree)/include/generated/rustc_cfg \
$(rustc_target_flags) $(rustdoc_test_target_flags) \
$(rustdoc_test_quiet) \
@@ -171,7 +182,8 @@ quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $<
rm -rf $(objtree)/$(obj)/test/doctests/kernel; \
mkdir -p $(objtree)/$(obj)/test/doctests/kernel; \
OBJTREE=$(abspath $(objtree)) \
- $(RUSTDOC) --test $(rust_flags) \
+ $(RUSTDOC) --test \
+ $(filter-out $(rustc_strict_provenance_lints),$(rust_flags)) \
-L$(objtree)/$(obj) --extern ffi --extern kernel \
--extern build_error --extern macros \
--extern bindings --extern uapi \
@@ -193,6 +205,7 @@ quiet_cmd_rustc_test = $(RUSTC_OR_CLIPPY_QUIET) T $<
cmd_rustc_test = \
OBJTREE=$(abspath $(objtree)) \
$(RUSTC_OR_CLIPPY) --test $(rust_common_flags) \
+ $(rustc_strict_provenance_flags) \
@$(objtree)/include/generated/rustc_cfg \
$(rustc_target_flags) --out-dir $(objtree)/$(obj)/test \
-L$(objtree)/$(obj)/test \
@@ -362,6 +375,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
cmd_rustc_procmacro = \
$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
+ $(rustc_strict_provenance_flags) \
-Clinker-flavor=gcc -Clinker=$(HOSTCC) \
-Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \
--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
@@ -376,7 +390,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
cmd_rustc_library = \
OBJTREE=$(abspath $(objtree)) \
$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
- $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
+ $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags) $(rustc_strict_provenance_flags)) \
--emit=dep-info=$(depfile) --emit=obj=$@ \
--emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
--crate-type rlib -L$(objtree)/$(obj) \
@@ -436,7 +450,7 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
$(obj)/exports.o: private skip_gendwarfksyms = 1
$(obj)/core.o: private skip_clippy = 1
-$(obj)/core.o: private skip_flags = -Wunreachable_pub
+$(obj)/core.o: private skip_flags = -Wunreachable_pub -Wlossy_provenance_casts $(rustc_strict_provenance_flags)
$(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
$(obj)/core.o: private rustc_target_flags = $(core-cfgs)
$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index fc9c9c41cd79..59199a6da2ed 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -217,7 +217,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) {
/// Returns a properly aligned dangling pointer from the given `layout`.
pub(crate) fn dangling_from_layout(layout: Layout) -> NonNull<u8> {
- let ptr = layout.align() as *mut u8;
+ let ptr = crate::with_exposed_provenance_mut(layout.align());
// SAFETY: `layout.align()` (and hence `ptr`) is guaranteed to be non-zero.
unsafe { NonNull::new_unchecked(ptr) }
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 34571f992f0d..e8232bb771b2 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -64,14 +64,14 @@ struct DevresInner<T> {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
+/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); };
/// }
/// }
///
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 9d2aadf40edf..0a018ad7478a 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -5,7 +5,7 @@
//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
use crate::error::{code::EINVAL, Result};
-use crate::{bindings, build_assert, ffi::c_void};
+use crate::{bindings, build_assert};
/// Raw representation of an MMIO region.
///
@@ -75,14 +75,14 @@ pub fn maxsize(&self) -> usize {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
+/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); };
/// }
/// }
///
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(addr as *const c_void) }
+ unsafe { bindings::$name(crate::with_exposed_provenance(addr)) }
}
/// Read IO data from a given offset.
@@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$name(addr as *const c_void) })
+ Ok(unsafe { bindings::$name(crate::with_exposed_provenance(addr)) })
}
};
}
@@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as *mut c_void) }
+ unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) }
}
/// Write IO data from a given offset.
@@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as *mut c_void) }
+ unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) }
Ok(())
}
};
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 9cd6b6864739..ebf7db3ad9ee 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -25,6 +25,26 @@
#![feature(const_ptr_write)]
#![feature(const_refs_to_cell)]
+#[allow(clippy::incompatible_msrv)]
+mod strict_provenance {
+ #[doc(hidden)]
+ pub fn expose_provenance<T>(addr: *const T) -> usize {
+ addr.expose_provenance()
+ }
+
+ #[doc(hidden)]
+ pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
+ core::ptr::with_exposed_provenance(addr)
+ }
+
+ #[doc(hidden)]
+ pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
+ core::ptr::with_exposed_provenance_mut(addr)
+ }
+}
+
+pub use strict_provenance::*;
+
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
#[cfg(not(CONFIG_RUST))]
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 40d1bd13682c..f9459694cbdc 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- self.0.data as usize
+ crate::expose_provenance(self.0.data)
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index a925732f6c7a..bb38d83e2608 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
// `pdev` is valid by the invariants of `Device`.
// `num` is checked for validity by a previous call to `Device::resource_len`.
// `name` is always valid.
- let ioptr: usize = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) } as usize;
+ let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
if ioptr == 0 {
// SAFETY:
// `pdev` valid by the invariants of `Device`.
@@ -320,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
// `ioptr` is valid by the safety requirements.
// `num` is valid by the safety requirements.
unsafe {
- bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
+ bindings::pci_iounmap(pdev.as_raw(), crate::with_exposed_provenance_mut(ioptr));
bindings::pci_release_region(pdev.as_raw(), num);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 0b80a119d5f0..6bc6357293e4 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -692,9 +692,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: pos as usize,
- pos: pos as usize,
- end: end as usize,
+ beg: crate::expose_provenance(pos),
+ pos: crate::expose_provenance(pos),
+ end: crate::expose_provenance(end),
}
}
@@ -705,7 +705,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
/// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
/// for the lifetime of the returned [`RawFormatter`].
pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
- let pos = buf as usize;
+ let pos = crate::expose_provenance(buf);
// INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
// guarantees that the memory region is valid for writes.
Self {
@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
///
/// N.B. It may point to invalid memory.
pub(crate) fn pos(&self) -> *mut u8 {
- self.pos as *mut u8
+ crate::with_exposed_provenance_mut(self.pos)
}
/// Returns the number of bytes written to the formatter.
@@ -741,11 +741,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
// SAFETY: If `len_to_copy` is non-zero, then we know `pos` has not gone past `end`
// yet, so it is valid for write per the type invariants.
unsafe {
- core::ptr::copy_nonoverlapping(
- s.as_bytes().as_ptr(),
- self.pos as *mut u8,
- len_to_copy,
- )
+ core::ptr::copy_nonoverlapping(s.as_bytes().as_ptr(), self.pos(), len_to_copy)
};
}
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 719b0a48ff55..96393bcf6bd7 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
}
// SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
// that many bytes to it.
- let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
+ let res = unsafe {
+ bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
+ };
if res != 0 {
return Err(EFAULT);
}
@@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
let res = unsafe {
bindings::_copy_from_user(
out.as_mut_ptr().cast::<c_void>(),
- self.ptr as *const c_void,
+ crate::with_exposed_provenance(self.ptr),
len,
)
};
@@ -330,7 +332,9 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
}
// SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
// that many bytes from it.
- let res = unsafe { bindings::copy_to_user(self.ptr as *mut c_void, data_ptr, len) };
+ let res = unsafe {
+ bindings::copy_to_user(crate::with_exposed_provenance_mut(self.ptr), data_ptr, len)
+ };
if res != 0 {
return Err(EFAULT);
}
@@ -357,7 +361,7 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result {
// is a compile-time constant.
let res = unsafe {
bindings::_copy_to_user(
- self.ptr as *mut c_void,
+ crate::with_exposed_provenance_mut(self.ptr),
(value as *const T).cast::<c_void>(),
len,
)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 993708d11874..34575f3be0fc 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -226,7 +226,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
# Compile Rust sources (.rs)
# ---------------------------------------------------------------------------
-rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons
+rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,$(rustc_strict_provenance_feature)
# `--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
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index c1dedf646a39..7e2f35bff59c 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -87,10 +87,14 @@ hostcxx_flags = -Wp,-MMD,$(depfile) \
$(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \
$(HOSTCXXFLAGS_$(target-stem).o)
+rust_allowed_features := $(rustc_strict_provenance_feature)
+
# `--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.
hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
+ -Zallow-features=$(rust_allowed_features) \
+ -Zcrate-attr='feature($(rust_allowed_features))' \
-Clinker-flavor=gcc -Clinker=$(HOSTCC) \
-Clink-args='$(call escsq,$(KBUILD_HOSTLDFLAGS))' \
$(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 12:28 ` [PATCH v3 6/6] rust: use strict provenance APIs Tamir Duberstein
@ 2025-03-14 20:18 ` Benno Lossin
2025-03-14 22:00 ` Miguel Ojeda
2025-03-14 21:54 ` Boqun Feng
1 sibling, 1 reply; 19+ messages in thread
From: Benno Lossin @ 2025-03-14 20:18 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Fri Mar 14, 2025 at 1:28 PM CET, Tamir Duberstein wrote:
> Rust 1.84.0 stabilized the strict provenance APIs[1].
>
> This patch enables the (unstable) lints `fuzzy_provenance_casts` and
> `lossy_provenance_casts` (available since Rust 1.61.0[2]) and uses
> strict provenance APIs where these lints triggered. The `kernel` crate
> is kept backwards-compatible by introducing forwarding functions at the
> root which are marked `#[allow(clippy::incompatible_msrv)]` to avoid
> warnings on rustc < 1.84.0.
>
> The discussion in the tracking Issue for strict_provenance_lints[3]
> seems to be nearing resolution with the only open question being:
>
>> do we really want two separate lints for the two directions?
>
> which seems minor enough that this is unlikely to cause significant
> churn when stabilized.
>
> This is limited to the `kernel` crate because adding these lints in the
> root `Makefile` causes `core` itself to be compiled with them, which in
> turn causes warnings on the implementations of the strict provenance
> APIs themselves.
This isn't the case anymore? (ie it isn't limited to the `kernel`
crate?)
> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> Link: https://github.com/rust-lang/rust/blob/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/compiler/rustc_feature/src/unstable.rs#L605 [2]
> Link: https://github.com/rust-lang/rust/issues/130351 [3]
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
Missing SoB.
> ---
> Makefile | 9 ++++++++-
> init/Kconfig | 3 +++
> rust/Makefile | 26 ++++++++++++++++++++------
> rust/kernel/alloc.rs | 2 +-
> rust/kernel/devres.rs | 4 ++--
> rust/kernel/io.rs | 14 +++++++-------
> rust/kernel/lib.rs | 20 ++++++++++++++++++++
> rust/kernel/of.rs | 2 +-
> rust/kernel/pci.rs | 4 ++--
> rust/kernel/str.rs | 16 ++++++----------
> rust/kernel/uaccess.rs | 12 ++++++++----
> scripts/Makefile.build | 2 +-
> scripts/Makefile.host | 4 ++++
> 13 files changed, 83 insertions(+), 35 deletions(-)
Thanks for making the effort and getting this to work, unfortunately, I
have checked if this compiles with 1.78 and sadly it doesn't. That's
because that version doesn't have the `with_exposed_provenance`
function! I am very sorry about that, I thought I had checked that
before suggesting it to you. It does exist in >=1.79 && <1.84 under a
different feature: `exposed_provenance`. (I haven't checked if
everything works when also adding that)
I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is
going to be in debian trixie, so eventually we could bump it to that,
but I'm not sure what the time frame will be for that.
Maybe we can salvage this effort by gating both the lint and the
unstable features on the versions where it works? @Miguel, what's your
opinion?
We could even make it simple, requiring 1.84 and not bothering with the
older versions.
> diff --git a/Makefile b/Makefile
> index 2af40bfed9ce..bc12650783f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -473,6 +473,8 @@ export rust_common_flags := --edition=2021 \
> -Astable_features \
> -Dnon_ascii_idents \
> -Dunsafe_op_in_unsafe_fn \
> + -Wfuzzy_provenance_casts \
> + -Wlossy_provenance_casts \
> -Wmissing_docs \
> -Wrust_2018_idioms \
> -Wunreachable_pub \
> @@ -498,7 +500,7 @@ KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \
> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) \
> -I $(srctree)/scripts/include
> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> - -Zallow-features= $(HOSTRUSTFLAGS)
> + $(HOSTRUSTFLAGS)
This should be mentioned explicitly in the commit message.
> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
> KBUILD_PROCMACROLDFLAGS := $(or $(PROCMACROLDFLAGS),$(KBUILD_HOSTLDFLAGS))
> @@ -870,6 +872,11 @@ KBUILD_CFLAGS += -Os
> KBUILD_RUSTFLAGS += -Copt-level=s
> endif
>
> +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized.
> +#
> +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64.
> +export rustc_strict_provenance_feature := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance)
> +
> # Always set `debug-assertions` and `overflow-checks` because their default
> # depends on `opt-level` and `debug-assertions`, respectively.
> KBUILD_RUSTFLAGS += -Cdebug-assertions=$(if $(CONFIG_RUST_DEBUG_ASSERTIONS),y,n)
> diff --git a/init/Kconfig b/init/Kconfig
> index 324c2886b2ea..04df2893348c 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
> config RUSTC_HAS_COERCE_POINTEE
> def_bool RUSTC_VERSION >= 108400
>
> +config RUSTC_HAS_STABLE_STRICT_PROVENANCE
> + def_bool RUSTC_VERSION >= 108400
> +
> config PAHOLE_VERSION
> int
> default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/Makefile b/rust/Makefile
> index ea3849eb78f6..dad47bea19f3 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -57,10 +57,12 @@ endif
> core-cfgs = \
> --cfg no_fp_fmt_parse
>
> +rustc_strict_provenance_flags = -Zcrate-attr='feature($(rustc_strict_provenance_feature))'
> +
> quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
> cmd_rustdoc = \
> OBJTREE=$(abspath $(objtree)) \
> - $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags))) \
> + $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) $(rustc_strict_provenance_flags)) \
> $(rustc_target_flags) -L$(objtree)/$(obj) \
> -Zunstable-options --generate-link-to-definition \
> --output $(rustdoc_output) \
> @@ -99,7 +101,7 @@ rustdoc-macros: $(src)/macros/lib.rs FORCE
>
> # Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should
> # not be needed -- see https://github.com/rust-lang/rust/pull/128307.
> -rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks
> +rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks $(rustc_strict_provenance_flags)
Why aren't the lints excluded?
> rustdoc-core: private rustc_target_flags = $(core-cfgs)
> rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
> +$(call if_changed,rustdoc)
> @@ -436,7 +450,7 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE
> $(obj)/exports.o: private skip_gendwarfksyms = 1
>
> $(obj)/core.o: private skip_clippy = 1
> -$(obj)/core.o: private skip_flags = -Wunreachable_pub
> +$(obj)/core.o: private skip_flags = -Wunreachable_pub -Wlossy_provenance_casts $(rustc_strict_provenance_flags)
Why not also the `-Wfuzzy_provenance_casts`? They could be introduced at
any moment in core, so I don't think that we should enable the lint
there.
> $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
> $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
> $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 9cd6b6864739..ebf7db3ad9ee 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -25,6 +25,26 @@
> #![feature(const_ptr_write)]
> #![feature(const_refs_to_cell)]
>
> +#[allow(clippy::incompatible_msrv)]
> +mod strict_provenance {
> + #[doc(hidden)]
> + pub fn expose_provenance<T>(addr: *const T) -> usize {
> + addr.expose_provenance()
> + }
> +
> + #[doc(hidden)]
> + pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
> + core::ptr::with_exposed_provenance(addr)
> + }
> +
> + #[doc(hidden)]
> + pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
> + core::ptr::with_exposed_provenance_mut(addr)
> + }
> +}
Still, I don't think we should re-export them. It'll only confuse folks
and the `incompatible_msrv` lint isn't useful at the moment.
> +
> +pub use strict_provenance::*;
> +
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> #[cfg(not(CONFIG_RUST))]
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 0b80a119d5f0..6bc6357293e4 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -692,9 +692,9 @@ fn new() -> Self {
> pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> // INVARIANT: The safety requirements guarantee the type invariants.
> Self {
> - beg: pos as usize,
> - pos: pos as usize,
> - end: end as usize,
> + beg: crate::expose_provenance(pos),
> + pos: crate::expose_provenance(pos),
> + end: crate::expose_provenance(end),
Just import it, you're also using it below in the file.
---
Cheers,
Benno
> }
> }
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 20:18 ` Benno Lossin
@ 2025-03-14 22:00 ` Miguel Ojeda
2025-03-14 22:20 ` Tamir Duberstein
0 siblings, 1 reply; 19+ messages in thread
From: Miguel Ojeda @ 2025-03-14 22:00 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is
> going to be in debian trixie, so eventually we could bump it to that,
> but I'm not sure what the time frame will be for that.
>
> Maybe we can salvage this effort by gating both the lint and the
> unstable features on the versions where it works? @Miguel, what's your
> opinion?
>
> We could even make it simple, requiring 1.84 and not bothering with the
> older versions.
Regarding Debian Trixie: unknown, since my understanding is that it
does not have a release date yet, but apparently mid May is the Hard
Freeze and then it may take e.g. a month or two to the release.
And when it releases, we may want to wait a while before bumping it,
depending on how much time has passed since Rust 1.85.0 and depending
on whether we managed to get e.g. Ubuntu LTSs to provide a versioned
package etc.
If something simple works, then let's just go for that -- we do not
care too much about older versions for linting purposes, since people
should be testing with the latest stable too anyway.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 22:00 ` Miguel Ojeda
@ 2025-03-14 22:20 ` Tamir Duberstein
2025-03-15 9:44 ` Benno Lossin
0 siblings, 1 reply; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-14 22:20 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is
> > going to be in debian trixie, so eventually we could bump it to that,
> > but I'm not sure what the time frame will be for that.
> >
> > Maybe we can salvage this effort by gating both the lint and the
> > unstable features on the versions where it works? @Miguel, what's your
> > opinion?
> >
> > We could even make it simple, requiring 1.84 and not bothering with the
> > older versions.
>
> Regarding Debian Trixie: unknown, since my understanding is that it
> does not have a release date yet, but apparently mid May is the Hard
> Freeze and then it may take e.g. a month or two to the release.
>
> And when it releases, we may want to wait a while before bumping it,
> depending on how much time has passed since Rust 1.85.0 and depending
> on whether we managed to get e.g. Ubuntu LTSs to provide a versioned
> package etc.
>
> If something simple works, then let's just go for that -- we do not
> care too much about older versions for linting purposes, since people
> should be testing with the latest stable too anyway.
It's not going to be simple because `rust_common_flags` is defined
before the config is read, which means I'll have to sprinkle
conditional logic in even more places to enable the lints.
The most minimal version of this patch would drop all the build system
changes and just have conditionally compiled polyfills for the strict
provenance APIs. Are folks OK with that?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 22:20 ` Tamir Duberstein
@ 2025-03-15 9:44 ` Benno Lossin
2025-03-15 11:40 ` Tamir Duberstein
0 siblings, 1 reply; 19+ messages in thread
From: Benno Lossin @ 2025-03-15 9:44 UTC (permalink / raw)
To: Tamir Duberstein, Miguel Ojeda
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Fri Mar 14, 2025 at 11:20 PM CET, Tamir Duberstein wrote:
> On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >
>> > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is
>> > going to be in debian trixie, so eventually we could bump it to that,
>> > but I'm not sure what the time frame will be for that.
>> >
>> > Maybe we can salvage this effort by gating both the lint and the
>> > unstable features on the versions where it works? @Miguel, what's your
>> > opinion?
>> >
>> > We could even make it simple, requiring 1.84 and not bothering with the
>> > older versions.
>>
>> Regarding Debian Trixie: unknown, since my understanding is that it
>> does not have a release date yet, but apparently mid May is the Hard
>> Freeze and then it may take e.g. a month or two to the release.
>>
>> And when it releases, we may want to wait a while before bumping it,
>> depending on how much time has passed since Rust 1.85.0 and depending
>> on whether we managed to get e.g. Ubuntu LTSs to provide a versioned
>> package etc.
Yeah that's what I thought, thanks for confirming.
>> If something simple works, then let's just go for that -- we do not
>> care too much about older versions for linting purposes, since people
>> should be testing with the latest stable too anyway.
>
> It's not going to be simple because `rust_common_flags` is defined
> before the config is read, which means I'll have to sprinkle
> conditional logic in even more places to enable the lints.
>
> The most minimal version of this patch would drop all the build system
> changes and just have conditionally compiled polyfills for the strict
> provenance APIs. Are folks OK with that?
So you'd not enable the lint, but fix all occurrences? I think we should
still have the lint (if it's too cumbersome, then let's only enable it
in the kernel crate).
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 9:44 ` Benno Lossin
@ 2025-03-15 11:40 ` Tamir Duberstein
0 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-15 11:40 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sat, Mar 15, 2025 at 5:44 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Fri Mar 14, 2025 at 11:20 PM CET, Tamir Duberstein wrote:
> > On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda
> > <miguel.ojeda.sandonis@gmail.com> wrote:
> >>
> >> On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >
> >> > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is
> >> > going to be in debian trixie, so eventually we could bump it to that,
> >> > but I'm not sure what the time frame will be for that.
> >> >
> >> > Maybe we can salvage this effort by gating both the lint and the
> >> > unstable features on the versions where it works? @Miguel, what's your
> >> > opinion?
> >> >
> >> > We could even make it simple, requiring 1.84 and not bothering with the
> >> > older versions.
> >>
> >> Regarding Debian Trixie: unknown, since my understanding is that it
> >> does not have a release date yet, but apparently mid May is the Hard
> >> Freeze and then it may take e.g. a month or two to the release.
> >>
> >> And when it releases, we may want to wait a while before bumping it,
> >> depending on how much time has passed since Rust 1.85.0 and depending
> >> on whether we managed to get e.g. Ubuntu LTSs to provide a versioned
> >> package etc.
>
> Yeah that's what I thought, thanks for confirming.
>
> >> If something simple works, then let's just go for that -- we do not
> >> care too much about older versions for linting purposes, since people
> >> should be testing with the latest stable too anyway.
> >
> > It's not going to be simple because `rust_common_flags` is defined
> > before the config is read, which means I'll have to sprinkle
> > conditional logic in even more places to enable the lints.
> >
> > The most minimal version of this patch would drop all the build system
> > changes and just have conditionally compiled polyfills for the strict
> > provenance APIs. Are folks OK with that?
>
> So you'd not enable the lint, but fix all occurrences? I think we should
> still have the lint (if it's too cumbersome, then let's only enable it
> in the kernel crate).
👍
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 12:28 ` [PATCH v3 6/6] rust: use strict provenance APIs Tamir Duberstein
2025-03-14 20:18 ` Benno Lossin
@ 2025-03-14 21:54 ` Boqun Feng
2025-03-15 9:34 ` Benno Lossin
1 sibling, 1 reply; 19+ messages in thread
From: Boqun Feng @ 2025-03-14 21:54 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Fri, Mar 14, 2025 at 08:28:10AM -0400, Tamir Duberstein wrote:
[...]
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -217,7 +217,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) {
>
> /// Returns a properly aligned dangling pointer from the given `layout`.
> pub(crate) fn dangling_from_layout(layout: Layout) -> NonNull<u8> {
> - let ptr = layout.align() as *mut u8;
> + let ptr = crate::with_exposed_provenance_mut(layout.align());
Dangling pointers don't have provenance, neither has its provenance been
exposed. I think should use `without_provenance_mut()` here:
https://doc.rust-lang.org/std/ptr/fn.without_provenance_mut.html
see also the source of core::ptr::dangling().
The rest Rust code changes look good to me. Although I would suggest you
to split this patch into several patches: you can do the conversion from
"as" pattern to provenance API one file by one file, and this make it
easier for people to review. And after the conversions are done, you can
introduce the Makefile changes.
Regards,
Boqun
[...]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-14 21:54 ` Boqun Feng
@ 2025-03-15 9:34 ` Benno Lossin
2025-03-15 12:37 ` Boqun Feng
0 siblings, 1 reply; 19+ messages in thread
From: Benno Lossin @ 2025-03-15 9:34 UTC (permalink / raw)
To: Boqun Feng, Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, Brendan Higgins, David Gow, Rae Moar,
Bjorn Helgaas, Luis Chamberlain, Russ Weight, Rob Herring,
Saravana Kannan, linux-kbuild, linux-kernel, rust-for-linux,
linux-kselftest, kunit-dev, linux-pci, linux-block, devicetree
On Fri Mar 14, 2025 at 10:54 PM CET, Boqun Feng wrote:
> On Fri, Mar 14, 2025 at 08:28:10AM -0400, Tamir Duberstein wrote:
> [...]
>> --- a/rust/kernel/alloc.rs
>> +++ b/rust/kernel/alloc.rs
>> @@ -217,7 +217,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) {
>>
>> /// Returns a properly aligned dangling pointer from the given `layout`.
>> pub(crate) fn dangling_from_layout(layout: Layout) -> NonNull<u8> {
>> - let ptr = layout.align() as *mut u8;
>> + let ptr = crate::with_exposed_provenance_mut(layout.align());
>
> Dangling pointers don't have provenance, neither has its provenance been
> exposed. I think should use `without_provenance_mut()` here:
>
> https://doc.rust-lang.org/std/ptr/fn.without_provenance_mut.html
>
> see also the source of core::ptr::dangling().
Good catch.
> The rest Rust code changes look good to me. Although I would suggest you
> to split this patch into several patches: you can do the conversion from
> "as" pattern to provenance API one file by one file, and this make it
> easier for people to review. And after the conversions are done, you can
> introduce the Makefile changes.
I think it's fine to do several of the `as` conversions in a single
patch, but splitting off the makefile changes is a good idea.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 9:34 ` Benno Lossin
@ 2025-03-15 12:37 ` Boqun Feng
2025-03-15 12:41 ` Tamir Duberstein
2025-03-15 18:00 ` Benno Lossin
0 siblings, 2 replies; 19+ messages in thread
From: Boqun Feng @ 2025-03-15 12:37 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote:
[...]
> > The rest Rust code changes look good to me. Although I would suggest you
> > to split this patch into several patches: you can do the conversion from
> > "as" pattern to provenance API one file by one file, and this make it
> > easier for people to review. And after the conversions are done, you can
> > introduce the Makefile changes.
>
> I think it's fine to do several of the `as` conversions in a single
Well, "fine" != "recommended", right? ;-) If the patch was split,
reviewers would be able to give Reviewed-by to individual patches that
looks fine trivially. Then it's easier to make progress every iteration,
and also allows partially applying the changes. Of course it doesn't
have to be file-by-file.
Regards,
Boqun
> patch, but splitting off the makefile changes is a good idea.
>
> ---
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 12:37 ` Boqun Feng
@ 2025-03-15 12:41 ` Tamir Duberstein
2025-03-15 12:59 ` Boqun Feng
2025-03-15 18:00 ` Benno Lossin
1 sibling, 1 reply; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-15 12:41 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote:
> [...]
> > > The rest Rust code changes look good to me. Although I would suggest you
> > > to split this patch into several patches: you can do the conversion from
> > > "as" pattern to provenance API one file by one file, and this make it
> > > easier for people to review. And after the conversions are done, you can
> > > introduce the Makefile changes.
> >
> > I think it's fine to do several of the `as` conversions in a single
>
> Well, "fine" != "recommended", right? ;-) If the patch was split,
> reviewers would be able to give Reviewed-by to individual patches that
> looks fine trivially. Then it's easier to make progress every iteration,
> and also allows partially applying the changes. Of course it doesn't
> have to be file-by-file.
I sent v4 a little while ago, hopefully the resulting complexity is
manageable now that the build system is untouched.
Cheers.
Tamir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 12:41 ` Tamir Duberstein
@ 2025-03-15 12:59 ` Boqun Feng
2025-03-15 14:52 ` Tamir Duberstein
0 siblings, 1 reply; 19+ messages in thread
From: Boqun Feng @ 2025-03-15 12:59 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Sat, Mar 15, 2025 at 08:41:49AM -0400, Tamir Duberstein wrote:
> On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote:
> > [...]
> > > > The rest Rust code changes look good to me. Although I would suggest you
> > > > to split this patch into several patches: you can do the conversion from
> > > > "as" pattern to provenance API one file by one file, and this make it
> > > > easier for people to review. And after the conversions are done, you can
> > > > introduce the Makefile changes.
> > >
> > > I think it's fine to do several of the `as` conversions in a single
> >
> > Well, "fine" != "recommended", right? ;-) If the patch was split,
> > reviewers would be able to give Reviewed-by to individual patches that
> > looks fine trivially. Then it's easier to make progress every iteration,
> > and also allows partially applying the changes. Of course it doesn't
> > have to be file-by-file.
>
> I sent v4 a little while ago, hopefully the resulting complexity is
> manageable now that the build system is untouched.
>
I have fun plans today (skiing!), so won't be able to take another
detailed look. What I was trying to say is that: should you split the
patches, I would have already given some Reviewed-bys ;-) But as Benno
said, it's fine, so don't worry, I will take another look later. Thanks!
Regards,
Boqun
> Cheers.
>
> Tamir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 12:59 ` Boqun Feng
@ 2025-03-15 14:52 ` Tamir Duberstein
0 siblings, 0 replies; 19+ messages in thread
From: Tamir Duberstein @ 2025-03-15 14:52 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Sat, Mar 15, 2025 at 8:59 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Sat, Mar 15, 2025 at 08:41:49AM -0400, Tamir Duberstein wrote:
> > On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote:
> > > [...]
> > > > > The rest Rust code changes look good to me. Although I would suggest you
> > > > > to split this patch into several patches: you can do the conversion from
> > > > > "as" pattern to provenance API one file by one file, and this make it
> > > > > easier for people to review. And after the conversions are done, you can
> > > > > introduce the Makefile changes.
> > > >
> > > > I think it's fine to do several of the `as` conversions in a single
> > >
> > > Well, "fine" != "recommended", right? ;-) If the patch was split,
> > > reviewers would be able to give Reviewed-by to individual patches that
> > > looks fine trivially. Then it's easier to make progress every iteration,
> > > and also allows partially applying the changes. Of course it doesn't
> > > have to be file-by-file.
> >
> > I sent v4 a little while ago, hopefully the resulting complexity is
> > manageable now that the build system is untouched.
> >
>
> I have fun plans today (skiing!), so won't be able to take another
> detailed look. What I was trying to say is that: should you split the
> patches, I would have already given some Reviewed-bys ;-) But as Benno
> said, it's fine, so don't worry, I will take another look later. Thanks!
Have fun! ⛷️
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 6/6] rust: use strict provenance APIs
2025-03-15 12:37 ` Boqun Feng
2025-03-15 12:41 ` Tamir Duberstein
@ 2025-03-15 18:00 ` Benno Lossin
1 sibling, 0 replies; 19+ messages in thread
From: Benno Lossin @ 2025-03-15 18:00 UTC (permalink / raw)
To: Boqun Feng
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Sat Mar 15, 2025 at 1:37 PM CET, Boqun Feng wrote:
> On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote:
> [...]
>> > The rest Rust code changes look good to me. Although I would suggest you
>> > to split this patch into several patches: you can do the conversion from
>> > "as" pattern to provenance API one file by one file, and this make it
>> > easier for people to review. And after the conversions are done, you can
>> > introduce the Makefile changes.
>>
>> I think it's fine to do several of the `as` conversions in a single
>
> Well, "fine" != "recommended", right? ;-) If the patch was split,
> reviewers would be able to give Reviewed-by to individual patches that
> looks fine trivially. Then it's easier to make progress every iteration,
> and also allows partially applying the changes. Of course it doesn't
> have to be file-by-file.
While I see your point, in this case splitting file-by-file is too much.
v4 has: 9 files changed, 82 insertions(+), 27 deletions(-). I've seen
much bigger changes that do smaller things like this patch.
At around 150 lines added + deleted I find it more and more difficult.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 19+ messages in thread