rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Andreas Hindborg <a.hindborg@kernel.org>
Cc: "Tamir Duberstein" <tamird@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Anna-Maria Behnsen" <anna-maria@linutronix.de>,
	"Frederic Weisbecker" <frederic@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Lyude Paul" <lyude@redhat.com>,
	"Guangbo Cui" <2407018371@qq.com>,
	"Dirk Behme" <dirk.behme@gmail.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 02/14] rust: hrtimer: introduce hrtimer support
Date: Sat, 22 Feb 2025 13:25:46 -0800	[thread overview]
Message-ID: <Z7pA2rnb3a892r1S@boqun-archlinux> (raw)
In-Reply-To: <87h64mcimw.fsf@kernel.org>

(we had some discussion on zulip: 

	https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Timer.20support/near/501291973

, replying to restate what I've said for list discussion)

On Sat, Feb 22, 2025 at 12:40:23PM +0100, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
> 
> > "Boqun Feng" <boqun.feng@gmail.com> writes:
> >
> >> On Fri, Feb 21, 2025 at 09:46:08AM -0500, Tamir Duberstein wrote:
> >>> On Fri, Feb 21, 2025 at 9:40 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >>> >
> >>> > Hmm... if you mean:
> >>> >
> >>> > trait HasHrTimer {
> >>> >     unsafe fn start(&self, expires: Ktime) {
> >>> >         ...
> >>> >     }
> >>> > }
> >>> >
> >>> > Then it'll be problematic because the pointer derived from `&self`
> >>> > doesn't have write provenance, therefore in a timer callback, the
> >>> > pointer cannot be used for write, which means for example you cannot
> >>> > convert the pointer back into a `Pin<Box<HasTimer>>`.
> >>> >
> >>> > To answer Tamir's question, pointers are heavily used here because we
> >>> > need to preserve the provenance.
> >>>
> >>> Wouldn't the natural implication be that &mut self is needed? Maybe
> >>
> >> For an `Arc<HasTimer>`, you cannot get `&mut self`.
> >>
> >>> you can help me understand why pointers can express a contract that
> >>> references can't?
> >>
> >> I assume you already know what a pointer provenance is?
> >>
> >> 	http://doc.rust-lang.org/std/ptr/index.html#provenance
> >>
> >> Passing a pointer (including offset operation on it) preserves the
> >> provenance (determined as derive time), however, deriving a pointer from
> >> a reference gives the pointer a provenance based on the reference type.
> >> For example, let's say we have an `Arc<i32>` and a clone:
> >>
> >> 	let arc = Arc::new(42);
> >> 	let clone = arc.clone();
> >>
> >> you can obviously do a into_raw() + from_raw() pair:
> >>
> >> 	let ptr = Arc::into_raw(arc);
> >> 	let arc = unsafe { Arc::from_raw(arc) };
> >>
> >> however, if you create a reference based on `Arc::into_raw()`, and then
> >> derive a pointer from that, you change the provenance,
> >
> > In this case, the pointer will have the pointer of `Arc::into_raw()`
> > will have the provenance of the original reference. When you turn that
> > pointer back into a reference, won't the reference inherit the
> > provenance of the pointer, which is the same as the original reference?

If the reference is an immutable one, then it won't have the write
provenance, and if the reference is only to the `Timer` field, it won't
have the provenance for the whole `HasHrTimer` struct.

> >
> > As I read the docs, getting a reference to a `Timer` from a reference to
> > a `<MyType as HasHrTimer>` by converting `&MyType` to a `*const MyType`,
> > doing a `ptr.cast::<u8>().add(offset).cast::<HrTimer<T>>()` and
> > converting that pointer to a reference should be fine? The final pointer
> > before converting back to a reference will still have provenance of the
> > original reference. Converting to a reference at the end will shrink the
> > provenance, but it is still fine.
> >
> > Going from a `&HrTimer<T>` to a `&T` is a problem, because that would
> > require offset outside spatial permission of pointer provenance, and it
> > would require increasing the size of the spatial permission.
> >
> > Is this correctly understood?

Right. Also in this case, since eventually, we need to convert to a
`Arc<T>`, the pointer from `&HrTimer` or `&T` doesn't work because write
provenance is required.

> 
> How does provenance work across language boundaries? Should we actually
> use `with_addr` [1] when we get pointers from C round trips?
> 

I would suggest we assume that C can pass the provenance via pointers,
that is, if we pass a pointer value to C and eventually get it back from
C, we should treat it as having the same provenance. The rationale is 1)
considering LTO, C may also have the same or similar provenance model
(e.g [1]), and 2) the C part may get rewritten into Rust in the future,
the correctness about provenance should not be changed by that
possibility. `with_addr()` is not necessary then, because pointers
should have the proper provenance when it passes Rust/C boundary.

[1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3005.pdf

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
> 
> 
> [1] https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.with_addr
> 

  reply	other threads:[~2025-02-22 21:26 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <aIJ0ymzdUceCN05hwJpth4erH5u2SHYzYl52wGeT3uiO9bdk92ZkEmEEq9a9NXsInJYSz9uziwq-1fvdsXoeDA==@protonmail.internalid>
2025-02-18 13:27 ` [PATCH v8 00/14] hrtimer Rust API Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 01/14] rust: time: Add Ktime::from_ns() Andreas Hindborg
2025-02-19 11:58     ` Benno Lossin
2025-02-19 14:53       ` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 02/14] rust: hrtimer: introduce hrtimer support Andreas Hindborg
2025-02-20 17:04     ` Tamir Duberstein
2025-02-20 21:18       ` Andreas Hindborg
2025-02-20 21:37         ` Tamir Duberstein
2025-02-21  8:19           ` Andreas Hindborg
2025-02-21 13:04             ` Tamir Duberstein
2025-02-21 13:17               ` Andreas Hindborg
2025-02-21 14:19                 ` Tamir Duberstein
2025-02-21  8:36           ` Andreas Hindborg
2025-02-21 13:14             ` Tamir Duberstein
2025-02-21 13:28             ` Andreas Hindborg
2025-02-21 14:21               ` Tamir Duberstein
2025-02-22 18:25                 ` Andreas Hindborg
2025-02-22 18:41                   ` Tamir Duberstein
2025-02-21 14:40               ` Boqun Feng
2025-02-21 14:46                 ` Tamir Duberstein
2025-02-21 15:19                   ` Boqun Feng
2025-02-21 19:46                     ` Tamir Duberstein
2025-02-21 22:37                       ` Boqun Feng
2025-02-22  1:08                         ` Tamir Duberstein
2025-02-22  1:38                           ` Boqun Feng
2025-02-22  9:25                     ` Andreas Hindborg
2025-02-22 11:40                       ` Andreas Hindborg
2025-02-22 21:25                         ` Boqun Feng [this message]
2025-02-20 23:46     ` Benno Lossin
2025-02-21  9:03       ` Andreas Hindborg
2025-02-21 10:15         ` Andreas Hindborg
2025-02-21 11:05           ` Benno Lossin
2025-02-21 12:17             ` Andreas Hindborg
2025-02-22  9:37               ` Benno Lossin
2025-02-22 11:27                 ` Andreas Hindborg
2025-02-21  9:05       ` Andreas Hindborg
2025-02-21 11:29       ` Frederic Weisbecker
2025-02-21 11:44         ` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 03/14] rust: sync: add `Arc::as_ptr` Andreas Hindborg
2025-02-20 23:18     ` Benno Lossin
2025-02-18 13:27   ` [PATCH v8 04/14] rust: hrtimer: implement `HrTimerPointer` for `Arc` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 05/14] rust: hrtimer: allow timer restart from timer handler Andreas Hindborg
2025-02-20 23:47     ` Benno Lossin
2025-02-21  9:09       ` Andreas Hindborg
2025-02-21 10:15         ` Benno Lossin
2025-02-18 13:27   ` [PATCH v8 06/14] rust: hrtimer: add `UnsafeHrTimerPointer` Andreas Hindborg
2025-02-20 23:18     ` Benno Lossin
2025-02-18 13:27   ` [PATCH v8 07/14] rust: hrtimer: add `hrtimer::ScopedHrTimerPointer` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 08/14] rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&T>` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 09/14] rust: hrtimer: implement `UnsafeHrTimerPointer` for `Pin<&mut T>` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 10/14] rust: alloc: add `Box::into_pin` Andreas Hindborg
2025-02-20 23:20     ` Benno Lossin
2025-02-21  9:10       ` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 11/14] rust: hrtimer: implement `HrTimerPointer` for `Pin<Box<T>>` Andreas Hindborg
2025-02-18 13:27   ` [PATCH v8 12/14] rust: hrtimer: add `HrTimerMode` Andreas Hindborg
2025-02-20 23:23     ` Benno Lossin
2025-02-21  9:17       ` Andreas Hindborg
2025-02-21 10:19         ` Benno Lossin
2025-02-21 11:39           ` Andreas Hindborg
2025-02-22  9:34             ` Benno Lossin
2025-02-18 13:27   ` [PATCH v8 13/14] rust: hrtimer: add clocksource selection through `ClockSource` Andreas Hindborg
2025-02-20 23:27     ` Benno Lossin
2025-02-21  9:29       ` Andreas Hindborg
2025-02-21 10:22         ` Benno Lossin
2025-02-18 13:27   ` [PATCH v8 14/14] rust: hrtimer: add maintainer entry Andreas Hindborg
2025-02-19 11:02   ` [PATCH v8 00/14] hrtimer Rust API Andreas Hindborg
2025-02-20 21:03     ` Frederic Weisbecker
2025-02-21  8:40       ` Andreas Hindborg
2025-02-21 11:20         ` Frederic Weisbecker
2025-02-22 13:04           ` Miguel Ojeda
2025-02-22 13:10             ` Miguel Ojeda
2025-03-05 17:34             ` Frederic Weisbecker
2025-02-20 22:35   ` Frederic Weisbecker

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=Z7pA2rnb3a892r1S@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=2407018371@qq.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=anna-maria@linutronix.de \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dirk.behme@gmail.com \
    --cc=frederic@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@gmail.com \
    --cc=tglx@linutronix.de \
    --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).