linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Benno Lossin <benno.lossin@proton.me>
To: Danilo Krummrich <dakr@kernel.org>
Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com,
	boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com,
	a.hindborg@samsung.com, aliceryhl@google.com,
	akpm@linux-foundation.org, daniel.almeida@collabora.com,
	faith.ekstrand@collabora.com, boris.brezillon@collabora.com,
	lina@asahilina.net, mcanal@igalia.com, zhiw@nvidia.com,
	cjia@nvidia.com, jhubbard@nvidia.com, airlied@redhat.com,
	ajanulgu@redhat.com, lyude@redhat.com,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v6 09/26] rust: alloc: implement kernel `Box`
Date: Wed, 11 Sep 2024 13:26:32 +0000	[thread overview]
Message-ID: <b80cb238-5fcc-4bbb-8b03-42e173c28653@proton.me> (raw)
In-Reply-To: <ZuF444VO8gK8l7UR@pollux>

On 11.09.24 13:02, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 08:36:38AM +0000, Benno Lossin wrote:
>> On 11.09.24 01:25, Danilo Krummrich wrote:
>>> On Tue, Sep 10, 2024 at 07:49:42PM +0000, Benno Lossin wrote:
>>>> On 10.09.24 19:40, Danilo Krummrich wrote:
>>>>> On Sat, Aug 31, 2024 at 05:39:07AM +0000, Benno Lossin wrote:
>>>>>> On 16.08.24 02:10, Danilo Krummrich wrote:
>>>>>>> +///
>>>>>>> +/// ```
>>>>>>> +/// # use kernel::bindings;
>>>>>>> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
>>>>>>> +/// struct Huge([u8; SIZE]);
>>>>>>> +///
>>>>>>> +/// assert!(KVBox::<Huge>::new_uninit(GFP_KERNEL).is_ok());
>>>>>>> +/// ```
>>>>>>
>>>>>> Similarly, you could then say above this one "Instead use either `VBox`
>>>>>> or `KVBox`:"
>>>>>>
>>>>>>> +///
>>>>>>> +/// # Invariants
>>>>>>> +///
>>>>>>> +/// The [`Box`]' pointer is always properly aligned and either points to memory allocated with `A`
>>>>>>
>>>>>> Please use `self.0` instead of "[`Box`]'".
>>>>>>
>>>>>>> +/// or, for zero-sized types, is a dangling pointer.
>>>>>>
>>>>>> Probably "dangling, well aligned pointer.".
>>>>>
>>>>> Does this add any value? For ZSTs everything is "well aligned", isn't it?
>>>>
>>>> ZSTs can have alignment and then unaligned pointers do exist for them
>>>> (and dereferencing them is UB!):
>>>
>>> Where is this documented? The documentation says:
>>>
>>> "For operations of size zero, *every* pointer is valid, including the null
>>> pointer. The following points are only concerned with non-zero-sized accesses."
>>> [1]
>>
>> That's a good point, the documentation looks a bit outdated. I found
>> this page in the nomicon: https://doc.rust-lang.org/nomicon/vec/vec-zsts.html
>> The first iterator implementation has an alignment issue. (Nevertheless,
>> that chapter of the nomicon is probably useful to you, since it goes
>> over implementing `Vec`, but maybe you already saw it)
>>
>>> [1] https://doc.rust-lang.org/std/ptr/index.html
>>
>> Might be a good idea to improve/complain about this at the rust project.
> 
> Well, my point is how do we know? There's no language specification and the
> documentation is (at least) ambiguous.

So I went through the unsafe-coding-guidelines issues list and only
found this one: https://github.com/rust-lang/unsafe-code-guidelines/issues/93
Maybe I missed something. You could also try to ask at the rust zulip in
the t-opsem channel for further clarification.

I think we should just be on the safe side and assume that ZSTs require
alignment. But if you get a convincing answer and if they say that they
will document it, then I don't mind removing the alignment requirement.

---
Cheers,
Benno



  reply	other threads:[~2024-09-11 13:26 UTC|newest]

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-16  0:10 [PATCH v6 00/26] Generic `Allocator` support for Rust Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 01/26] rust: alloc: add `Allocator` trait Danilo Krummrich
2024-08-29 18:19   ` Benno Lossin
2024-08-29 21:56     ` Danilo Krummrich
2024-08-30 13:06       ` Benno Lossin
2024-09-03 11:56         ` Danilo Krummrich
2024-09-10 13:03           ` Benno Lossin
2024-09-10 13:23             ` Danilo Krummrich
2024-09-10 19:37               ` Benno Lossin
2024-08-30 13:44   ` Benno Lossin
2024-08-31 12:01   ` Gary Guo
2024-08-16  0:10 ` [PATCH v6 02/26] rust: alloc: separate `aligned_size` from `krealloc_aligned` Danilo Krummrich
2024-08-31 12:16   ` Gary Guo
2024-08-16  0:10 ` [PATCH v6 03/26] rust: alloc: rename `KernelAllocator` to `Kmalloc` Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 04/26] rust: alloc: implement `Allocator` for `Kmalloc` Danilo Krummrich
2024-08-29 18:32   ` Benno Lossin
2024-08-29 22:04     ` Danilo Krummrich
2024-08-30 14:45       ` Benno Lossin
2024-09-03 11:48         ` Danilo Krummrich
2024-09-10 13:11           ` Benno Lossin
2024-09-10 13:37             ` Danilo Krummrich
2024-09-10 19:42               ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 05/26] rust: alloc: add module `allocator_test` Danilo Krummrich
2024-08-31 12:18   ` Gary Guo
2024-08-16  0:10 ` [PATCH v6 06/26] rust: alloc: implement `Vmalloc` allocator Danilo Krummrich
2024-08-31  5:21   ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 07/26] rust: alloc: implement `KVmalloc` allocator Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 08/26] rust: alloc: add __GFP_NOWARN to `Flags` Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 09/26] rust: alloc: implement kernel `Box` Danilo Krummrich
2024-08-20  9:47   ` Alice Ryhl
2024-08-20 15:26     ` Danilo Krummrich
2024-08-27 19:21       ` Boqun Feng
2024-08-31  5:39   ` Benno Lossin
2024-09-10 17:40     ` Danilo Krummrich
2024-09-10 19:49       ` Benno Lossin
2024-09-10 23:25         ` Danilo Krummrich
2024-09-11  8:36           ` Benno Lossin
2024-09-11 11:02             ` Danilo Krummrich
2024-09-11 13:26               ` Benno Lossin [this message]
2024-09-11 13:27                 ` Alice Ryhl
2024-09-11 14:50                   ` Danilo Krummrich
2024-09-12  8:03                     ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 10/26] rust: treewide: switch to our kernel `Box` type Danilo Krummrich
2024-08-29 18:35   ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 11/26] rust: alloc: remove `BoxExt` extension Danilo Krummrich
2024-08-29 18:38   ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 12/26] rust: alloc: add `Box` to prelude Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 13/26] rust: alloc: implement kernel `Vec` type Danilo Krummrich
2024-09-03 19:08   ` Boqun Feng
2024-09-10 18:26     ` Danilo Krummrich
2024-09-10 19:33       ` Benno Lossin
2024-09-10 19:32   ` Benno Lossin
2024-09-11  0:18     ` Danilo Krummrich
2024-09-11  8:46       ` Benno Lossin
2024-09-10 20:07   ` Benno Lossin
2024-09-11 21:59     ` Danilo Krummrich
2024-09-23  9:24       ` Alice Ryhl
2024-08-16  0:10 ` [PATCH v6 14/26] rust: alloc: implement `IntoIterator` for `Vec` Danilo Krummrich
2024-09-04 10:29   ` Alice Ryhl
2024-09-10 20:04   ` Benno Lossin
2024-09-10 23:39     ` Danilo Krummrich
2024-09-11  8:52       ` Benno Lossin
2024-09-11 11:32         ` Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 15/26] rust: alloc: implement `collect` for `IntoIter` Danilo Krummrich
2024-09-10 20:12   ` Benno Lossin
2024-09-11  0:22     ` Danilo Krummrich
2024-09-11  8:53       ` Benno Lossin
2024-09-11 11:33         ` Danilo Krummrich
2024-08-16  0:10 ` [PATCH v6 16/26] rust: treewide: switch to the kernel `Vec` type Danilo Krummrich
2024-08-29 18:41   ` Benno Lossin
2024-08-16  0:10 ` [PATCH v6 17/26] rust: alloc: remove `VecExt` extension Danilo Krummrich
2024-08-16  0:11 ` [PATCH v6 18/26] rust: alloc: add `Vec` to prelude Danilo Krummrich
2024-08-16  0:11 ` [PATCH v6 19/26] rust: error: use `core::alloc::LayoutError` Danilo Krummrich
2024-08-16  0:11 ` [PATCH v6 20/26] rust: error: check for config `test` in `Error::name` Danilo Krummrich
2024-08-29 18:41   ` Benno Lossin
2024-08-16  0:11 ` [PATCH v6 21/26] rust: alloc: implement `contains` for `Flags` Danilo Krummrich
2024-08-29 18:42   ` Benno Lossin
2024-08-16  0:11 ` [PATCH v6 22/26] rust: alloc: implement `Cmalloc` in module allocator_test Danilo Krummrich
2024-08-29 19:14   ` Benno Lossin
2024-08-29 22:25     ` Danilo Krummrich
2024-08-30 12:56       ` Benno Lossin
2024-09-11 12:31       ` Danilo Krummrich
2024-09-11 13:32         ` Benno Lossin
2024-09-11 14:37           ` Danilo Krummrich
2024-09-12  8:18             ` Benno Lossin
2024-08-16  0:11 ` [PATCH v6 23/26] rust: str: test: replace `alloc::format` Danilo Krummrich
2024-08-29 18:43   ` Benno Lossin
2024-08-16  0:11 ` [PATCH v6 24/26] rust: alloc: update module comment of alloc.rs Danilo Krummrich
2024-08-16  0:11 ` [PATCH v6 25/26] kbuild: rust: remove the `alloc` crate and `GlobalAlloc` Danilo Krummrich
2024-08-21 21:34   ` Benno Lossin
2024-08-16  0:11 ` [PATCH v6 26/26] MAINTAINERS: add entry for the Rust `alloc` module Danilo Krummrich
2024-08-31 12:57   ` Gary Guo
2024-09-03 12:03     ` Danilo Krummrich
2024-09-04 10:15       ` Alice Ryhl
2024-09-04 12:51         ` Benno Lossin
     [not found]           ` <CANiq72=MD8jmWb9EGA8yW6eMT6Prj8fYEiJM81-HTq3p4dKmGg@mail.gmail.com>
2024-09-10 13:26             ` Benno Lossin
2024-09-10 13:42               ` Danilo Krummrich
2024-09-10 14:27                 ` Benno Lossin
2024-08-27 19:17 ` [PATCH v6 00/26] Generic `Allocator` support for Rust Boqun Feng

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=b80cb238-5fcc-4bbb-8b03-42e173c28653@proton.me \
    --to=benno.lossin@proton.me \
    --cc=a.hindborg@samsung.com \
    --cc=airlied@redhat.com \
    --cc=ajanulgu@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=cjia@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=faith.ekstrand@collabora.com \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=lina@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lyude@redhat.com \
    --cc=mcanal@igalia.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@gmail.com \
    --cc=zhiw@nvidia.com \
    /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