Re: [PATCH v3 4/4] rust: add parameter support to the `module!` macro

Alice Ryhl
2025-01-08T13:17:51Z

On Wed, Jan 8, 2025 at 1:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:
>
> > On Fri, Dec 13, 2024 at 2:17 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >> scheduled for removal. Interior mutability via `SyncUnsafeCell` provides
> >> the same functionality and it is my understanding that this feature is
> >> on track to be stabilized.
> >
> > I am not sure about the last bit, but even if it is on track, we do
> > not want to start using new language features or APIs that could
> > potentially change.
> >
> > And even if it is a feature that we are sure will not change, we
> > should still let upstream Rust know before using it, since we are
> > actively discussing with them the remaining unstable items that the
> > kernel needs and they are putting the kernel in their roadmap.
> >
> > So I suggest we mention it next week in the Rust/Rust for Linux meeting.
>
> I don't think we ever discussed this?
>
> I was going to put this in the commit trailer for v4:
>
> ---
> Version 3 of this patch enabled the unstable feature `sync_unsafe_cell` [1] to
> avoid `static mut` variables as suggested by Trevor Tross and Benno Lossin [2].
>
> As we are moving closer to a new edition, it is now clear that `static mut` is
> not being deprecated in the 2024 edition as first anticipated [3].
>
> Still, `SyncUnsafeCell` allows us to use safe code when referring to the
> parameter value:
>
> ```
> {param_name}.as_mut_ptr().cast()
> ```
>
> rather than unsafe code:
>
> ```
> unsafe { addr_of_mut!(__{name}_{param_name}_value) }.cast()
> ```
>
> Thus, this version (4) keeps the feature enabled.
>
> [1] https://github.com/rust-lang/rust/issues/95439
> [2] https://lore.kernel.org/all/CALNs47sqt==o+hM5M1b0vTayKH177naybg_KurcirXszYAa22A@mail.gmail.com/
> [3] https://github.com/rust-lang/rust/issues/53639#issuecomment-2434023115
> ---
>
> What do you think?

It's pretty easy to avoid needing this feature, so I don't think we
should enable it.

Alice