Re: [PATCH V8 00/14] Rust bindings for cpufreq and OPP core + sample driver

Miguel Ojeda
2025-02-17T08:39:21Z

On Mon, Feb 10, 2025 at 9:06 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> error[E0133]: use of mutable static is unsafe and requires unsafe block
> --> /mnt/ssd/all/work/repos/kernel/linux/rust/kernel/cpufreq.rs:632:26
> |
> 632 | addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as *mut _;
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use of mutable static

Ah, I see now -- yeah, this is due to:

https://blog.rust-lang.org/2024/10/17/Rust-1.82.0.html#safely-addressing-unsafe-statics

You could do (probably with a comment):

pub fn new(name: &'static CStr, data: T::Data, flags: u16,
boost: bool) -> Result<Self> {
+ #![allow(unused_unsafe)]
+
let mut drv = KBox::new(

Yeah, a bit annoying... :(

> I don't remember seeing a CLIPPY warning ever about removing the
> unsafe here, as reported in your case.

Please use a newer version to see them, e.g. the latest stable 1.84.1.

In general, please test patches with the minimum version and the
latest stable. The latest will give you more lints in general, and the
minimum will make sure it builds for older versions.

> It would require clippy::undocumented-unsafe-blocks to be disabled, in
> this case.

Hmm... why? We need the `allow` above because we need to keep the
`unsafe` for older Rust. But you can provide a comment nevertheless,
even if "dummy", so you should not need to disable anything else.

With your branch + the `allow` above + running `rustfmt`, it is Clippy
clean for me. Please see the diff below as an example (I also cleaned
the other Clippy warning -- and sorry for the wrapping).

Cheers,
Miguel

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index d2e7913e170b..e7c62770fc3b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -602,6 +602,8 @@ unsafe impl<T: Driver> Send for Registration<T> {}
impl<T: Driver> Registration<T> {
/// Registers a cpufreq driver with the rest of the kernel.
pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost:
bool) -> Result<Self> {
+ #![allow(unused_unsafe)]
+
let mut drv = KBox::new(
UnsafeCell::new(bindings::cpufreq_driver::default()),
GFP_KERNEL,
@@ -626,19 +628,15 @@ pub fn new(name: &'static CStr, data: T::Data,
flags: u16, boost: bool) -> Resul
let mut attr = KBox::new([ptr::null_mut(); 3], GFP_KERNEL)?;
let mut next = 0;

- // SAFETY: The C code returns a valid pointer here, which is
again passed to the C code in
- // an array.
- attr[next] = unsafe {
- addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs)
as *mut _
- };
+ attr[next] =
+ // SAFETY: ...
+ unsafe {
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_available_freqs) as
*mut _ };
next += 1;

if boost {
- // SAFETY: The C code returns a valid pointer here, which
is again passed to the C code
- // in an array.
- attr[next] = unsafe {
-
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut
_
- };
+ attr[next] =
+ // SAFETY: ...
+ unsafe {
addr_of_mut!(bindings::cpufreq_freq_attr_scaling_boost_freqs) as *mut
_ };
next += 1;
}
attr[next] = ptr::null_mut();
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index b83bd97a4f37..aaf650f46ccb 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -781,7 +781,6 @@ fn drop(&mut self) {
/// represents a pointer that owns a reference count on the OPP.
///
/// A reference to the `OPP`, `&OPP` isn't refcounted by the Rust code.
-
#[repr(transparent)]
pub struct OPP(Opaque<bindings::dev_pm_opp>);