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

[Danilo Krummrich <dakr@redhat.com>]

On Tue, Mar 26, 2024 at 09:48:07PM -0300, Wedson Almeida Filho wrote:
> 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.

Agree, it would have been better to directly contact you again.

Except for patches 2 & 3 the other ones are created from patches that
"extracted" code from the HEAD of R4L/rust. I just fixed authorship before
sending them, hence I think I didn't really notice that you are the original
author of all device / driver abstractions.

Please also note that I announced these efforts a couple of times, e.g. in [1]
and [2].

> 
> 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/o

For the patches 2 & 3 I'm aware of the previous discussions. However, I have to
admit it's been a while since I read them and hence I forgot to mention that
this series is, besides others, also a follow up of that one.

For everything else I'm not aware of previous discussions, if there are any,
I'm sorry I missed them. Also please let me know if there are any, such that I
can follow up.

> 
> 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 [2] I was asking about the preferred way to get some immediate discussions
going. When I proposed to send links to the corresponding branches to the
mailing list or send a patch series (for rust-device I ended up doing both) I
did not hear any contradiction.

To me the mailing list seems to be a good place to revisit, review and improve
these patches.

I'd propose to just continue with this series and collaborate on improving it.
Are you fine with that?

- Danilo

[1] https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u
[2] https://rust-for-linux.zulipchat.com/#narrow/stream/415254-DRM/topic/Topic.20branches.20for.20staging.20Rust.20abstractions/near/426580539

> 
> 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
> >
>