linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andreas Hindborg <a.hindborg@kernel.org>
To: Oliver Mangold <oliver.mangold@pm.me>, Benno Lossin <lossin@kernel.org>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Asahi Lina" <lina+kernel@asahilina.net>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v11 1/4] rust: types: Add Ownable/Owned types
Date: Tue, 19 Aug 2025 11:00:15 +0200	[thread overview]
Message-ID: <87h5y3d6u8.fsf@t14s.mail-host-address-is-not-set> (raw)
In-Reply-To: <aKQ5rHqxOBEI2n4w@mango>

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> On 250819 1026, Benno Lossin wrote:
>> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>> > On 250819 0027, Benno Lossin wrote:
>> >> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>> >> > On 250818 1446, Andreas Hindborg wrote:
>> >> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> >> >> > +impl<T: OwnableMut> DerefMut for Owned<T> {
>> >> >> > +    fn deref_mut(&mut self) -> &mut Self::Target {
>> >> >> > +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>> >> >> > +        // return a mutable reference to it.
>> >> >> > +        unsafe { self.ptr.as_mut() }
>> >> >> > +    }
>> >> >> > +}
>> >> >>
>> >> >> I think someone mentioned this before, but handing out mutable
>> >> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>> >> >> to hand out `&mut Page` in case of `Owned<Page>`.
>> >> >>
>> >> >
>> >> > That was the reason, why `OwnableMut` was introduced in the first place.
>> >> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>> >>
>> >> Yeah the safety requirements ensure that you can't implement it on
>> >> `!Unpin` types.
>> >>
>> >> But I'm not sure it's useful then? As you said there aren't many types
>> >> that will implement the type then, so how about we change the meaning
>> >> and make it give out a pinned mutable reference instead?
>> >
>> > Making `deref_mut()` give out a pinned type won't work. The return types of
>> > deref() are required to match.
>>
>> I meant the changes that Andreas suggested.
>>
>> >> > Good question, I have been thinking about it, too. But it might
>> >> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>> >> > are around non-movable C structs. Do we actually have a useful application
>> >> > for OwnableMut?
>> >>
>> >> Also, do we even need two different traits? Which types would only
>> >> implement `Ownable` but not `OwnableMut`?
>> >
>> > I'm not 100% sure, but on a quick glance it looks indeed be safe to
>> > substitute `OwnableMut` by `Unpin`.
>>
>> We just have to change the safety requirements of `OwnableMut`.
>
> You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question
> in that context is, what it actually means to have an `&mut Opaque<T>`
> where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`,
> it would we easy, I guess.

You should not be able to get a `&mut T` from a `&mut Opaque<T>`.
`Opaque` opts out of invariants that normally hold for rust references.

>
>> > If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>> > it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>>
>> Well the `DerefMut` impl still is convenient in the `Unpin` case.
>
> I agree. What I meant is, it could not introduce an extra safety
> requirement having it, if that indirect method can be used anyway.

As I mention in my other email, I think we can still have `OwnableMut`
if we add a trait bound on `Unpin`.

>
> But what I am wondering is, if we actually want to start using `Pin`
> at all. Isn't `Opaque` currently used about everywhere pinning is needed?

`Opaque` is `!Unpin`, but pinning guarantees does not come into effect
until we produce a `Pin<Opaque<T>>`.


Best regards,
Andreas Hindborg




  reply	other threads:[~2025-08-19  9:00 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <OYpTDi4YYXiWvLG3nO_8_WKsgOl9KOpun9l3a34m0jza6nmEWDCLTldSwCfZ2PRRprjXqGmrgSL2JN8rPOQH8Q==@protonmail.internalid>
2025-06-18 12:27 ` [PATCH v11 0/4] New trait OwnableRefCounted for ARef<->Owned conversion Oliver Mangold
2025-06-18 12:27   ` [PATCH v11 1/4] rust: types: Add Ownable/Owned types Oliver Mangold
2025-07-02 11:03     ` Benno Lossin
2025-07-07  6:58       ` Oliver Mangold
2025-07-07  9:23         ` Benno Lossin
2025-07-08  9:56           ` Oliver Mangold
2025-07-08 10:16             ` Miguel Ojeda
2025-07-08 13:06               ` Benno Lossin
2025-07-08 18:30                 ` Andreas Hindborg
2025-07-08 19:18                   ` Benno Lossin
2025-07-09  8:53                     ` Andreas Hindborg
2025-07-09  9:11                       ` Benno Lossin
2025-07-08 13:22               ` Andreas Hindborg
2025-07-08 14:53                 ` Benno Lossin
2025-07-08 15:00             ` Benno Lossin
2025-07-07 12:26         ` Miguel Ojeda
2025-08-18 12:46     ` Andreas Hindborg
2025-08-18 13:04       ` Oliver Mangold
2025-08-18 22:27         ` Benno Lossin
2025-08-19  6:04           ` Oliver Mangold
2025-08-19  8:26             ` Benno Lossin
2025-08-19  8:45               ` Oliver Mangold
2025-08-19  9:00                 ` Andreas Hindborg [this message]
2025-08-19 17:15                   ` Benno Lossin
2025-08-20 10:48                     ` Andreas Hindborg
2025-08-19  8:53               ` Andreas Hindborg
2025-08-19 17:13                 ` Benno Lossin
2025-08-19 18:28                   ` Andreas Hindborg
2025-08-20  6:02                   ` Oliver Mangold
2025-08-20  7:41                     ` Benno Lossin
2025-08-20  7:43                       ` Oliver Mangold
2025-08-20 10:51                         ` Andreas Hindborg
2025-06-18 12:27   ` [PATCH v11 2/4] rust: Split `AlwaysRefCounted` into two traits Oliver Mangold
2025-06-19  3:15     ` kernel test robot
2025-07-02 11:23     ` Benno Lossin
2025-07-07  7:42       ` Oliver Mangold
2025-07-07  9:27         ` Benno Lossin
2025-06-18 12:27   ` [PATCH v11 3/4] rust: Add missing SAFETY documentation for `ARef` example Oliver Mangold
2025-06-18 12:27   ` [PATCH v11 4/4] rust: Add `OwnableRefCounted` Oliver Mangold
2025-07-02 13:24     ` Benno Lossin
2025-07-07  8:07       ` Oliver Mangold
2025-07-07  9:33         ` Benno Lossin
2025-07-07 11:12           ` Andreas Hindborg
2025-07-07 11:47             ` Benno Lossin
2025-07-07 13:21               ` Andreas Hindborg
2025-07-07 15:39                 ` Benno Lossin
2025-07-08 13:15                   ` Andreas Hindborg
2025-07-08 14:50                     ` Benno Lossin
2025-07-08 15:35                       ` Andreas Hindborg
2025-07-08  9:36           ` Oliver Mangold
2025-07-08 13:42             ` Benno Lossin
2025-08-05 17:23   ` [PATCH v11 0/4] New trait OwnableRefCounted for ARef<->Owned conversion Danilo Krummrich
2025-08-06  5:56     ` Oliver Mangold
2025-08-15 10:12   ` Andreas Hindborg
2025-08-18  5:59     ` Oliver Mangold

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=87h5y3d6u8.fsf@t14s.mail-host-address-is-not-set \
    --to=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=lina+kernel@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=oliver.mangold@pm.me \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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).