Received-SPF: pass (google.com: domain of linux-kernel+bounces-60345-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240205-b4-rbtree-v1-0-995e3eee38c0@google.com> <20240205-b4-rbtree-v1-1-995e3eee38c0@google.com>
In-Reply-To: <20240205-b4-rbtree-v1-1-995e3eee38c0@google.com>
From: Trevor Gross <tmgross@umich.edu>
Date: Sat, 10 Feb 2024 02:05:29 -0600
Message-ID: <CALNs47vgryKctpsP9EV_SsVF8TX8kfwzQ4KCLy9vr5FS_vBq6A@mail.gmail.com>
Subject: Re: [PATCH 1/6] rust: add `container_of!` macro
To: mattgilbride@google.com
Cc: Miguel Ojeda <ojeda@kernel.org>, Alex Gaynor <alex.gaynor@gmail.com>, 
	Wedson Almeida Filho <wedsonaf@gmail.com>, Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>, 
	=?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>, 
	Benno Lossin <benno.lossin@proton.me>, Andreas Hindborg <a.hindborg@samsung.com>, 
	Alice Ryhl <aliceryhl@google.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, 
	=?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= <arve@android.com>, 
	Todd Kjos <tkjos@android.com>, Martijn Coenen <maco@android.com>, 
	Joel Fernandes <joel@joelfernandes.org>, Carlos Llamas <cmllamas@google.com>, 
	Suren Baghdasaryan <surenb@google.com>, Christian Brauner <brauner@kernel.org>, Rob Landley <rob@landley.net>, 
	Davidlohr Bueso <dave@stgolabs.net>, Michel Lespinasse <michel@lespinasse.org>, 
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 5, 2024 at 9:50=E2=80=AFAM <mattgilbride@google.com> wrote:
>
> From: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> This macro is used to obtain a pointer to an entire struct
> when given a pointer to a field in that struct.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Matt Gilbride <mattgilbride@google.com>
> ---
>  rust/kernel/lib.rs | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7ac39874aeac..c7963efd1318 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -102,3 +102,35 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>      // SAFETY: FFI call.
>      unsafe { bindings::BUG() };
>  }
> +
> +/// Produces a pointer to an object from a pointer to one of its fields.

It is in the examples but a note would be good to make it obvious:

    ///
    /// This macro must be called from within an `unsafe { }` block.

> +/// # Safety
> +///
> +/// The pointer passed to this macro, and the pointer returned by this m=
acro, must both be in
> +/// bounds of the same allocation.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::container_of;
> +/// struct Test {
> +///     a: u64,
> +///     b: u32,
> +/// }
> +///
> +/// let test =3D Test { a: 10, b: 20 };
> +/// let b_ptr =3D &test.b;
> +/// // SAFETY: The pointer points at the `b` field of a `Test`, so the r=
esulting pointer will be
> +/// // in-bounds of the same allocation as `b_ptr`.
> +/// let test_alias =3D unsafe { container_of!(b_ptr, Test, b) };
> +/// assert!(core::ptr::eq(&test, test_alias));
> +/// ```
> +#[macro_export]
> +macro_rules! container_of {
> +    ($ptr:expr, $type:ty, $($f:tt)*) =3D> {{
> +        let ptr =3D $ptr as *const _ as *const u8;
> +        let offset: usize =3D ::core::mem::offset_of!($type, $($f)*);

`offset_of` will be stable in 1.77 BUT only for a single field [1]. I
don't know if there are other blockers in the kernel already, but if
this could be changed to call `offset_of!` recursively then  it will
work with the stable version.

We might want an `offset_of_many!(a, b, c)` macro somewhere if there
are other places that need this nesting.

[1]: https://github.com/rust-lang/rust/pull/118799

> +        ptr.sub(offset) as *const $type

Instead of casting to and from `u8`, you should be able to use `byte_sub`

> +    }}
> +}

- Trevor