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

Andreas Hindborg
2025-01-08T14:20:45Z

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Wed, Jan 8, 2025 at 1:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> I don't think we ever discussed this?
>
> I don't think so -- we discussed other things related to 2025H1 the
> last meeting. Perhaps you/we can bring it up in the next one?
>
>> 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:
>
>> What do you think?
>
> The justification seems fairly weak... Unless we are fairly confident
> the API will be stable (even if not stabilized right now), I am not
> sure why we would want to do this right now.
>
> Can we provide our own `SyncUnsafeCell` instead in the meantime, if
> you want to keep the advantages?

I like having the shared `ModuleParamAccess` struct to encapsulate the access
to the parameters rather than emitting an enum for every parameter
instance. For that to work without unsafe code when the user accesses
the parameter value, we need a non-mutable static that provides interior
mutability.

I could implement `Sync` for `ModuleParamAccess` or I could add
`kernel::types::SyncUnsafeCell`. Either way is fine for me.

>
>> Returning an error and `pr_warn!` is doable. As far as I can tell, we do
>> not have `WARN_ON_ONCE` yet?
>
> Please see https://lore.kernel.org/rust-for-linux/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/
> in case it helps.

Cool, I can list that as a prerequisite.

Best regards,
Andreas Hindborg