On Sun, 31 Mar 2024 at 07:27, Benno Lossin benno.lossin@proton.me wrote:
On 31.03.24 03:00, Wedson Almeida Filho wrote:
On Wed, 27 Mar 2024 at 13:04, Benno Lossin benno.lossin@proton.me wrote:
#[cfg(not(MODULE))]#[doc(hidden)]#[no_mangle]pub extern \"C\" fn __{name}_exit() {{__exit()I just noticed this should be wrapped in an `unsafe` block with a SAFETY comment. Will fix this in v2.
}}
#[cfg(not(MODULE))]#[doc(hidden)]#[no_mangle]pub extern \"C\" fn __{name}_exit() {{__exit()}}
/// # Safety////// This function must/// - only be called once,/// - not be called concurrently with `__exit`.I don't think the second item is needed here, it really is a requirement on `__exit`.
Fixed.
unsafe fn __init() -> core::ffi::c_int {{match <{type_} as kernel::Module>::init(&THIS_MODULE) {{Ok(m) => {{// SAFETY:// no data race, since `__MOD` can only be accessed by this module and// there only `__init` and `__exit` access it. These functions are only// called once and `__exit` cannot be called before or during `__init`.unsafe {{__MOD = Some(m);}}return 0;}}Err(e) => {{return e.to_errno();}}}}}}
fn __init() -> core::ffi::c_int {{match <{type_} as kernel::Module>::init(&THIS_MODULE) {{Ok(m) => {{
/// # Safety////// This function must/// - only be called once,/// - be called after `__init`,/// - not be called concurrently with `__init`.The second item is incomplete: it must be called after `__init` *succeeds*.
Indeed.
With that added (which is a different precondition), I think the third item can be dropped because if you have to wait to see whether `__init` succeeded or failed before you can call `__exit`, then certainly you cannot call it concurrently with `__init`.
I would love to drop that requirement, but I am not sure we can. With that requirement, I wanted to ensure that no data race on `__MOD` can happen. If you need to verify that `__init` succeeded, one might think that it is not possible to call `__exit` such that a data race occurs, but I think it could theoretically be done if the concrete `Module` implementation never failed.
I see. If you're concerned about compiler reordering, then we need compiler barriers.
Do you have any suggestion for what I could add to the "be called after `__init` was called and returned `0`" requirement to make a data race impossible?
If you're concerned with reordering from the processor as well, then we need cpu barriers. You'd have to say that the cpu/thread executing `__init` must have a release barrier after `__init` completes, and the thread/cpu doing `__exit` must have an acquire barrier before starting `__exit`.
But I'm not sure we need to go that far. Mostly because C is going to guarantee that ordering for us, so I'd say we can just omit this or perhaps say "This function must only be called from the exit module implementation".
-- Cheers, Benno
unsafe fn __exit() {{// SAFETY:// no data race, since `__MOD` can only be accessed by this module and there// only `__init` and `__exit` access it. These functions are only called once// and `__init` was already called. unsafe {{
__MOD = Some(m);
// Invokes `drop()` on `__MOD`, which should be used for cleanup.__MOD = None; }}
return 0; }}Err(e) => {{return e.to_errno();}}}}}}fn __exit() {{unsafe {{// Invokes `drop()` on `__MOD`, which should be used for cleanup.__MOD = None;
{modinfo} }} }}
{modinfo} ", type_ = info.type_, name = info.name,base-commit: 4cece764965020c22cff7665b18a012006359095
2.44.0