Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ARef::into_raw #1056

Open
wants to merge 1 commit into
base: rust-next
Choose a base branch
from
Open

Add ARef::into_raw #1056

wants to merge 1 commit into from

Conversation

Kartik1397
Copy link

@Kartik1397 Kartik1397 commented Jan 15, 2024 •
edited
Loading

Closes: #1044

@Kartik1397
Copy link
Author

Opened this PR for the initial review.

@Kartik1397 Kartik1397 marked this pull request as ready for review January 16, 2024 14:39
@y86-dev
Copy link
Member

y86-dev commented Jan 24, 2024

Thanks for the patch!

I think it would be a good idea to mention that this function does not decrement the reference count.

For the example it would be very nice to not have to create an Empty type that implements AlwaysRefCounted. In the rust-dev branch you could use File .

@Kartik1397
rust: kernel: types: Add ARef::into_raw
10cef0c 
Add the function `into_raw` to `ARef<T>`. This method
can be used to turn an `ARef` into a raw pointer.

Signed-off-by: Kartik Prajapati <kartikprajapati987@gmail.com>
@Kartik1397
Copy link
Author

Thanks for checking the patch @y86-dev!

I think it would be a good idea to mention that this function does not decrement the reference count.

I've added this as a note in doc comment.

For the example it would be very nice to not have to create an Empty type that implements AlwaysRefCounted. In the rust-dev branch you could use File.

In rust-next only struct Task implements AlwaysRefCounted trait. Should I try to write an example using Task?
Also, do changes to rust-dev branch also go through the mailing list or github PR?

@ojeda
Copy link
Member

ojeda commented Jan 25, 2024 •
edited
Loading

Everything for upstream goes through the mailing list -- please see https://rust-for-linux.com/contributing#the-kernel-development-process.

While File is not yet in rust-next, it may land relatively soon. So if you wish to use File, you could send the patch on top of the File patch series.

@Kartik1397
Copy link
Author

While File is not yet in rust-next, it may land relatively soon. So if you wish to use File, you could send the patch on top of the File patch series.

To send a patch on top of another patch series, do I need to do anything apart from mentioning dependent patch in commit message as described in this doc?

@ojeda
Copy link
Member

ojeda commented Jan 27, 2024

That would be enough, yeah (below the --- marker, i.e. outside the commit message itself). Ideally, you can put the URL to the patch series you used, to be unambiguous. For instance: https://lore.kernel.org/rust-for-linux/20240118-alice-file-v3-0-9694b6f9580c@google.com/

Thanks!

@vincenzopalazzo
Copy link

@ojeda this patch when upstream? or I can take to push it to the finish line?

@ojeda
Copy link
Member

ojeda commented Mar 20, 2024

I don't think it was sent to the mailing list.

@ojeda
Copy link
Member

ojeda commented Mar 20, 2024 •
edited
Loading

@Kartik1397 Do you plan to send this to the mailing list, or would you prefer that @vincenzopalazzo sends it on your behalf? (you would still be mentioned as author or co-author)

Thanks!

@Kartik1397
Copy link
Author

@ojeda I wouldn't be able to work on this for at least two more weeks as I'm currently engaged in another project.
@vincenzopalazzo Feel free to take this patch and push it to the finish line. This would be great!

Thanks!

Darksonn added a commit to Darksonn/linux that referenced this pull request Aug 21, 2024
@Darksonn
# No title
06993f6 
Sending a v2 that is merged with [1]. I didn't realize that I was
duplicating work, sorry! I'll update the author information to list
Kartik as the primary author. I kept my function body so that it returns
NonNull to mirror the type used by `ARef::from_raw`.

This change was previously included in
https://lore.kernel.org/all/20240801-vma-v3-1-db6c1c0afda9@google.com/
but is being split out in its own commit at Danilo's suggestion.

To: Miguel Ojeda <ojeda@kernel.org>
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Wedson Almeida Filho <wedsonaf@gmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Gary Guo <gary@garyguo.net>
Cc: Björn Roy Baron <bjorn3_gh@protonmail.com>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Andreas Hindborg <a.hindborg@samsung.com>
Cc: Andreas Hindborg <a.hindborg@samsung.com>
Cc: rust-for-linux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: Rust-for-Linux#1056 [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>

---
Changes in v2:
- Add example from [1], and fix the imports in the example so it compiles.
- Update author information to reflect merge with [1].
- Link to v1: https://lore.kernel.org/r/20240801-aref-into-raw-v1-1-33401e2fbac8@google.com

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 2,
    "change-id": "20240801-aref-into-raw-e0518131442f",
    "prefixes": [],
    "history": {
      "v1": [
        "20240801-aref-into-raw-v1-1-33401e2fbac8@google.com"
      ]
    }
  }
}
Darksonn added a commit to Darksonn/linux that referenced this pull request Aug 21, 2024
@Darksonn
# No title
a3e8db1 
Sending a v2 that is merged with [1]. I didn't realize that I was
duplicating work, sorry! I'll update the author information to list
Kartik as the primary author. I kept my function body so that it returns
NonNull to mirror the type used by `ARef::from_raw`.

This change was previously included in
https://lore.kernel.org/all/20240801-vma-v3-1-db6c1c0afda9@google.com/
but is being split out in its own commit at Danilo's suggestion.

To: Miguel Ojeda <ojeda@kernel.org>
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: Wedson Almeida Filho <wedsonaf@gmail.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Gary Guo <gary@garyguo.net>
Cc: Björn Roy Baron <bjorn3_gh@protonmail.com>
Cc: Benno Lossin <benno.lossin@proton.me>
Cc: Andreas Hindborg <a.hindborg@samsung.com>
Cc: Andreas Hindborg <a.hindborg@samsung.com>
Cc: rust-for-linux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: Rust-for-Linux#1056 [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>

---
Changes in v3:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v2: https://lore.kernel.org/r/20240821-aref-into-raw-v2-1-9215bbca5720@google.com

Changes in v2:
- Add example from [1], and fix the imports in the example so it compiles.
- Update author information to reflect merge with [1].
- Link to v1: https://lore.kernel.org/r/20240801-aref-into-raw-v1-1-33401e2fbac8@google.com

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 3,
    "change-id": "20240801-aref-into-raw-e0518131442f",
    "prefixes": [],
    "history": {
      "v1": [
        "20240801-aref-into-raw-v1-1-33401e2fbac8@google.com"
      ],
      "v2": [
        "20240821-aref-into-raw-v2-1-9215bbca5720@google.com"
      ]
    }
  }
}
@ojeda ojeda force-pushed the rust-next branch 3 times, most recently from c9b5ce6 to ce1c54f Compare October 9, 2024 22:37
@ojeda ojeda force-pushed the rust-next branch 3 times, most recently from 9ee7197 to 6ce162a Compare October 15, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ARef::into_raw
4 participants
@Kartik1397 @y86-dev @ojeda @vincenzopalazzo