On 30.10.23 14:58, Alice Ryhl wrote: > Boqun Feng <boqun.feng@xxxxxxxxx> writes: >>> On Wed, Oct 25, 2023 at 09:51:28PM +0000, Benno Lossin wrote: >>>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs >>>> index d849e1979ac7..b0872f751f97 100644 >>>> --- a/rust/kernel/types.rs >>>> +++ b/rust/kernel/types.rs >>> >>> I don't think this should go into `types.rs`. But I do not have a good >>> name for the new module. >> >> kernel::sync? > > I think `kernel::sync` is a reasonable choice, but here's another > possibility: Put them in the `bindings` crate. > > Why? Well, they are a utility that intends to replicate the C > `READ_ONCE` and `WRITE_ONCE` macros. All of our other methods that do > the same thing for C functions are functions in the bindings crate. I think we should keep things separate, that way `bindings` can be fully automatically generated. Stuff in the bindings crate is just an interface to the C world. Functions are not implemented there, but rather linked to. > Similarly, if we ever decide to reimplement a C function in Rust for > performance/inlining reasons, then I also think that it is reasonable to > put that Rust-reimplementation in the bindings crate. This approach also > makes it easy to transparently handle cases where we only reimplement a > function in Rust under some configurations. Is it really going to make things easier? We would have to make bindgen conditionally not create bindings for an item, so I think it would be easier to just always have the bindings function and handle the conditional reimplementation fully in the Rust code. Having extra code in the bindings crate will also make it more difficult to ensure that only abstractions use the bindings (we already have some exceptions, but why make it worse?). -- Cheers, Benno
