[PATCH v3 0/3] replace `allow(...)` lints with `expect(...)`

[PATCH v3 0/3] replace `allow(...)` lints with `expect(...)`

[Onur Özkan]

Changes in v3:
  - `#![expect(internal_features)]` is reverted from
  `rust/compiler_builtins.rs` which was done in the
  first 2 versions.

Onur Özkan (3):
  replace `#[allow(...)]` with `#[expect(...)]`
  rust: remove `#[allow(clippy::unnecessary_cast)]`
  rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

 drivers/gpu/nova-core/regs.rs       | 2 +-
 rust/kernel/alloc/allocator_test.rs | 2 +-
 rust/kernel/cpufreq.rs              | 1 -
 rust/kernel/devres.rs               | 2 +-
 rust/kernel/driver.rs               | 2 +-
 rust/kernel/drm/ioctl.rs            | 8 ++++----
 rust/kernel/error.rs                | 3 +--
 rust/kernel/init.rs                 | 6 +++---
 rust/kernel/kunit.rs                | 2 +-
 rust/kernel/opp.rs                  | 4 ++--
 rust/kernel/types.rs                | 2 +-
 rust/macros/helpers.rs              | 2 +-
 12 files changed, 17 insertions(+), 19 deletions(-)

--
2.50.0

[PATCH v3 1/3] replace `#[allow(...)]` with `#[expect(...)]`

[Onur Özkan]

This makes it clear that the warning is expected not just
ignored, so we don't end up having various unnecessary
linting rules in the codebase.

Some parts of the codebase already use this approach, this
patch just applies it more broadly.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 drivers/gpu/nova-core/regs.rs       | 2 +-
 rust/kernel/alloc/allocator_test.rs | 2 +-
 rust/kernel/devres.rs               | 2 +-
 rust/kernel/driver.rs               | 2 +-
 rust/kernel/drm/ioctl.rs            | 8 ++++----
 rust/kernel/error.rs                | 2 +-
 rust/kernel/init.rs                 | 6 +++---
 rust/kernel/kunit.rs                | 2 +-
 rust/kernel/opp.rs                  | 4 ++--
 rust/kernel/types.rs                | 2 +-
 rust/macros/helpers.rs              | 2 +-
 11 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 5a1273230306..87e5963f1ebb 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -2,7 +2,7 @@

 // Required to retain the original register names used by OpenRM, which are all capital snake case
 // but are mapped to types.
-#![allow(non_camel_case_types)]
+#![expect(non_camel_case_types)]

 #[macro_use]
 mod macros;
diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs
index d19c06ef0498..844197d7194e 100644
--- a/rust/kernel/alloc/allocator_test.rs
+++ b/rust/kernel/alloc/allocator_test.rs
@@ -7,7 +7,7 @@
 //! `Cmalloc` allocator within the `allocator_test` module and type alias all kernel allocators to
 //! `Cmalloc`. The `Cmalloc` allocator uses libc's `realloc()` function as allocator backend.

-#![allow(missing_docs)]
+#![expect(missing_docs)]

 use super::{flags::*, AllocError, Allocator, Flags};
 use core::alloc::Layout;
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 57502534d985..0e9510cf4625 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -157,7 +157,7 @@ fn remove_action(this: &Arc<Self>) -> bool {
         success
     }

-    #[allow(clippy::missing_safety_doc)]
+    #[expect(clippy::missing_safety_doc)]
     unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
         let ptr = ptr as *mut DevresInner<T>;
         // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index ec9166cedfa7..fc7bd65b01f1 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -168,7 +168,7 @@ fn of_id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
     }

     #[cfg(not(CONFIG_OF))]
-    #[allow(missing_docs)]
+    #[expect(missing_docs)]
     fn of_id_info(_dev: &device::Device) -> Option<&'static Self::IdInfo> {
         None
     }
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 445639404fb7..3ae8d2d8263f 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -9,28 +9,28 @@
 const BASE: u32 = uapi::DRM_IOCTL_BASE as u32;

 /// Construct a DRM ioctl number with no argument.
-#[allow(non_snake_case)]
+#[expect(non_snake_case)]
 #[inline(always)]
 pub const fn IO(nr: u32) -> u32 {
     ioctl::_IO(BASE, nr)
 }

 /// Construct a DRM ioctl number with a read-only argument.
-#[allow(non_snake_case)]
+#[expect(non_snake_case)]
 #[inline(always)]
 pub const fn IOR<T>(nr: u32) -> u32 {
     ioctl::_IOR::<T>(BASE, nr)
 }

 /// Construct a DRM ioctl number with a write-only argument.
-#[allow(non_snake_case)]
+#[expect(non_snake_case)]
 #[inline(always)]
 pub const fn IOW<T>(nr: u32) -> u32 {
     ioctl::_IOW::<T>(BASE, nr)
 }

 /// Construct a DRM ioctl number with a read-write argument.
-#[allow(non_snake_case)]
+#[expect(non_snake_case)]
 #[inline(always)]
 pub const fn IOWR<T>(nr: u32) -> u32 {
     ioctl::_IOWR::<T>(BASE, nr)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 3dee3139fcd4..a59613918d4c 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -263,7 +263,7 @@ fn from(e: core::convert::Infallible) -> Error {
 /// [`samples/rust/rust_minimal.rs`]):
 ///
 /// ```
-/// # #[allow(clippy::single_match)]
+/// # #[expect(clippy::single_match)]
 /// fn example() -> Result {
 ///     let mut numbers = KVec::new();
 ///
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 8d228c237954..288b1c2a290d 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -30,7 +30,7 @@
 //! ## General Examples
 //!
 //! ```rust,ignore
-//! # #![allow(clippy::disallowed_names)]
+//! # #![expect(clippy::disallowed_names)]
 //! use kernel::types::Opaque;
 //! use pin_init::pin_init_from_closure;
 //!
@@ -67,11 +67,11 @@
 //! ```
 //!
 //! ```rust,ignore
-//! # #![allow(unreachable_pub, clippy::disallowed_names)]
+//! # #![expect(unreachable_pub, clippy::disallowed_names)]
 //! use kernel::{prelude::*, types::Opaque};
 //! use core::{ptr::addr_of_mut, marker::PhantomPinned, pin::Pin};
 //! # mod bindings {
-//! #     #![allow(non_camel_case_types)]
+//! #     #![expect(non_camel_case_types)]
 //! #     pub struct foo;
 //! #     pub unsafe fn init_foo(_ptr: *mut foo) {}
 //! #     pub unsafe fn destroy_foo(_ptr: *mut foo) {}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 4b8cdcb21e77..91710a1d7b87 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -280,7 +280,7 @@ macro_rules! kunit_unsafe_test_suite {
             static mut KUNIT_TEST_SUITE: ::kernel::bindings::kunit_suite =
                 ::kernel::bindings::kunit_suite {
                     name: KUNIT_TEST_SUITE_NAME,
-                    #[allow(unused_unsafe)]
+                    #[expect(unused_unsafe)]
                     // SAFETY: `$test_cases` is passed in by the user, and
                     // (as documented) must be valid for the lifetime of
                     // the suite (i.e., static).
diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
index a566fc3e7dcb..aaeefe84861a 100644
--- a/rust/kernel/opp.rs
+++ b/rust/kernel/opp.rs
@@ -600,9 +600,9 @@ extern "C" fn config_regulators(
 pub struct Table {
     ptr: *mut bindings::opp_table,
     dev: ARef<Device>,
-    #[allow(dead_code)]
+    #[expect(dead_code)]
     em: bool,
-    #[allow(dead_code)]
+    #[expect(dead_code)]
     of: bool,
     cpus: Option<CpumaskVar>,
 }
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 22985b6f6982..a5d5a4737a41 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -600,5 +600,5 @@ pub enum Either<L, R> {
 /// constructed.
 ///
 /// [`NotThreadSafe`]: type@NotThreadSafe
-#[allow(non_upper_case_globals)]
+#[expect(non_upper_case_globals)]
 pub const NotThreadSafe: NotThreadSafe = PhantomData;
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c1..6fd1462ff01a 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -98,7 +98,7 @@ pub(crate) fn file() -> String {
     }

     #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)]
-    #[allow(clippy::incompatible_msrv)]
+    #[expect(clippy::incompatible_msrv)]
     {
         proc_macro::Span::call_site().file()
     }
--
2.50.0

Re: [PATCH v3 1/3] replace `#[allow(...)]` with `#[expect(...)]`

[kernel test robot]

Hi Onur,

kernel test robot noticed the following build warnings:

[auto build test WARNING on driver-core/driver-core-next]
[also build test WARNING on driver-core/driver-core-linus rust/alloc-next shuah-kselftest/kunit shuah-kselftest/kunit-fixes rafael-pm/linux-next rafael-pm/bleeding-edge linus/master v6.16-rc3]
[cannot apply to rust/rust-next driver-core/driver-core-testing rust/pin-init-next amd-pstate/linux-next amd-pstate/bleeding-edge next-20250627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Onur-zkan/replace-allow-with-expect/20250628-121734
base:   driver-core/driver-core-next
patch link:    https://lore.kernel.org/r/20250628040956.2181-2-work%40onurozkan.dev
patch subject: [PATCH v3 1/3] replace `#[allow(...)]` with `#[expect(...)]`
config: x86_64-randconfig-008-20250629 (https://download.01.org/0day-ci/archive/20250629/202506291507.M9eg5kic-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250629/202506291507.M9eg5kic-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506291507.M9eg5kic-lkp@intel.com/

All warnings (new ones prefixed by >>):

   ***
   *** Rust bindings generator 'bindgen' < 0.69.5 together with libclang >= 19.1
   *** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824),
   *** unless patched (like Debian's).
   ***   Your bindgen version:  0.65.1
   ***   Your libclang version: 20.1.7
   ***
   ***
   *** Please see Documentation/rust/quick-start.rst for details
   *** on how to set up the Rust support.
   ***
>> warning: this lint expectation is unfulfilled
   --> rust/kernel/opp.rs:603:14
   |
   603 |     #[expect(dead_code)]
   |              ^^^^^^^^^
   |
   = note: `#[warn(unfulfilled_lint_expectations)]` on by default
--
>> warning: this lint expectation is unfulfilled
   --> rust/kernel/opp.rs:605:14
   |
   605 |     #[expect(dead_code)]
   |              ^^^^^^^^^

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3 2/3] rust: remove `#[allow(clippy::unnecessary_cast)]`

[Onur Özkan]

This isn't needed anymore since `kernel::ffi::c_int` type
is always `i32` which differs from `err` (isize).

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/error.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index a59613918d4c..05c6e71c0afb 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -413,7 +413,6 @@ pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
         // SAFETY: The FFI function does not deref the pointer.
         let err = unsafe { bindings::PTR_ERR(const_ptr) };

-        #[allow(clippy::unnecessary_cast)]
         // CAST: If `IS_ERR()` returns `true`,
         // then `PTR_ERR()` is guaranteed to return a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`,
--
2.50.0

[PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur Özkan]

Clippy no longer complains about this lint.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/cpufreq.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 11b03e9d7e89..97de9b0573da 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -907,7 +907,6 @@ fn register_em(_policy: &mut Policy) {
 /// or CPUs, so it is safe to share it.
 unsafe impl<T: Driver> Sync for Registration<T> {}

-#[allow(clippy::non_send_fields_in_send_ty)]
 /// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any
 /// thread.
 unsafe impl<T: Driver> Send for Registration<T> {}
--
2.50.0

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 6:10 AM Onur Özkan <work@onurozkan.dev> wrote:
>
> Clippy no longer complains about this lint.

Do you have more context? For instance, do you know since when it no
longer complains, or why was the reason for the change? i.e. why we
had the `allow` in the first place, so that we know we don't need it
anymore?

For instance, please how I reasoned about it in commit 5e7c9b84ad08
("rust: sync: remove unneeded
`#[allow(clippy::non_send_fields_in_send_ty)]`").

(It may happen to be the same reason, or not.)

Thanks!

Cheers,
Miguel

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur]

On Sat, 28 Jun 2025 09:13:50 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sat, Jun 28, 2025 at 6:10 AM Onur Özkan <work@onurozkan.dev> wrote:
> >
> > Clippy no longer complains about this lint.
> 
> Do you have more context? For instance, do you know since when it no
> longer complains, or why was the reason for the change? i.e. why we
> had the `allow` in the first place, so that we know we don't need it
> anymore?
> 
> For instance, please how I reasoned about it in commit 5e7c9b84ad08
> ("rust: sync: remove unneeded
> `#[allow(clippy::non_send_fields_in_send_ty)]`").
> 
> (It may happen to be the same reason, or not.)
> 
> Thanks!
> 
> Cheers,
> Miguel

It doesn't seem to be the same reason. I rebased over
c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where that
lint was added) but still Clippy did not complain about it on the
MSRV. So it was either a leftover, or there is a version between
1.78 and the current stable where Clippy did complain. I can dig into it
more during the week if you would like.

IMO, we should require people to add a comment explaining the reason
for adding these lint rules to the codebase. It would make both reading
and modifying the code much simpler and clearer.

Regards,
Onur

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 12:30 PM Onur <work@onurozkan.dev> wrote:
>
> It doesn't seem to be the same reason. I rebased over
> c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where that
> lint was added) but still Clippy did not complain about it on the
> MSRV. So it was either a leftover, or there is a version between
> 1.78 and the current stable where Clippy did complain. I can dig into it
> more during the week if you would like.

Are you sure? The lint is actually disabled, as I mention in 5e7c9b84ad08.

From a quick test, I enabled it in that file, and I get the warning.

Thus it seems to me Clippy would still complain about it just fine.

It doesn't mean we shouldn't remove it, though.

> IMO, we should require people to add a comment explaining the reason
> for adding these lint rules to the codebase. It would make both reading
> and modifying the code much simpler and clearer.

Do you mean using the lint reasons feature? IIRC we discussed at some
point doing that when the feature was added (we enabled it for the
`expect` side of things).

For non-obvious cases or uncommon lints, it would be definitely nice
(a comment is also OK). I am not sure if it is worth enforcing it for
every single case, though.

It would be nice if `clippy::allow_attributes_without_reason` could be
enabled just for `allow`, or ignore it for certain lints.

Cheers,
Miguel

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur]

On Sat, 28 Jun 2025 14:18:53 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sat, Jun 28, 2025 at 12:30 PM Onur <work@onurozkan.dev> wrote:
> >
> > It doesn't seem to be the same reason. I rebased over
> > c6af9a1191d042839e56abff69e8b0302d117988 (the exact commit where
> > that lint was added) but still Clippy did not complain about it on
> > the MSRV. So it was either a leftover, or there is a version between
> > 1.78 and the current stable where Clippy did complain. I can dig
> > into it more during the week if you would like.
> 
> Are you sure? The lint is actually disabled, as I mention in
> 5e7c9b84ad08.
> 
> From a quick test, I enabled it in that file, and I get the warning.
> 
> Thus it seems to me Clippy would still complain about it just fine.

Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am
testing c6af9a1191d042839e56abff69e8b0302d117988 where
`#[allow(clippy::non_send_fields_in_send_ty)]` was added on
`unsafe impl<T: Driver> Send for Registration<T> {}`.

Switching from `allow` to `expect` produced the following result on my
end:

```
$ make LLVM=1 -j $(nproc)
CLIPPY=1 DESCEND objtool
  CALL    scripts/checksyscalls.sh
  INSTALL libsubcmd_headers
  CLIPPY L rust/kernel.o
error: this lint expectation is unfulfilled
   --> rust/kernel/cpufreq.rs:908:10
    |
908 | #[expect(clippy::non_send_fields_in_send_ty)]
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D unfulfilled-lint-expectations` implied by `-D warnings`
    = help: to override `-D warnings` add
`#[allow(unfulfilled_lint_expectations)]`

error: aborting due to 1 previous error

make[2]: *** [rust/Makefile:534: rust/kernel.o] Error 1
make[1]: *** [/home/nimda/devspace/onur-ozkan/linux/Makefile:1283:
prepare] Error 2 make: *** [Makefile:248: __sub-make] Error 2

$ git log -1
commit c6af9a1191d042839e56abff69e8b0302d117988 (HEAD -> master)
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Jan 24 12:36:33 2024 +0530

    rust: cpufreq: Extend abstractions for driver registration

    Extend the cpufreq abstractions to support driver registration from
    Rust.

    Reviewed-by: Danilo Krummrich <dakr@kernel.org>
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

$ rustc --version --verbose
rustc 1.78.0 (9b00956e5 2024-04-29)
binary: rustc
commit-hash: 9b00956e56009bab2aa15d7bff10916599e3d6d6
commit-date: 2024-04-29
host: x86_64-unknown-linux-gnu
release: 1.78.0
LLVM version: 18.1.2
```

Regards,
Onur

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 2:42 PM Onur <work@onurozkan.dev> wrote:
>
> Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am
> testing c6af9a1191d042839e56abff69e8b0302d117988 where
> `#[allow(clippy::non_send_fields_in_send_ty)]` was added on
> `unsafe impl<T: Driver> Send for Registration<T> {}`.
>
> Switching from `allow` to `expect` produced the following result on my
> end:

Yes, of course it does -- what I am telling you (and what 5e7c9b84ad08
says) is that the lint is disabled.

And since it is disabled, if you change the line to `expect`, then it
will obviously complain.

If you actually enabled the lint with e.g.

    #![warn(clippy::non_send_fields_in_send_ty)]

at the top of the file, and then used `expect`, it will build fine.

Cheers,
Miguel

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur]

On Sat, 28 Jun 2025 14:51:15 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sat, Jun 28, 2025 at 2:42 PM Onur <work@onurozkan.dev> wrote:
> >
> > Yes, I am sure. Just to clarify, I am not testing 5e7c9b84ad08. I am
> > testing c6af9a1191d042839e56abff69e8b0302d117988 where
> > `#[allow(clippy::non_send_fields_in_send_ty)]` was added on
> > `unsafe impl<T: Driver> Send for Registration<T> {}`.
> >
> > Switching from `allow` to `expect` produced the following result on
> > my end:
> 
> Yes, of course it does -- what I am telling you (and what 5e7c9b84ad08
> says) is that the lint is disabled.
> 
> And since it is disabled, if you change the line to `expect`, then it
> will obviously complain.
> 
> If you actually enabled the lint with e.g.
> 
>     #![warn(clippy::non_send_fields_in_send_ty)]
> 
> at the top of the file, and then used `expect`, it will build fine.


Aha, I see. I missed that. I guess `allow` was added when the author
had this lint enabled on their checkout, but their work was merged when
lint removal was merged before that.

Do you want me to update the patch description by including
5e7c9b84ad08 ref and send v4?

Sorry for misunderstanding by the way!

Many thanks,
Onur

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 3:11 PM Onur <work@onurozkan.dev> wrote:
>
> Aha, I see. I missed that. I guess `allow` was added when the author
> had this lint enabled on their checkout, but their work was merged when
> lint removal was merged before that.

Yeah, some of the code going around was written years ago, so
sometimes this sort of thing happens. :)

> Do you want me to update the patch description by including
> 5e7c9b84ad08 ref and send v4?

Sure -- maybe wait a few days, to see if anyone says anything else.
Then we will need to wait for Acked-bys from other maintainers.

Or, actually, if you are sending a new version and you are willing to
do it, then it would be easier to land if you split the first patch
also by subsystem -- that way each maintainer can take their patches
on their own time instead. Since each patch is independent, you can
send them in independent patch series, that makes it even easier for
maintainers to track.

For instance, you could do this grouping:

     rust/kernel/error.rs                | 2 +-
     rust/kernel/types.rs                | 2 +-
     rust/macros/helpers.rs              | 2 +-
     (+ this patch #2)

     rust/kernel/init.rs                 | 6 +++---

     rust/kernel/kunit.rs                | 2 +-

     drivers/gpu/nova-core/regs.rs       | 2 +-
     rust/kernel/drm/ioctl.rs            | 8 ++++----

     rust/kernel/devres.rs               | 2 +-
     rust/kernel/driver.rs               | 2 +-

     rust/kernel/alloc/allocator_test.rs | 2 +-

     rust/kernel/opp.rs                  | 4 ++--
     (+ this patch #3)

So e.g. the top one (Rust) would be a series of 2 patches, then the
next one (pin-init) a single independent patch, and so on.

> Sorry for misunderstanding by the way!

No worries at all! It happens :)

And thanks for doing these cleanups! They are appreciated.

Cheers,
Miguel

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur]

On Sat, 28 Jun 2025 15:28:29 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sat, Jun 28, 2025 at 3:11 PM Onur <work@onurozkan.dev> wrote:
> >
> > Aha, I see. I missed that. I guess `allow` was added when the author
> > had this lint enabled on their checkout, but their work was merged
> > when lint removal was merged before that.
> 
> Yeah, some of the code going around was written years ago, so
> sometimes this sort of thing happens. :)
> 
> > Do you want me to update the patch description by including
> > 5e7c9b84ad08 ref and send v4?
> 
> Sure -- maybe wait a few days, to see if anyone says anything else.
> Then we will need to wait for Acked-bys from other maintainers.
> 
> Or, actually, if you are sending a new version and you are willing to
> do it, then it would be easier to land if you split the first patch
> also by subsystem -- that way each maintainer can take their patches
> on their own time instead. Since each patch is independent, you can
> send them in independent patch series, that makes it even easier for
> maintainers to track.

I don't have enough time to do it right now, but I would be happy
to do it in ~3 days during the week (assuming it's still not being
reviewed by the maintainers by then).

Regards,
Onur

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 3:42 PM Onur <work@onurozkan.dev> wrote:
>
> I don't have enough time to do it right now, but I would be happy
> to do it in ~3 days during the week (assuming it's still not being
> reviewed by the maintainers by then).

No worries, there is no rush on these cleanups. Even if it gets
reviewed, you can still split, keeping their tag. Thanks!

Cheers,
Miguel

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Onur]

On Sat, 28 Jun 2025 14:18:53 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:


> > IMO, we should require people to add a comment explaining the reason
> > for adding these lint rules to the codebase. It would make both
> > reading and modifying the code much simpler and clearer.
> 
> Do you mean using the lint reasons feature? IIRC we discussed at some
> point doing that when the feature was added (we enabled it for the
> `expect` side of things).

Yeah, I meant that it't taking more effort than it should, like digging
through historical changes in the relevant parts of the source code,
trying to figuring out whether it was just a false positive or if there
was a specific reason behind it, etc.

Re: [PATCH v3 3/3] rust: remove `#[allow(clippy::non_send_fields_in_send_ty)]`

[Miguel Ojeda]

On Sat, Jun 28, 2025 at 3:04 PM Onur <work@onurozkan.dev> wrote:
>
> Yeah, I meant that it't taking more effort than it should, like digging
> through historical changes in the relevant parts of the source code,
> trying to figuring out whether it was just a false positive or if there
> was a specific reason behind it, etc.

Yeah, that is a big part of kernel development, especially on the
maintenance side :)

I definitely agree that a good comment in the source code is better
than going through Git history, and the kernel sometimes has had some
things documented in the Git log that should have been in the source
code instead. It happens.

However, in some cases like this one it is not clear it would help.
For instance, here the lint reason message could have been something
that made sense back then when the lint was enabled, and yet we would
still have had to notice the lint got disabled later on, so we would
end up still going into the Git log.

`expect` is great to mitigate some of that -- sadly we cannot use it
as much as we would like due to sometimes being conditional to an arch
or the kernel config or the Rust version. (And your first patch may
have some cases that perhaps we cannot convert due to that -- I didn't
check)

Cheers,
Miguel