2023-05-23 15:13:35

by Alice Ryhl

[permalink] [raw]
Subject: [PATCH v2 0/4] Update thread safety markers

In Rust, the `Send` and `Sync` traits are used to mark in what ways a
specific type is thread safe. In this patch series, I add some missing
thread safety markers and improve the documentation related to them.
This change will let you compile some code that would currently fail to
compile even though it doesn't actually violate any thread safety rules.

You can find a definition of what these marker traits mean at [1].

[1]: https://stackoverflow.com/a/68708557/1704411

Alice Ryhl (4):
rust: sync: reword the `Arc` safety comment for `Send`
rust: sync: reword the `Arc` safety comment for `Sync`
rust: specify when `ARef` is thread safe
rust: task: add `Send` marker to `Task`

rust/kernel/sync/arc.rs | 12 +++++++-----
rust/kernel/task.rs | 10 ++++++++--
rust/kernel/types.rs | 13 +++++++++++++
3 files changed, 28 insertions(+), 7 deletions(-)


base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.40.1.698.g37aff9b760-goog



2023-05-23 15:14:01

by Alice Ryhl

[permalink] [raw]
Subject: [PATCH v2 1/4] rust: sync: reword the `Arc` safety comment for `Send`

The safety comment on `impl Send for Arc` talks about "directly"
accessing the value, when it really means "accessing the value with a
mutable reference". This commit clarifies that.

Suggested-by: Boqun Feng <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
Reviewed-by: Andreas Hindborg <[email protected]>
Reviewed-by: Boqun Feng <[email protected]>
---
rust/kernel/sync/arc.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index e6d206242465..87a4c9ed712b 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -146,8 +146,8 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Ar

// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
-// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
-// example, when the reference count reaches zero and `T` is dropped.
+// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` using a
+// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}

// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
--
2.40.1.698.g37aff9b760-goog


2023-05-23 15:57:33

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] rust: sync: reword the `Arc` safety comment for `Send`

On Tue, 23 May 2023 14:44:15 +0000
Alice Ryhl <[email protected]> wrote:

> The safety comment on `impl Send for Arc` talks about "directly"
> accessing the value, when it really means "accessing the value with a
> mutable reference". This commit clarifies that.
>
> Suggested-by: Boqun Feng <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Boqun Feng <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..87a4c9ed712b 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -146,8 +146,8 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Ar
>
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> -// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> -// example, when the reference count reaches zero and `T` is dropped.
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` using a
> +// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
>
> // SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the


Subject: Re: [PATCH v2 1/4] rust: sync: reword the `Arc` safety comment for `Send`

On 5/23/23 11:44, Alice Ryhl wrote:
> The safety comment on `impl Send for Arc` talks about "directly"
> accessing the value, when it really means "accessing the value with a
> mutable reference". This commit clarifies that.
>
> Suggested-by: Boqun Feng <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Boqun Feng <[email protected]>
> ---
> [...]
>
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> -// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> -// example, when the reference count reaches zero and `T` is dropped.
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` using a
> +// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
>
> // SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

2023-05-25 13:50:54

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] rust: sync: reword the `Arc` safety comment for `Send`

On 5/23/23 16:44, Alice Ryhl wrote:
> The safety comment on `impl Send for Arc` talks about "directly"
> accessing the value, when it really means "accessing the value with a
> mutable reference". This commit clarifies that.
>
> Suggested-by: Boqun Feng <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>
> Reviewed-by: Andreas Hindborg <[email protected]>
> Reviewed-by: Boqun Feng <[email protected]>

Reviewed-by: Benno Lossin <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..87a4c9ed712b 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -146,8 +146,8 @@ impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Ar
>
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> -// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> -// example, when the reference count reaches zero and `T` is dropped.
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` using a
> +// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
>
> // SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> --
> 2.40.1.698.g37aff9b760-goog
>

--
Cheers,
Benno