[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 2 Feb 2023 10:14:06 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Boqun Feng <boqun.feng@...il.com>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Vincenzo Palazzo <vincenzopalazzodev@...il.com>
Subject: Re: [RFC 2/5] rust: sync: Arc: Introduces ArcInner::count()
On Wed, Feb 01, 2023 at 03:22:41PM -0800, Boqun Feng wrote:
> This allows reading the current count of a refcount in an `ArcInner`.
>
> Signed-off-by: Boqun Feng <boqun.feng@...il.com>
> ---
> rust/helpers.c | 6 ++++++
> rust/kernel/sync/arc.rs | 9 +++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 09a4d93f9d62..afc5f1a39fef 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -46,6 +46,12 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
> }
> EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
>
> +unsigned int rust_helper_refcount_read(refcount_t *r)
> +{
> + return refcount_read(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_read);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index fc680a4a795c..fbfceaa3096e 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -127,6 +127,15 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +impl<T: ?Sized> ArcInner<T> {
> + /// Returns the current reference count of [`ArcInner`].
> + fn count(&self) -> u32 {
> + // SAFETY: `self.refcount.get()` is always a valid pointer, and `refcount_read()` is a
> + // normal atomic read (i.e. no data race) only requiring on the address is valid.
> + unsafe { bindings::refcount_read(self.refcount.get()) }
> + }
> +}
This is completely unsafe vs concurrency. In order to enable correct
tracing of refcount manipulations we have the __refcount_*(.oldp) API.
Admittedly, I have no idea how to sanely wire that up in this thing, but
it seems odd to me to have this 'safe' language display fundamentally
unsafe data like this.
Powered by blists - more mailing lists