Messages in this thread | | | From | Wedson Almeida Filho <> | Date | Thu, 28 Mar 2024 09:57:52 -0300 | Subject | Re: [PATCH 1/2] rust: introduce `InPlaceModule` |
| |
On Wed, 27 Mar 2024 at 12:56, Benno Lossin <benno.lossin@proton.me> wrote: > > On 27.03.24 15:23, Wedson Almeida Filho wrote: > > On Wed, 27 Mar 2024 at 05:13, Valentin Obst <kernel@valentinobst.de> wrote: > >> > >>> This allows modules to be initialised in-place in pinned memory, which > >>> enables the usage of pinned types (e.g., mutexes, spinlocks, driver > >>> registrations, etc.) in modules without any extra allocations. > >>> > >>> Drivers that don't need this may continue to implement `Module` without > >>> any changes. > >>> > >>> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com> > >>> --- > >>> rust/kernel/lib.rs | 25 ++++++++++++++++++++++++- > >>> rust/macros/module.rs | 18 ++++++------------ > >>> 2 files changed, 30 insertions(+), 13 deletions(-) > >>> > >>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > >>> index 5c641233e26d..64aee4fbc53b 100644 > >>> --- a/rust/kernel/lib.rs > >>> +++ b/rust/kernel/lib.rs > >>> @@ -62,7 +62,7 @@ > >>> /// The top level entrypoint to implementing a kernel module. > >>> /// > >>> /// For any teardown or cleanup operations, your type may implement [`Drop`]. > >>> -pub trait Module: Sized + Sync { > >>> +pub trait Module: Sized + Sync + Send { > >> > >> This does not compile with `CONFIG_AX88796B_RUST_PHY=y || m` (or the > >> phylib abstractions' doctests) since the module `Registration` is not > >> `Send`. > > > > Thanks for the heads up. I thought I had enabled all rust code but > > indeed I was missing this. I will fix it in v2. > > > >> I remember Trevor raising the question whether we want to require modules > >> to be `Send`. I am not aware of any examples of `!Send` modules but I guess > >> it would be possible to write code that is only correct under the > >> assumption that it is loaded/unloaded in the same context. > > > > It might be possible in the future, but I don't believe it is now > > because all rust modules support unloading. And there is no guarantee > > that the thread unloading (and therefore calling module_exit) is the > > same that loaded (and called module_init), so a module must be Send to > > properly handle drop being called from a different thread. > > > > Not requiring Send on the original Module trait was an oversight that > > I don't want to repeat in InPlaceModule. > > I think that this change should go to the stable tree, can you split it > into its own patch?
Sure, I split it off in v2.
Note that you'll also need the [new] patch to `rust/kernel/net/phy.rs`.
> -- > Cheers, > Benno > > > > >> > >> @Trevor: Are you aware of any modules with that requirement? > >> > >> I have been using this patch for quite a while with my TCP CCAs now > >> (without the `Send` bound) and did not experience any other issues; thus > >> offering: > >> Tested-by: Valentin Obst <kernel@valentinobst.de> > > > > Thanks! > > > >> > >> - Best Valentin > >> >
| |