[PATCH v2 0/5] In-place module initialisation

Wedson Almeida Filho posted 5 patches 1 year, 10 months ago
rust/kernel/lib.rs           | 25 +++++++++++++++++++-
rust/kernel/net/phy.rs       |  4 ++++
rust/macros/module.rs        | 36 ++++++++++++-----------------
samples/rust/Kconfig         | 11 +++++++++
samples/rust/Makefile        |  1 +
samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
6 files changed, 99 insertions(+), 22 deletions(-)
create mode 100644 samples/rust/rust_inplace.rs
[PATCH v2 0/5] In-place module initialisation
Posted by Wedson Almeida Filho 1 year, 10 months ago
From: Wedson Almeida Filho <walmeida@microsoft.com>

Introduce `InPlaceModule`, which allows modules to be initialised
inplace, as opposed to the current state where modules must return an
instance which is moved to its final memory location.

This allows us to have modules whose state hold objects that must be
initialised inplace like locks. It also allows us to implement
registrations (e.g., driver registration) inplace and have them similar
to their C counterparts where no new allocations are needed.

This is built on top of the allocation APIs because the sample module is
a modified version of rust_minimal, which would be incompatible with the
allocation API series because it uses vectors.

---

Changes in v2:

- Rebased to latest rust-next 
- Split off adding `Send` requirement to `Module` into a separate patch
- Prefixed all `core` and `kernel` references with `::` in
  `module!` macro-generated code.
- Calling `__pinned_init` with explicit full path.
- Add `Mutex` to `rust_inplace`  sample.
- Added `Send` implementation for `Registration` in net/phy.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#t

---
Wedson Almeida Filho (5):
  rust: phy: implement `Send` for `Registration`
  rust: kernel: require `Send` for `Module` implementations
  rust: module: prefix all module paths with `::`
  rust: introduce `InPlaceModule`
  samples: rust: add in-place initialisation sample

 rust/kernel/lib.rs           | 25 +++++++++++++++++++-
 rust/kernel/net/phy.rs       |  4 ++++
 rust/macros/module.rs        | 36 ++++++++++++-----------------
 samples/rust/Kconfig         | 11 +++++++++
 samples/rust/Makefile        |  1 +
 samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
 6 files changed, 99 insertions(+), 22 deletions(-)
 create mode 100644 samples/rust/rust_inplace.rs


base-commit: 453275c66aa4d87c73c5152140b3573ab2435bb7
-- 
2.34.1
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Valentin Obst 1 year, 10 months ago
> Introduce `InPlaceModule`, which allows modules to be initialised
> inplace, as opposed to the current state where modules must return an
> instance which is moved to its final memory location.
>
> This allows us to have modules whose state hold objects that must be
> initialised inplace like locks. It also allows us to implement
> registrations (e.g., driver registration) inplace and have them similar
> to their C counterparts where no new allocations are needed.
>
> This is built on top of the allocation APIs because the sample module is
> a modified version of rust_minimal, which would be incompatible with the
> allocation API series because it uses vectors.
>
> ---
>
> Changes in v2:
>
> - Rebased to latest rust-next
> - Split off adding `Send` requirement to `Module` into a separate patch

I think the idea in [1] was to have this patch being included in the
stable trees. I got little experience with stable trees but wouldn't the
easiest way be that you add:

	Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
	Cc: stable@vger.kernel.org
	Fixes: 247b365dc8dc ("rust: add `kernel` crate")

in the sign-off section for this patch? (Or mark the first one for stable
inclusion as well, [2] has more information on that).

	- Best Valentin

[1]: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#m4b4daf27038f920a0c6fafff453efb3c8da72067
[2]: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

> - Prefixed all `core` and `kernel` references with `::` in
>   `module!` macro-generated code.
> - Calling `__pinned_init` with explicit full path.
> - Add `Mutex` to `rust_inplace`  sample.
> - Added `Send` implementation for `Registration` in net/phy.
> - Link to v1: https://lore.kernel.org/rust-for-linux/20240327032337.188938-1-wedsonaf@gmail.com/T/#t
>
> ---
> Wedson Almeida Filho (5):
>   rust: phy: implement `Send` for `Registration`
>   rust: kernel: require `Send` for `Module` implementations
>   rust: module: prefix all module paths with `::`
>   rust: introduce `InPlaceModule`
>   samples: rust: add in-place initialisation sample
>
>  rust/kernel/lib.rs           | 25 +++++++++++++++++++-
>  rust/kernel/net/phy.rs       |  4 ++++
>  rust/macros/module.rs        | 36 ++++++++++++-----------------
>  samples/rust/Kconfig         | 11 +++++++++
>  samples/rust/Makefile        |  1 +
>  samples/rust/rust_inplace.rs | 44 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 99 insertions(+), 22 deletions(-)
>  create mode 100644 samples/rust/rust_inplace.rs
>
>
> base-commit: 453275c66aa4d87c73c5152140b3573ab2435bb7
> --
> 2.34.1
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Miguel Ojeda 1 year, 10 months ago
On Fri, Mar 29, 2024 at 1:11 PM Valentin Obst <kernel@valentinobst.de> wrote:
>
> I think the idea in [1] was to have this patch being included in the
> stable trees. I got little experience with stable trees but wouldn't the
> easiest way be that you add:
>
>         Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
>         Cc: stable@vger.kernel.org
>         Fixes: 247b365dc8dc ("rust: add `kernel` crate")
>
> in the sign-off section for this patch? (Or mark the first one for stable
> inclusion as well, [2] has more information on that).

715dd8950d4e is your local hash for 1/5, right? So I would drop the
hash, because it may be confusing.

It may be possible to remove the first line (since 1/5 will only apply
to 6.8.x and it is already the previous patch in the series, while the
`Fixes` tag here may make it clear that 2/5 should still go everywhere
regardless of 1/5), but I guess it does not hurt to be extra clear.

What about:

    Cc: stable@vger.kernel.org # 6.8.x: rust: phy: implement `Send`
for `Registration`
    Cc: stable@vger.kernel.org # 6.1+
    Fixes: 247b365dc8dc ("rust: add `kernel` crate")

Cheers,
Miguel
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Valentin Obst 1 year, 10 months ago
> > I think the idea in [1] was to have this patch being included in the
> > stable trees. I got little experience with stable trees but wouldn't the
> > easiest way be that you add:
> >
> >         Cc: stable@vger.kernel.org # 6.8.x: 715dd8950d4e rust: phy: implement `Send` for `Registration`
> >         Cc: stable@vger.kernel.org
> >         Fixes: 247b365dc8dc ("rust: add `kernel` crate")
> >
> > in the sign-off section for this patch? (Or mark the first one for stable
> > inclusion as well, [2] has more information on that).
>
> 715dd8950d4e is your local hash for 1/5, right? So I would drop the
> hash, because it may be confusing.

Ah, right, of course this won't be the hash of the commit in mainline;

>
> It may be possible to remove the first line (since 1/5 will only apply
> to 6.8.x and it is already the previous patch in the series, while the

If I interpret the docs correctly, previous patches in the same series are
only implicitly considered as prerequisites for the marked patch if they
are marked themselves:

    "[...] you do not have to list patch1 as prerequisite of patch2
    if you have already marked patch1 for stable inclusion."

So I guess it is important to be explicit.

> `Fixes` tag here may make it clear that 2/5 should still go everywhere
> regardless of 1/5), but I guess it does not hurt to be extra clear.
>
> What about:
>
>     Cc: stable@vger.kernel.org # 6.8.x: rust: phy: implement `Send`
> for `Registration`
>     Cc: stable@vger.kernel.org # 6.1+
>     Fixes: 247b365dc8dc ("rust: add `kernel` crate")

Looks reasonable to me; Also think that the 6.1+ is not striclty necessary
due to the `Fixes` tag though.

    - Best Valentin

>
> Cheers,
> Miguel
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Miguel Ojeda 1 year, 10 months ago
On Fri, Mar 29, 2024 at 3:00 PM Valentin Obst <kernel@valentinobst.de> wrote:
>
> If I interpret the docs correctly, previous patches in the same series are
> only implicitly considered as prerequisites for the marked patch if they
> are marked themselves:
>
>     "[...] you do not have to list patch1 as prerequisite of patch2
>     if you have already marked patch1 for stable inclusion."

Right, "Cc: stable" would be needed in both 1/5 and 2/5 (but one could
remove the "# ..." comments in that case, i.e. it seems simpler).

Cheers,
Miguel
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Miguel Ojeda 1 year, 9 months ago
On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> From: Wedson Almeida Filho <walmeida@microsoft.com>
>
> Introduce `InPlaceModule`, which allows modules to be initialised
> inplace, as opposed to the current state where modules must return an
> instance which is moved to its final memory location.
>
> This allows us to have modules whose state hold objects that must be
> initialised inplace like locks. It also allows us to implement
> registrations (e.g., driver registration) inplace and have them similar
> to their C counterparts where no new allocations are needed.
>
> This is built on top of the allocation APIs because the sample module is
> a modified version of rust_minimal, which would be incompatible with the
> allocation API series because it uses vectors.

For the moment, applied 1/5 and 2/5 to `rust-fixes` (tagged for stable
too) -- thanks everyone!

Cheers,
Miguel
Re: [PATCH v2 0/5] In-place module initialisation
Posted by Danilo Krummrich 1 year, 7 months ago
Hi,

On 4/23/24 02:42, Miguel Ojeda wrote:
> On Thu, Mar 28, 2024 at 8:55 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>>
>> From: Wedson Almeida Filho <walmeida@microsoft.com>
>>
>> Introduce `InPlaceModule`, which allows modules to be initialised
>> inplace, as opposed to the current state where modules must return an
>> instance which is moved to its final memory location.
>>
>> This allows us to have modules whose state hold objects that must be
>> initialised inplace like locks. It also allows us to implement
>> registrations (e.g., driver registration) inplace and have them similar
>> to their C counterparts where no new allocations are needed.
>>
>> This is built on top of the allocation APIs because the sample module is
>> a modified version of rust_minimal, which would be incompatible with the
>> allocation API series because it uses vectors.
> 
> For the moment, applied 1/5 and 2/5 to `rust-fixes` (tagged for stable
> too) -- thanks everyone!

What's the status of this series? I'd need this for the device / driver patches.

- Danilo

> 
> Cheers,
> Miguel