linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com,
	boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com,
	benno.lossin@proton.me, a.hindborg@samsung.com,
	aliceryhl@google.com, akpm@linux-foundation.org
Cc: 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,
	Danilo Krummrich <dakr@kernel.org>
Subject: [PATCH v7 15/26] rust: alloc: implement `collect` for `IntoIter`
Date: Thu, 12 Sep 2024 00:52:51 +0200	[thread overview]
Message-ID: <20240911225449.152928-16-dakr@kernel.org> (raw)
In-Reply-To: <20240911225449.152928-1-dakr@kernel.org>

Currently, we can't implement `FromIterator`. There are a couple of
issues with this trait in the kernel, namely:

  - Rust's specialization feature is unstable. This prevents us to
    optimze for the special case where `I::IntoIter` equals `Vec`'s
    `IntoIter` type.
  - We also can't use `I::IntoIter`'s type ID either to work around this,
    since `FromIterator` doesn't require this type to be `'static`.
  - `FromIterator::from_iter` does return `Self` instead of
    `Result<Self, AllocError>`, hence we can't properly handle allocation
    failures.
  - Neither `Iterator::collect` nor `FromIterator::from_iter` can handle
    additional allocation flags.

Instead, provide `IntoIter::collect`, such that we can at least convert
`IntoIter` into a `Vec` again.

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

diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index e91761c5c52d..686e969463f8 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -690,6 +690,92 @@ impl<T, A> IntoIter<T, A>
     fn as_raw_mut_slice(&mut self) -> *mut [T] {
         ptr::slice_from_raw_parts_mut(self.ptr, self.len)
     }
+
+    fn into_raw_parts(self) -> (*mut T, NonNull<T>, usize, usize) {
+        let me = ManuallyDrop::new(self);
+        let ptr = me.ptr;
+        let buf = me.buf;
+        let len = me.len;
+        let cap = me.cap;
+        (ptr, buf, len, cap)
+    }
+
+    /// Same as `Iterator::collect` but specialized for `Vec`'s `IntoIter`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let v = kernel::kvec![1, 2, 3]?;
+    /// let mut it = v.into_iter();
+    ///
+    /// assert_eq!(it.next(), Some(1));
+    ///
+    /// let v = it.collect(GFP_KERNEL);
+    /// assert_eq!(v, [2, 3]);
+    ///
+    /// # Ok::<(), Error>(())
+    /// ```
+    /// # Implementation Details
+    ///
+    /// Currently, we can't implement `FromIterator`. There are a couple of issues with this trait
+    /// in the kernel, namely:
+    ///
+    /// - Rust's specialization feature is unstable. This prevents us to optimze for the special
+    ///   case where `I::IntoIter` equals `Vec`'s `IntoIter` type.
+    /// - We also can't use `I::IntoIter`'s type ID either to work around this, since `FromIterator`
+    ///   doesn't require this type to be `'static`.
+    /// - `FromIterator::from_iter` does return `Self` instead of `Result<Self, AllocError>`, hence
+    ///   we can't properly handle allocation failures.
+    /// - Neither `Iterator::collect` nor `FromIterator::from_iter` can handle additional allocation
+    ///   flags.
+    ///
+    /// Instead, provide `IntoIter::collect`, such that we can at least convert a `IntoIter` into a
+    /// `Vec` again.
+    ///
+    /// Note that `IntoIter::collect` doesn't require `Flags`, since it re-uses the existing backing
+    /// buffer. However, this backing buffer may be shrunk to the actual count of elements.
+    pub fn collect(self, flags: Flags) -> Vec<T, A> {
+        let (mut ptr, buf, len, mut cap) = self.into_raw_parts();
+        let has_advanced = ptr != buf.as_ptr();
+
+        if has_advanced {
+            // Copy the contents we have advanced to at the beginning of the buffer.
+            //
+            // SAFETY:
+            // - `ptr` is valid for reads of `len * size_of::<T>()` bytes,
+            // - `buf.as_ptr()` is valid for writes of `len * size_of::<T>()` bytes,
+            // - `ptr` and `buf.as_ptr()` are not be subject to aliasing restrictions relative to
+            //   each other,
+            // - both `ptr` and `buf.ptr()` are properly aligned.
+            unsafe { ptr::copy(ptr, buf.as_ptr(), len) };
+            ptr = buf.as_ptr();
+        }
+
+        // This can never fail, `len` is guaranteed to be smaller than `cap`.
+        let layout = core::alloc::Layout::array::<T>(len).unwrap();
+
+        // SAFETY: `buf` points to the start of the backing buffer and `len` is guaranteed to be
+        // smaller than `cap`. Depending on `alloc` this operation may shrink the buffer or leaves
+        // it as it is.
+        ptr = match unsafe { A::realloc(Some(buf.cast()), layout, flags) } {
+            // If we fail to shrink, which likely can't even happen, continue with the existing
+            // buffer.
+            Err(_) => ptr,
+            Ok(ptr) => {
+                cap = len;
+                ptr.as_ptr().cast()
+            }
+        };
+
+        // SAFETY: If the iterator has been advanced, the advanced elements have been copied to
+        // the beginning of the buffer and `len` has been adjusted accordingly.
+        //
+        // - `ptr` is guaranteed to point to the start of the backing buffer.
+        // - `cap` is either the original capacity or, after shrinking the buffer, equal to `len`.
+        // - `alloc` is guaranteed to be unchanged since `into_iter` has been called on the original
+        //   `Vec`.
+        unsafe { Vec::from_raw_parts(ptr, len, cap) }
+    }
 }
 
 impl<T, A> Iterator for IntoIter<T, A>
-- 
2.46.0



  parent reply	other threads:[~2024-09-11 22:56 UTC|newest]

Thread overview: 74+ 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
2024-09-21 15:32   ` [RFC PATCH] rust: alloc: pass `old_layout` to `Allocator` Danilo Krummrich
     [not found]     ` <CAH5fLgievsJOe7QET+Wh2c0upygv-nhSnOuTN8K_QkruLwOPgA@mail.gmail.com>
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 ` Danilo Krummrich [this message]
2024-09-26 13:57   ` [PATCH v7 15/26] rust: alloc: implement `collect` for `IntoIter` 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
     [not found]     ` <CANiq72nuZ41eDXkybGBbGRMgJzUOe1rRZioS-amJfH7UV-9cMQ@mail.gmail.com>
2024-10-03 21:53       ` Danilo Krummrich
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=20240911225449.152928-16-dakr@kernel.org \
    --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