mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benno Lossin <benno.lossin@proton.me>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Andreas Hindborg" <a.hindborg@kernel.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/22] rust: pin-init: move impl `Zeroable` for `Opaque` and `Option<KBox<T>>` into the kernel crate
Date: Wed, 05 Mar 2025 12:17:18 +0000	[thread overview]
Message-ID: <D88BQVG0KLC5.27DTUSDE9D8C6@proton.me> (raw)
In-Reply-To: <CAH5fLghL+qzrD8KiCF1V3vf2YcC6aWySzkmaE2Zzrnh1gKj-hw@mail.gmail.com>

On Wed Mar 5, 2025 at 1:11 PM CET, Alice Ryhl wrote:
> On Wed, Mar 5, 2025 at 1:05 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> On Wed Mar 5, 2025 at 12:26 PM CET, Andreas Hindborg wrote:
>> > "Benno Lossin" <benno.lossin@proton.me> writes:
>> >
>> >> In order to make pin-init a standalone crate, move kernel-specific code
>> >> directly into the kernel crate. Since `Opaque<T>` and `KBox<T>` are part
>> >> of the kernel, move their `Zeroable` implementation into the kernel
>> >> crate.
>> >>
>> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> >> ---
>> >>  rust/kernel/alloc/kbox.rs | 8 +++++++-
>> >>  rust/kernel/types.rs      | 5 ++++-
>> >>  rust/pin-init/src/lib.rs  | 8 +-------
>> >>  3 files changed, 12 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
>> >> index 39a3ea7542da..9861433559dc 100644
>> >> --- a/rust/kernel/alloc/kbox.rs
>> >> +++ b/rust/kernel/alloc/kbox.rs
>> >> @@ -15,7 +15,7 @@
>> >>  use core::ptr::NonNull;
>> >>  use core::result::Result;
>> >>
>> >> -use crate::init::{InPlaceWrite, Init, PinInit};
>> >> +use crate::init::{InPlaceWrite, Init, PinInit, Zeroable};
>> >>  use crate::init_ext::InPlaceInit;
>> >>  use crate::types::ForeignOwnable;
>> >>
>> >> @@ -100,6 +100,12 @@
>> >>  /// ```
>> >>  pub type KVBox<T> = Box<T, super::allocator::KVmalloc>;
>> >>
>> >> +// SAFETY: All zeros is equivalent to `None` (option layout optimization guarantee).
>> >> +//
>> >> +// In this case we are allowed to use `T: ?Sized`, since all zeros is the `None` variant and there
>> >> +// is no problem with a VTABLE pointer being null.
>> >> +unsafe impl<T: ?Sized, A: Allocator> Zeroable for Option<Box<T, A>> {}
>> >
>> > Could you elaborate the statement related to vtable pointers? How does
>> > that come into play for `Option<Box<_>>`? Is it for fat pointers to
>> > trait objects?
>>
>> Yes it is for fat pointers, if you have a `x: *mut dyn Trait`, then you
>> aren't allowed to write all zeroes to `x`, because the VTABLE pointer
>> (that is part of the fat pointer) is not allowed to be null.
>>
>> Now for `Option<Box<_>>`, this doesn't matter, as there if the normal
>> pointer part of the fat pointer is all zeroes, then the VTABLE pointer
>> part is considered padding bytes, as it's the `None` variant.
>
> The standard library only guarantees that all zeros is valid for
> Option<Box<T,A>> when T:Sized and A=Global.
> https://doc.rust-lang.org/stable/std/option/index.html#representation

Oh! That's a problem then... I'll remove that then (and I can also get
rid of the `ZeroableOption` trait).

We should also backport (& fix it in mainline), I'll submit a patch
shortly.

---
Cheers,
Benno


  reply	other threads:[~2025-03-05 12:17 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250304225245.2033120-1-benno.lossin@proton.me>
2025-03-04 22:53 ` [PATCH 01/22] rust: init: disable doctests Benno Lossin
2025-03-05  8:51   ` Andreas Hindborg
2025-03-05 12:53     ` Benno Lossin
2025-03-05 13:00       ` Miguel Ojeda
2025-03-05 14:09       ` Andreas Hindborg
2025-03-05 14:31         ` Benno Lossin
2025-03-05  9:17   ` Fiona Behrens
2025-03-04 22:53 ` [PATCH 02/22] rust: move pin-init API into its own directory Benno Lossin
2025-03-05  9:03   ` Andreas Hindborg
2025-03-05  9:17   ` Fiona Behrens
2025-03-04 22:53 ` [PATCH 03/22] rust: add extensions to the pin-init crate and move relevant documentation there Benno Lossin
2025-03-05  9:11   ` Andreas Hindborg
2025-03-05 11:03     ` Benno Lossin
2025-03-05  9:17   ` Fiona Behrens
2025-03-04 22:53 ` [PATCH 04/22] rust: pin-init: move proc-macro documentation into pin-init crate Benno Lossin
2025-03-05  9:18   ` Fiona Behrens
2025-03-05  9:34   ` Andreas Hindborg
2025-03-05 11:05     ` Benno Lossin
2025-03-04 22:53 ` [PATCH 05/22] rust: pin-init: change examples to the user-space version Benno Lossin
2025-03-05  9:19   ` Fiona Behrens
2025-03-05 10:06   ` Andreas Hindborg
2025-03-04 22:53 ` [PATCH 06/22] rust: pin-init: call `try_[pin_]init!` from `[pin_]init!` instead of `__init_internal!` Benno Lossin
2025-03-05  9:19   ` Fiona Behrens
2025-03-05 10:12   ` Andreas Hindborg
2025-03-04 22:54 ` [PATCH 07/22] rust: pin-init: move the default error behavior of `try_[pin_]init` Benno Lossin
2025-03-05  9:21   ` Fiona Behrens
2025-03-05 10:29   ` Andreas Hindborg
2025-03-05 10:47     ` Benno Lossin
2025-03-04 22:54 ` [PATCH 08/22] rust: pin-init: move `InPlaceInit` and impls of `InPlaceWrite` into the kernel crate Benno Lossin
2025-03-05  9:23   ` Fiona Behrens
2025-03-05 11:18   ` Andreas Hindborg
2025-03-05 12:06     ` Benno Lossin
2025-03-05 12:28       ` Andreas Hindborg
2025-03-05 12:37         ` Benno Lossin
2025-03-04 22:54 ` [PATCH 09/22] rust: pin-init: move impl `Zeroable` for `Opaque` and `Option<KBox<T>>` " Benno Lossin
2025-03-05  9:24   ` Fiona Behrens
2025-03-05 11:26   ` Andreas Hindborg
2025-03-05 12:05     ` Benno Lossin
2025-03-05 12:11       ` Alice Ryhl
2025-03-05 12:17         ` Benno Lossin [this message]
2025-03-05 12:49           ` Alice Ryhl
2025-03-05 12:51             ` Benno Lossin
2025-03-04 22:54 ` [PATCH 10/22] rust: add `ZeroableOption` and implement it instead of `Zeroable` for `Option<Box<T, A>>` Benno Lossin
2025-03-05  9:25   ` Fiona Behrens
2025-03-05 11:30   ` Andreas Hindborg
2025-03-04 22:54 ` [PATCH 11/22] rust: pin-init: fix documentation links Benno Lossin
2025-03-05  9:26   ` Fiona Behrens
2025-03-05 11:37   ` Andreas Hindborg
2025-03-05 11:49     ` Benno Lossin
2025-03-04 22:54 ` [PATCH 12/22] rust: pin-init: remove kernel-crate dependency Benno Lossin
2025-03-05  9:27   ` Fiona Behrens
2025-03-05 11:49   ` Andreas Hindborg
2025-03-05 12:00     ` Benno Lossin
2025-03-05 12:27       ` Andreas Hindborg
2025-03-04 22:55 ` [PATCH 13/22] rust: pin-init: change the way the `paste!` macro is called Benno Lossin
2025-03-05  9:28   ` Fiona Behrens
2025-03-05 11:52   ` Andreas Hindborg
2025-03-04 22:55 ` [PATCH 14/22] rust: add pin-init crate build infrastructure Benno Lossin
2025-03-05 11:59   ` Andreas Hindborg
2025-03-05 12:10     ` Benno Lossin
2025-03-05 12:31       ` Andreas Hindborg
2025-03-05 12:50         ` Miguel Ojeda
2025-03-05 13:00           ` Benno Lossin
2025-03-05 14:19           ` Andreas Hindborg
2025-03-05 14:34             ` Benno Lossin
2025-03-05 12:47     ` Miguel Ojeda
2025-03-04 22:55 ` [PATCH 15/22] rust: make pin-init its own crate Benno Lossin
2025-03-05  9:29   ` Fiona Behrens
2025-03-05 12:12   ` Andreas Hindborg
2025-03-05 13:40     ` Benno Lossin
2025-03-05 14:20       ` Andreas Hindborg
2025-03-04 22:55 ` [PATCH 16/22] rust: pin-init: add `std` and `alloc` support from the user-space version Benno Lossin
2025-03-05  9:32   ` Fiona Behrens
2025-03-05 12:22   ` Andreas Hindborg
2025-03-05 13:55     ` Benno Lossin
2025-03-05 14:29       ` Andreas Hindborg
2025-03-05 15:05         ` Benno Lossin
2025-03-05 17:27           ` Andreas Hindborg
2025-03-04 22:55 ` [PATCH 17/22] rust: pin-init: synchronize documentation with " Benno Lossin
2025-03-05  9:33   ` Fiona Behrens
2025-03-05 12:52   ` Andreas Hindborg
2025-03-04 22:55 ` [PATCH 18/22] rust: pin-init: internal: synchronize with " Benno Lossin
2025-03-05 12:56   ` Andreas Hindborg
2025-03-04 22:56 ` [PATCH 19/22] rust: pin-init: miscellaneous synchronization with the " Benno Lossin
2025-03-05 12:57   ` Andreas Hindborg
2025-03-04 22:56 ` [PATCH 20/22] rust: pin-init: add miscellaneous files from " Benno Lossin
2025-03-05  9:35   ` Fiona Behrens
2025-03-05 13:04   ` Andreas Hindborg
2025-03-05 13:37     ` Miguel Ojeda
2025-03-05 13:58       ` Benno Lossin
2025-03-04 22:56 ` [PATCH 21/22] rust: pin-init: re-enable doctests Benno Lossin
2025-03-05  9:35   ` Fiona Behrens
2025-03-05 13:05   ` Andreas Hindborg
2025-03-04 22:56 ` [PATCH 22/22] MAINTAINERS: add entry for the `pin-init` crate Benno Lossin
2025-03-05  0:17   ` Jarkko Sakkinen
2025-03-05  0:43     ` Benno Lossin
2025-03-05  5:14       ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D88BQVG0KLC5.27DTUSDE9D8C6@proton.me \
    --to=benno.lossin@proton.me \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®