Re: [PATCH v7 0/8] [RFC] Rust device / driver abstractions

[Wedson Almeida Filho <wedsonaf@gmail.com>]

On Mon, 25 Mar 2024 at 14:47, Danilo Krummrich <dakr@redhat.com> wrote:
>
> Hi,
>
> This patch series provides some initial Rust abstractions around the device /
> driver model, including an abstraction for device private data.
>
> This patch series is sent in the context of [1] and is also available at [2].
>
> - Danilo
>
> [1] https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u
> [2] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
>
> Danilo Krummrich (1):
>   arch: x86: tools: increase symbol name size
>
> Wedson Almeida Filho (7):
>   rust: device: Add a minimal RawDevice trait
>   rust: device: Add a stub abstraction for devices
>   rust: add driver abstraction
>   rust: add rcu abstraction
>   rust: add revocable mutex
>   rust: add revocable objects
>   rust: add device::Data

Danilo,

It seems like I'm the original author of the vast majority of the code
in this RFC series, yet I wasn't contacted by you for coordination
before you sent this.

A bunch (all?) of these patches were already submitted upstream with a
lot of discussion and decisions made to modify things. Why are you
resubmitting them basically ignoring all previous discussions? Take
patches 2 & 3 as examples (I don't bother to look for others now):

https://lore.kernel.org/lkml/20230224-rust-iopt-rtkit-v1-2-49ced3391295@asahilina.net/
https://lore.kernel.org/lkml/20230224-rust-iopt-rtkit-v1-5-49ced3391295@asahilina.net/

Also, these patches were written in the rust branch. Before we
upstream them, we have to revisit them to check if changes are needed
given the changes/improvements we have made; for example, pin init now
allows us to initialise pinned objects safely -- we need to follow the
new way now and I see that you don't in `RevocableMutex`. PinInit also
 enables us to have pinned modules, which simplifies how we do
registrations (so they also need to be updated), locks have been
redone with a common `Lock` type, etc.

In summary, we can't just copy code, we need to revisit some of it and
at least check suitability before submitting them.

>  arch/x86/tools/insn_decoder_test.c |   2 +-
>  rust/bindings/bindings_helper.h    |   1 +
>  rust/helpers.c                     |  28 ++
>  rust/kernel/device.rs              | 215 +++++++++++++
>  rust/kernel/driver.rs              | 493 +++++++++++++++++++++++++++++
>  rust/kernel/lib.rs                 |   6 +-
>  rust/kernel/revocable.rs           | 438 +++++++++++++++++++++++++
>  rust/kernel/sync.rs                |   3 +
>  rust/kernel/sync/rcu.rs            |  52 +++
>  rust/kernel/sync/revocable.rs      |  98 ++++++
>  rust/macros/module.rs              |   2 +-
>  samples/rust/rust_minimal.rs       |   2 +-
>  samples/rust/rust_print.rs         |   2 +-
>  13 files changed, 1337 insertions(+), 5 deletions(-)
>  create mode 100644 rust/kernel/device.rs
>  create mode 100644 rust/kernel/driver.rs
>  create mode 100644 rust/kernel/revocable.rs
>  create mode 100644 rust/kernel/sync/rcu.rs
>  create mode 100644 rust/kernel/sync/revocable.rs
>
>
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> --
> 2.44.0
>