linux-kernel - Re: [RFC] rust: types: Add read_once and write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Yx813Gi8SUJm_qipa7I_y1k7MYo3Sn8Hoyn_x4rSOcngjqEvS-os6Sbjkoavf0nUIymeYKrzdyLZdEd2aPKnXdDJ46cdPXyAyl33tAN_UYo=@proton.me>
Date:   Mon, 30 Oct 2023 16:36:35 +0000
From:   Benno Lossin <benno.lossin@...ton.me>
To:     Alice Ryhl <aliceryhl@...gle.com>
Cc:     boqun.feng@...il.com, a.hindborg@...sung.com, akiyks@...il.com,
        alex.gaynor@...il.com, bjorn3_gh@...tonmail.com,
        brauner@...nel.org, david@...morbit.com, dhowells@...hat.com,
        dlustig@...dia.com, elver@...gle.com, gary@...yguo.net,
        gregkh@...uxfoundation.org, j.alglave@....ac.uk,
        joel@...lfernandes.org, kent.overstreet@...il.com,
        linux-arch@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
        luc.maranget@...ia.fr, nathan@...nel.org, ndesaulniers@...gle.com,
        npiggin@...il.com, ojeda@...nel.org, parri.andrea@...il.com,
        paulmck@...nel.org, peterz@...radead.org,
        rust-for-linux@...r.kernel.org, stern@...land.harvard.edu,
        trix@...hat.com, viro@...iv.linux.org.uk, wedsonaf@...il.com,
        will@...nel.org, willy@...radead.org
Subject: Re: [RFC] rust: types: Add read_once and write_once

On 30.10.23 14:58, Alice Ryhl wrote:
> Boqun Feng <boqun.feng@...il.com> 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