[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024100507-percolate-kinship-fc9a@gregkh>
Date: Sat, 5 Oct 2024 09:47:25 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Gary Guo <gary@...yguo.net>
Cc: Andreas Hindborg <a.hindborg@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Jens Axboe <axboe@...nel.dk>, Will Deacon <will@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Mark Rutland <mark.rutland@....com>, linux-block@...r.kernel.org,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] rust: block: convert `block::mq` to use `Refcount`
On Fri, Oct 04, 2024 at 04:52:24PM +0100, Gary Guo wrote:
> There is an operation needed by `block::mq`, atomically decreasing
> refcount from 2 to 0, which is not available through refcount.h, so
> I exposed `Refcount::as_atomic` which allows accessing the refcount
> directly.
That's scary, and of course feels wrong on many levels, but:
> @@ -91,13 +95,17 @@ pub(crate) unsafe fn start_unchecked(this: &ARef<Self>) {
> /// C `struct request`. If the operation fails, `this` is returned in the
> /// `Err` variant.
> fn try_set_end(this: ARef<Self>) -> Result<*mut bindings::request, ARef<Self>> {
> - // We can race with `TagSet::tag_to_rq`
> - if let Err(_old) = this.wrapper_ref().refcount().compare_exchange(
> - 2,
> - 0,
> - Ordering::Relaxed,
> - Ordering::Relaxed,
> - ) {
> + // To hand back the ownership, we need the current refcount to be 2.
> + // Since we can race with `TagSet::tag_to_rq`, this needs to atomically reduce
> + // refcount to 0. `Refcount` does not provide a way to do this, so use the underlying
> + // atomics directly.
> + if this
> + .wrapper_ref()
> + .refcount()
> + .as_atomic()
> + .compare_exchange(2, 0, Ordering::Relaxed, Ordering::Relaxed)
> + .is_err()
Why not just call rust_helper_refcount_set()? Or is the issue that you
think you might not be 2 here? And if you HAVE to be 2, why that magic
value (i.e. why not just always be 1 and rely on normal
increment/decrement?)
I know some refcounts are odd in the kernel, but I don't see where the
block layer is caring about 2 as a refcount anywhere, what am I missing?
thanks,
greg k-h
Powered by blists - more mailing lists