Add `ARef::into_raw` #1044

y86-dev · 2023-12-02T10:10:12Z

Add the function into_raw to ARef<T>, the function should:

consume the ARef<T> by value, but not run the destructor, so you need to mem::forget the ARef<T>.
return a NonNull<T> or a *mut T, figure out which one works better.
have an example and some documentation.

Also adjust instances where ARefs are manually forgotten to use this new function.

This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes, to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and so on. Please see https://docs.kernel.org/process/submitting-patches.html and https://rust-for-linux.com/contributing for details.

Please take this issue only if you are new to the kernel development process and you would like to use it as a test to submit your first patch to the kernel.

The text was updated successfully, but these errors were encountered:

Kartik1397 · 2023-12-03T12:48:36Z

I'll take this one.

y86-dev · 2023-12-03T13:53:35Z

Sure!

Note that there currently are no instances of ARefs getting manually forgotten. I created this issue, while reviewing [PATCH 4/7] rust: file: add FileDescriptorReservation. So depending on which thing makes it first into upstream you will or won't have to adjust other stuff.

Redhawk18 · 2024-01-14T06:41:01Z

I'll take this one.

Are you currently working on it?

Kartik1397 · 2024-01-14T09:17:19Z

Yes @Redhawk18, I've the patch ready here, which I'm planning to submit in couple of days once I complete testing of changes in rust/kernel/net/filter.rs and rust/kernel/file.rs files.

Kartik1397 · 2024-01-15T18:58:08Z

Hi @y86-dev, I've drafted a PR for initial review. Please take a look when you can.
I'll send the patch to the mailing list after that.

Add the function `into_raw` to `ARef<T>`. This method can be used to turn an `ARef` into a raw pointer. Link: Rust-for-Linux#1044 Co-developed-by: Vincenzo Palazzo <[email protected]> Signed-off-by: Kartik Prajapati <[email protected]>

vincenzopalazzo · 2024-03-29T20:26:31Z

The PR is sent upstream through ML https://lore.kernel.org/all/[email protected]/

Add the function `into_raw` to `ARef<T>`. This method can be used to turn an `ARef` into a raw pointer. Link: Rust-for-Linux#1044 Co-developed-by: Vincenzo Palazzo <[email protected]> Signed-off-by: Kartik Prajapati <[email protected]>

ojeda · 2024-08-20T16:57:06Z

@Kartik1397 We are thinking of using NonNull as the return type -- Alice sent an alternative patch (unaware of yours), and I think ideally we would get a combined approach with the best of both, i.e. an example like yours but returning NonNull like she did. Would you be OK to be the primary author of that approach? (i.e. with NonNull)?

https://lore.kernel.org/rust-for-linux/[email protected]/
https://lore.kernel.org/rust-for-linux/[email protected]/

Alice is happy sending the patch (with you as the primary author), but if you want to send it yourself, that is also fine.

Thanks!

ojeda · 2024-08-20T16:58:29Z

Something like (from Alice):

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index ee7dd1f963ef..9e7ca066355c 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -7,7 +7,7 @@
 use core::{
     cell::UnsafeCell,
     marker::{PhantomData, PhantomPinned},
-    mem::MaybeUninit,
+    mem::{ManuallyDrop, MaybeUninit},
     ops::{Deref, DerefMut},
     pin::Pin,
     ptr::NonNull,
@@ -396,6 +396,35 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
             _p: PhantomData,
         }
     }
+
+    /// Consumes the `ARef`, returning a raw pointer.
+    ///
+    /// This function does not change the refcount. After calling this function, the caller is
+    /// responsible for the refcount previously managed by the `ARef`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use core::ptr::NonNull;
+    /// use kernel::types::{ARef, AlwaysRefCounted};
+    ///
+    /// struct Empty {}
+    ///
+    /// unsafe impl AlwaysRefCounted for Empty {
+    ///     fn inc_ref(&self) {}
+    ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
+    /// }
+    ///
+    /// let mut data = Empty {};
+    /// let ptr = NonNull::<Empty>::new(&mut data as *mut _).unwrap();
+    /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
+    /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
+    ///
+    /// assert_eq!(ptr, raw_ptr);
+    /// ```
+    pub fn into_raw(me: Self) -> NonNull<T> {
+        ManuallyDrop::new(me).ptr
+    }

ojeda · 2024-08-20T17:15:08Z

By the way, I think @vincenzopalazzo did not change the code when he sent @Kartik1397 patch to the mailing list. But if Vincenzo contributed to the patch and should have a Co-developed-by, then please let us know (https://lore.kernel.org/rust-for-linux/CANiq72==OBG+KVJuzW0S5aYowAZ4oq91pk=phRgtedUrGYeHAg@mail.gmail.com/). Thanks!

vincenzopalazzo · 2024-08-20T22:37:30Z

@ojeda you are right, I did not change the code I just push it to the finish line through the ML :)

Kartik1397 · 2024-08-21T19:29:08Z

@ojeda I'm OK with using NonNull as the return type.
I'm also fine with Alice sending the patch with me as the primary author.

Thanks considering my patch!

Add a method for `ARef` that is analogous to `Arc::into_raw`. It is the inverse operation of `ARef::from_raw`, and allows you to convert the `ARef` back into a raw pointer while retaining ownership of the refcount. This new function will be used by [1] for converting the type in an `ARef` using `ARef::from_raw(ARef::into_raw(me).cast())`. The author has also needed the same function for other use-cases in the past, but [1] is the first to go upstream. This was implemented independently by Kartik and Alice. The two versions were merged by Alice, so all mistakes are Alice's. Link: https://lore.kernel.org/r/[email protected] [1] Closes: Rust-for-Linux#1044 Signed-off-by: Kartik Prajapati <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]>

Add a method for `ARef` that is analogous to `Arc::into_raw`. It is the inverse operation of `ARef::from_raw`, and allows you to convert the `ARef` back into a raw pointer while retaining ownership of the refcount. This new function will be used by [1] for converting the type in an `ARef` using `ARef::from_raw(ARef::into_raw(me).cast())`. Alice has also needed the same function for other use-cases in the past, but [1] is the first to go upstream. This was implemented independently by Kartik and Alice. The two versions were merged by Alice, so all mistakes are Alice's. Link: https://lore.kernel.org/r/[email protected] [1] Closes: Rust-for-Linux#1044 Signed-off-by: Kartik Prajapati <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Benno Lossin <[email protected]> [ Reworded to correct the name. - Miguel ] Signed-off-by: Miguel Ojeda <[email protected]>

Add a method for `ARef` that is analogous to `Arc::into_raw`. It is the inverse operation of `ARef::from_raw`, and allows you to convert the `ARef` back into a raw pointer while retaining ownership of the refcount. This new function will be used by [1] for converting the type in an `ARef` using `ARef::from_raw(ARef::into_raw(me).cast())`. Alice has also needed the same function for other use-cases in the past, but [1] is the first to go upstream. This was implemented independently by Kartik and Alice. The two versions were merged by Alice, so all mistakes are Alice's. Link: https://lore.kernel.org/r/[email protected] [1] Link: #1044 Signed-off-by: Kartik Prajapati <[email protected]> Co-developed-by: Alice Ryhl <[email protected]> Signed-off-by: Alice Ryhl <[email protected]> Reviewed-by: Benno Lossin <[email protected]> [ Reworded to correct the author reference and changed tag to Link since it is not a bug. - Miguel ] Signed-off-by: Miguel Ojeda <[email protected]>

ojeda · 2024-08-25T12:54:56Z

Applied to rust-next -- thanks everyone!

y86-dev added • lib Related to the `rust/` library. good first issue Good for newcomers labels Dec 2, 2023

Kartik1397 mentioned this issue Jan 15, 2024

Add ARef::into_raw #1056

Open

ojeda closed this as completed Aug 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `ARef::into_raw` #1044

Add `ARef::into_raw` #1044

y86-dev commented Dec 2, 2023

Kartik1397 commented Dec 3, 2023

y86-dev commented Dec 3, 2023

Redhawk18 commented Jan 14, 2024

Kartik1397 commented Jan 14, 2024

Kartik1397 commented Jan 15, 2024

vincenzopalazzo commented Mar 29, 2024

ojeda commented Aug 20, 2024

ojeda commented Aug 20, 2024 •

edited

Loading

ojeda commented Aug 20, 2024

vincenzopalazzo commented Aug 20, 2024

Kartik1397 commented Aug 21, 2024

ojeda commented Aug 25, 2024

Add ARef::into_raw #1044

Add ARef::into_raw #1044

Comments

y86-dev commented Dec 2, 2023

Kartik1397 commented Dec 3, 2023

y86-dev commented Dec 3, 2023

Redhawk18 commented Jan 14, 2024

Kartik1397 commented Jan 14, 2024

Kartik1397 commented Jan 15, 2024

vincenzopalazzo commented Mar 29, 2024

ojeda commented Aug 20, 2024

ojeda commented Aug 20, 2024 • edited Loading

ojeda commented Aug 20, 2024

vincenzopalazzo commented Aug 20, 2024

Kartik1397 commented Aug 21, 2024

ojeda commented Aug 25, 2024

Add `ARef::into_raw` #1044

Add `ARef::into_raw` #1044

ojeda commented Aug 20, 2024 •

edited

Loading