rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: Gary Guo <gary@garyguo.net>
Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com,
	boqun.feng@gmail.com, bjorn3_gh@protonmail.com,
	benno.lossin@proton.me, 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 v7 01/26] rust: alloc: add `Allocator` trait
Date: Sun, 15 Sep 2024 23:37:38 +0200	[thread overview]
Message-ID: <ZudTon205iOCdQbq@pollux> (raw)
In-Reply-To: <20240915202242.7a16b3d3.gary@garyguo.net>

On Sun, Sep 15, 2024 at 08:22:42PM +0100, Gary Guo wrote:
> On Sun, 15 Sep 2024 19:02:40 +0200
> Danilo Krummrich <dakr@kernel.org> wrote:
> > There is also 4.
> > 
> > Let `alloc` and `realloc` return a properly aligned dangling pointer for
> > `size == 0` and don't accept dangling pointers in `realloc` and `free`.
> 
> I'll consider the API design to be bad if I can't pass allocated pointer to
> free. If caller needs to handle ZST specially then we might as well
> just ban it completely.

Fine for me -- I don't see a need to support ZSTs with `Allocator`.

I think its main purpose is to give us an interface to actually allocate memory.
We probably don't need it to be a "dangling pointer generator".

> 
> > And 5.
> > 
> > Reject the combination of `None` and `size == 0` entirely, as earlier proposed
> > by Benno.
> > 
> > I'm fine with both, 4. and 5. with a slight preference for 4.
> > 
> > I'd also go along with 1., as a mix of 4. and 5.
> > 
> > I really don't like making `alloc` unsafe, and I really don't want to have
> > `old_layout` in `free`. Please let's not discuss this again. :-)
> 
> I don't buy it.
> 
> Your argument for having `old_layout` is so that the caller doesn't
> need to care about the size. But as demonstrated the caller *does* need
> to care about whether the size is zero.
> 
> Our previous discussion doesn't cover the particular case of ZST and
> you said that it reason arise that we need this extra parameter, then
> it could be added. It feels to me that sane behaviour when it comes
> to ZST allocation is a very good reason.

I don't see why we should "optimize" the API for creating dangling pointers and
be able to pass them to `free` (which does not serve any practical purpose).

I don't want to add arguments that are meaningless for the actual backing
allocators (such as Kmalloc, Vmalloc, etc.), only to be able to generate and
"free" pointers for ZSTs with arbitrary alignment.

Do we even have use cases for ZSTs with other alignments?

> 
> > 
> > > 
> > > Best,
> > > Gary
> > >   
> > > > 
> > > > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > > > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > > > ---
> > > >  rust/kernel/alloc.rs | 112 +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 112 insertions(+)
> 

  parent reply	other threads:[~2024-09-15 21:37 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-11 22:52 [PATCH v7 00/26] Generic `Allocator` support for Rust Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 01/26] rust: alloc: add `Allocator` trait Danilo Krummrich
2024-09-15 15:28   ` Gary Guo
2024-09-15 17:02     ` Danilo Krummrich
2024-09-15 19:22       ` Gary Guo
2024-09-15 20:08         ` Gary Guo
2024-09-15 21:39           ` Danilo Krummrich
2024-09-15 21:37         ` Danilo Krummrich [this message]
2024-09-21 15:32   ` [RFC PATCH] rust: alloc: pass `old_layout` to `Allocator` Danilo Krummrich
2024-09-23 13:56     ` Alice Ryhl
2024-09-23 15:20       ` Benno Lossin
2024-09-23 16:13       ` Gary Guo
2024-09-24 13:31         ` Danilo Krummrich
2024-09-24 13:34           ` Danilo Krummrich
2024-09-24 19:58           ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 02/26] rust: alloc: separate `aligned_size` from `krealloc_aligned` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 03/26] rust: alloc: rename `KernelAllocator` to `Kmalloc` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 04/26] rust: alloc: implement `Allocator` for `Kmalloc` Danilo Krummrich
2024-09-26 13:00   ` Benno Lossin
2024-09-26 13:24     ` Danilo Krummrich
2024-09-26 14:00       ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 05/26] rust: alloc: add module `allocator_test` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 06/26] rust: alloc: implement `Vmalloc` allocator Danilo Krummrich
2024-09-26 13:06   ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 07/26] rust: alloc: implement `KVmalloc` allocator Danilo Krummrich
2024-09-26 13:07   ` Benno Lossin
2024-09-11 22:52 ` [PATCH v7 08/26] rust: alloc: add __GFP_NOWARN to `Flags` Danilo Krummrich
2024-09-28 18:55   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 09/26] rust: alloc: implement kernel `Box` Danilo Krummrich
2024-09-26 13:23   ` Benno Lossin
2024-09-28 18:54   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 10/26] rust: treewide: switch to our kernel `Box` type Danilo Krummrich
2024-09-28 18:59   ` Gary Guo
2024-09-29 14:52     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 11/26] rust: alloc: remove extension of std's `Box` Danilo Krummrich
2024-09-28 19:00   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 12/26] rust: alloc: add `Box` to prelude Danilo Krummrich
2024-09-28 19:00   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 13/26] rust: alloc: implement kernel `Vec` type Danilo Krummrich
2024-09-26 13:47   ` Benno Lossin
2024-09-28 12:43     ` Danilo Krummrich
2024-09-28 13:20       ` Benno Lossin
2024-09-28 19:14   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 14/26] rust: alloc: implement `IntoIterator` for `Vec` Danilo Krummrich
2024-09-26 13:53   ` Benno Lossin
2024-09-28 19:20   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 15/26] rust: alloc: implement `collect` for `IntoIter` Danilo Krummrich
2024-09-26 13:57   ` Benno Lossin
2024-09-28 19:27   ` Gary Guo
2024-09-29 15:12     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 16/26] rust: treewide: switch to the kernel `Vec` type Danilo Krummrich
2024-09-28 19:28   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 17/26] rust: alloc: remove `VecExt` extension Danilo Krummrich
2024-09-28 19:29   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 18/26] rust: alloc: add `Vec` to prelude Danilo Krummrich
2024-09-28 19:29   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 19/26] rust: error: use `core::alloc::LayoutError` Danilo Krummrich
2024-09-28 19:30   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 20/26] rust: error: check for config `test` in `Error::name` Danilo Krummrich
2024-09-28 19:30   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 21/26] rust: alloc: implement `contains` for `Flags` Danilo Krummrich
2024-09-28 19:31   ` Gary Guo
2024-09-11 22:52 ` [PATCH v7 22/26] rust: alloc: implement `Cmalloc` in module allocator_test Danilo Krummrich
2024-09-28 19:35   ` Gary Guo
2024-09-29 15:14     ` Danilo Krummrich
2024-09-11 22:52 ` [PATCH v7 23/26] rust: str: test: replace `alloc::format` Danilo Krummrich
2024-09-28 19:37   ` Gary Guo
2024-09-11 22:53 ` [PATCH v7 24/26] rust: alloc: update module comment of alloc.rs Danilo Krummrich
2024-09-28 19:38   ` Gary Guo
2024-09-11 22:53 ` [PATCH v7 25/26] kbuild: rust: remove the `alloc` crate and `GlobalAlloc` Danilo Krummrich
2024-09-28 19:43   ` Gary Guo
2024-09-29 15:17     ` Danilo Krummrich
2024-10-01 13:27       ` Danilo Krummrich
2024-10-03 21:41     ` Miguel Ojeda
2024-10-03 21:53       ` Danilo Krummrich
2024-10-03 22:49         ` Miguel Ojeda
2024-09-11 22:53 ` [PATCH v7 26/26] MAINTAINERS: add entry for the Rust `alloc` module Danilo Krummrich

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=ZudTon205iOCdQbq@pollux \
    --to=dakr@kernel.org \
    --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=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=cjia@nvidia.com \
    --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;
as well as URLs for NNTP newsgroup(s).