Re: [PATCH v13 2/6] rust: introduce module_param module

[Benno Lossin]

On Mon Jun 30, 2025 at 3:15 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@xxxxxxxxxx> writes:
>> On Mon Jun 30, 2025 at 1:18 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@xxxxxxxxxx> writes:
>>>> (no idea if the orderings are correct, I always have to think way to
>>>> much about that... especially since our atomics seem to only take one
>>>> ordering in compare_exchange?)
>>>>
>>>>> As far as I can tell, atomics may not land in v6.17, so this series
>>>>> will probably not be ready for merge until v6.18 at the earliest.
>>>>
>>>> Yeah, sorry about that :(
>>>
>>> Actually, perhaps we could aim at merging this code without this
>>> synchronization?
>>
>> I won't remember this issue in a few weeks and I fear that it will just
>> get buried. In fact, I already had to re-read now what the actual issue
>> was...
>>
>>> The lack of synchronization is only a problem if we
>>> support custom parsing. This patch set does not allow custom parsing
>>> code, so it does not suffer this issue.
>>
>> ... In doing that, I saw my original example of UB:
>>
>>     module! {
>>         // ...
>>         params: {
>>             my_param: i64 {
>>                 default: 0,
>>                 description: "",
>>             },
>>         },
>>     }
>>
>>     static BAD: &'static i64 = module_parameters::my_param.get();
>>
>> That can happen without custom parsing, so it's still a problem...
>
> Ah, got it. Thanks.

On second thought, we *could* just make the accessor function `unsafe`.
Of course with a pinky promise to make the implementation safe once
atomics land. But I think if it helps you get your driver faster along,
then we should do it.

---
Cheers,
Benno