All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Benno Lossin <benno.lossin@proton.me>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Gary Guo" <gary@garyguo.net>, "Alice Ryhl" <alice@ryhl.io>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	rust-for-linux@vger.kernel.org, "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	linux-kernel@vger.kernel.org,
	"Wedson Almeida Filho" <walmeida@microsoft.com>
Subject: Re: [PATCH v2 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef`
Date: Tue, 26 Sep 2023 11:20:35 -0700	[thread overview]
Message-ID: <ZRMg81kOYlBP023z@boqun-archlinux> (raw)
In-Reply-To: <a4090608-d352-742b-fe5e-054db3a8e4a5@proton.me>

On Tue, Sep 26, 2023 at 05:15:52PM +0000, Benno Lossin wrote:
[...]
> > 
> > But here the difference it that we only derive a `*mut` from a `&`,
> > rather than transmute to a `&mut`, right? We only use `&` to get a
> > pointer value (a usize), so I don't think that rule applies here? Or in
> > other words, does the following implemenation look good to you?
> > 
> > 	impl<T: ?Sized> Arc<T> {
> > 	    pub fn as_with_ref(&self) -> &WithRef<T> {
> > 		// expose
> > 		let _ = self.ptr.as_ptr() as usize;
> > 		unsafe { self.ptr.as_ref() }
> > 	    }
> > 	}
> > 
> > 	impl<T: ?Sized> From<&WithRef<T>> for Arc<T> {
> > 	    fn from(b: &WithRef<T>) -> Self {
> > 		// from exposed
> > 		let ptr = unsafe { NonNull::new_unchecked(b as *const _ as usize as *mut _) };
> > 		// SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> > 		// guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> > 		// increment.
> > 		ManuallyDrop::new(unsafe { Arc::from_inner(ptr) })
> > 		    .deref()
> > 		    .clone()
> > 	    }
> > 	}
> > 
> > 
> > An equivalent code snippet is as below (in case anyone wants to try it
> > in miri):
> > ```rust
> >      let raw = Box::into_raw(arc);
> > 
> >      // as_with_ref()
> >      let _ = raw as usize;
> >      let reference = unsafe { &*raw };
> > 
> >      // from()
> >      let raw: *mut T = reference as *const _ as usize as  *mut _ ;
> > 
> >      // drop()
> >      let arc = unsafe { Box::from_raw(raw) };
> > ```
> 
> I don't understand why we are trying to use ptr2int to fix this.
> Simply wrapping the `T` field inside `WithRef` with `UnsafeCell`
> should be enough.
> 

BTW, how do you fix this with only wrapping `T` field in `WithRef`?

Let say `WithRef` is defined as:

struct WithRef<T> {
    refcount: Opaque<bindings::refcount_t>,
    data: UnsafeCell<T>,
}


impl<T: ?Sized> From<&WithRef<T>> for Arc<T> {
    fn from(b: &WithRef<T>) -> Self {
        let data_ptr: *mut T = b.data.get();

	let ptr = ?; // how to get a pointer to `WithRef<T>` with the
                     // provenance to the whole data?

        ManuallyDrop::new(unsafe { Arc::from_inner(ptr) })
            .deref()
            .clone()
    }
}

The `data_ptr` above only has provenance to part of the struct for the
similar reason that my proposal of (ab)using `b.refcount.get()`. Am I
missing something here?

Regards,
Boqun

> -- 
> Cheers,
> Benno
> 
> 

  parent reply	other threads:[~2023-09-26 18:21 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-23 14:49 [PATCH v2 0/2] Remove `ArcBorrow` Wedson Almeida Filho
2023-09-23 14:49 ` [PATCH v2 1/2] rust: arc: rename `ArcInner` to `WithRef` Wedson Almeida Filho
2023-09-23 19:31   ` Martin Rodriguez Reboredo
2023-09-24 11:59   ` Benno Lossin
2023-09-24 13:41   ` Jianguo Bao
2023-09-25  6:21   ` Alice Ryhl
2023-09-23 14:49 ` [PATCH v2 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef` Wedson Almeida Filho
2023-09-23 19:32   ` Martin Rodriguez Reboredo
2023-09-24 11:59   ` Benno Lossin
2023-09-24 13:36   ` Jianguo Bao
2023-09-25  6:29   ` Alice Ryhl
2023-09-25  9:14     ` Benno Lossin
2023-09-25 14:49       ` Boqun Feng
2023-09-25 15:00         ` Alice Ryhl
2023-09-25 15:17           ` Boqun Feng
2023-09-25 15:30             ` Alice Ryhl
2023-09-25 16:02               ` Boqun Feng
2023-09-25 16:11                 ` Benno Lossin
2023-09-25 15:07         ` Benno Lossin
2023-09-25 16:16           ` Boqun Feng
2023-09-25 17:00             ` Benno Lossin
2023-09-25 18:51               ` Boqun Feng
2023-09-25 21:03                 ` Benno Lossin
2023-09-25 21:55                   ` Boqun Feng
2023-09-25 21:58                     ` Alice Ryhl
2023-09-25 22:02                       ` Boqun Feng
2023-09-25 22:06                         ` Boqun Feng
2023-09-25 22:26                         ` Benno Lossin
2023-09-25 22:34                           ` Boqun Feng
2023-09-25 23:24                             ` Boqun Feng
2023-09-26  8:26                           ` Gary Guo
2023-09-26 15:24                             ` Boqun Feng
2023-09-26 15:41                               ` Alice Ryhl
2023-09-26 16:35                                 ` Boqun Feng
2023-09-26 17:15                                   ` Benno Lossin
2023-09-26 17:43                                     ` Boqun Feng
2023-09-26 18:26                                       ` Benno Lossin
2023-09-26 21:31                                       ` Alice Ryhl
2023-09-26 18:20                                     ` Boqun Feng [this message]
2023-09-26 21:27                                       ` Alice Ryhl
2023-09-25 15:04       ` Alice Ryhl

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=ZRMg81kOYlBP023z@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=alice@ryhl.io \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=walmeida@microsoft.com \
    --cc=wedsonaf@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.