From: Boqun Feng <boqun@kernel.org>
To: phasta@kernel.org
Cc: "Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Paul E. McKenney" <paulmck@kernel.org>,
"Frederic Weisbecker" <frederic@kernel.org>,
"Neeraj Upadhyay" <neeraj.upadhyay@kernel.org>,
"Joel Fernandes" <joelagnelf@nvidia.com>,
"Josh Triplett" <josh@joshtriplett.org>,
"Uladzislau Rezki" <urezki@gmail.com>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
"Lai Jiangshan" <jiangshanlai@gmail.com>,
Zqiang <qiang.zhang@linux.dev>,
"Joel Fernandes (NVIDIA)" <joel@joelfernandes.org>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
"Tamir Duberstein" <tamird@kernel.org>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
rcu@vger.kernel.org,
"Boris Brezillon" <boris.brezillon@collabora.com>
Subject: Re: [PATCH v1] rust: rcu: Add abstraction for call_rcu()
Date: Tue, 26 May 2026 09:10:07 -0700 [thread overview]
Message-ID: <ahXF39fwE5helcj6@tardis.local> (raw)
In-Reply-To: <857ba45ce62f7f209a3a764aeb2cd27dc067870d.camel@mailbox.org>
On Thu, May 21, 2026 at 09:25:28AM +0200, Philipp Stanner wrote:
> [+cc Boris]
>
> On Wed, 2026-05-20 at 06:59 -0700, Boqun Feng wrote:
> > Hi Philipp,
>
> Hi Boqun; hope you're doing well
>
> >
> > On Wed, May 20, 2026 at 03:17:26PM +0200, Philipp Stanner wrote:
> > > call_rcu() can be expected to be needed by a great variety of users.
> > > This functionality is almost always used for deallocating resources
> > > after all accessors are gone. Hence, it appears reasonable to implement
> > > the abstractions in such a way that the user merely passes data, which
> > > is later (after a grace period) dropped.
> > >
> > > In the rare cases where the user needs special action to take place,
> > > this could be achieved through implementing a custom drop() method.
> > >
> > > Implement a first minimal abstraction for call_rcu().
> > >
> >
> > Thanks for the patch! Do you have have any reference usage of this new
> > API, maybe contains how RCU readers will read the data?
>
> Read the data? This design does not intend to have any readers.
>
Please understand that from the perspective of an RCU maintainer, I
would like to examine how the current design would work with a more
general case, otherwise it's going to be a maintenance nightmare.
> I want it as some sort of trash-bin container that does nothing but
> defer a drop(). Intended user will be this section here:
>
> https://gitlab.freedesktop.org/pstanner/linux-drm-work/-/merge_requests/1/diffs#5ef8add7e1b3375ce9a0b47595b531244bf98dce_0_611
>
The fact that you need a "defer" means that there are readers, right?
You don't need to worry about here because the readers are in the
callback of fences.
>
> >
> > Compared to Alice's RcuBox proposal:
> >
> > https://lore.kernel.org/rust-for-linux/20260116-rcu-box-v1-1-38ebfbcd53f0@google.com/
> >
> > I do have a design question: is support data type like Arc<CallBack<T>>
> > or Pin<VBox<Callback<T>> in the plan of this API? If so, how would that
> > be like? A separate new() and submit() function or a separate data type?
>
> I wasn't aware of Alice's proposal. Let me try whether I can make it
> work for my purposes.
>
> The idea behind my code here would be to have some minimalist RCU
> wrapper that merely defers dropping data. So it's a fire-and-forget
> mechanism that would not support Arc: take over ownership of the data,
> have it be unaccessible, and drop it after a grace period.
>
Maybe then name this data structure `RcuDeferDropBox<T>` or something?
Because if the design goal is not to support a general RCU usage (with
readers), than it probably shouldn't take the rcu::Callback name.
Or maybe keep the `Callback<T>`, but only implement `new()` (with a
return type as `impl PinInit<Callback<T>>` and the rcu_head accessor of
it. Then based on it you can implement the `RcuDeferDropBox<T>`, in this
way, we could both support your usage and move towards a full-featured
RCU implementation. Thoughts?
Plus, I think Alice's patch here [1] would also benefit from having a
basic rcu::Callback (to replace `PollCondVarBoxInner`).
[1]: https://lore.kernel.org/rust-for-linux/20260523-upgrade-poll-v4-1-f5b4c747eac2@google.com/
Regards,
Boqun
> Reason is that call_rcu() is most commonly needed for delaying a free()
> operation.
>
> Alice's idea seems more generic.
>
> But I agree that large allocations, aka VBox, should be supported.
>
>
> P.
>
> > If not, what's the main difference between Callback API and RcuBox?
> >
> > Regards,
> > Boqun
> >
> > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > ---
> > > rust/helpers/rcu.c | 1 +
> > > rust/kernel/sync.rs | 1 +
> > > rust/kernel/sync/rcu.rs | 89 ++++++++++++++++++++++++++++++++++++++++-
> > > 3 files changed, 90 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/rust/helpers/rcu.c b/rust/helpers/rcu.c
> > > index 481274c05857..c9cfc99c93d5 100644
> > > --- a/rust/helpers/rcu.c
> > > +++ b/rust/helpers/rcu.c
> > > @@ -1,5 +1,6 @@
> > > // SPDX-License-Identifier: GPL-2.0
> > >
> > > +#include <linux/types.h> /* for callback_head */
> > > #include <linux/rcupdate.h>
> > >
> > > __rust_helper void rust_helper_rcu_read_lock(void)
> > > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > > index 993dbf2caa0e..1ddca3847b19 100644
> > > --- a/rust/kernel/sync.rs
> > > +++ b/rust/kernel/sync.rs
> > > @@ -31,6 +31,7 @@
> > > pub use locked_by::LockedBy;
> > > pub use refcount::Refcount;
> > > pub use set_once::SetOnce;
> > > +pub use rcu::Callback;
> > >
> > > /// Represents a lockdep class.
> > > ///
> > > diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs
> > > index a32bef6e490b..caf71fa46f5e 100644
> > > --- a/rust/kernel/sync/rcu.rs
> > > +++ b/rust/kernel/sync/rcu.rs
> > > @@ -4,7 +4,15 @@
> > > //!
> > > //! C header: [`include/linux/rcupdate.h`](srctree/include/linux/rcupdate.h)
> > >
> > > -use crate::{bindings, types::NotThreadSafe};
> > > +use crate::{
> > > + bindings,
> > > + prelude::*,
> > > + types::{
> > > + NotThreadSafe,
> > > + Opaque,
> > > + },
> > > + alloc::Flags,
> > > +};
> > >
> > > /// Evidence that the RCU read side lock is held on the current thread/CPU.
> > > ///
> > > @@ -50,3 +58,82 @@ fn drop(&mut self) {
> > > pub fn read_lock() -> Guard {
> > > Guard::new()
> > > }
> > > +
> > > +
> > > +/// An RCU callback object. Carries the user's data to drop() it once a grace period ellapsed.
> > > +///
> > > +/// This object serves to implement C's `call_rcu()` method. Since it is almost
> > > +/// always used to free a resource once a grace period ellapsed, the only thing
> > > +/// this implementation does is drop the user's data. In the rare cases in which
> > > +/// the user needs more action to take place, said actions need to be implemented
> > > +/// on the user's data via the [`Drop`] trait.
> > > +///
> > > +/// # Examples
> > > +///
> > > +/// ```
> > > +/// use kernel::sync::rcu::Callback;
> > > +///
> > > +/// struct Foo {};
> > > +///
> > > +/// impl Drop for Foo {
> > > +/// fn drop(&mut self) {
> > > +/// pr_info!("rcu::Foo Dropping.\n");
> > > +/// }
> > > +/// }
> > > +///
> > > +/// let data = Foo {};
> > > +///
> > > +/// let cb = Callback::new(data, GFP_KERNEL)?;
> > > +/// cb.submit();
> > > +///
> > > +/// Ok::<(), Error>(())
> > > +/// ```
> > > +#[repr(C)]
> > > +#[pin_data]
> > > +pub struct Callback<T: Send + 'static> {
> > > + /// The RCU head. Only used (and initialized) by the C backend.
> > > + #[pin]
> > > + inner: Opaque<bindings::callback_head>,
> > > + /// The user's data. This should implement [`Drop`] if the user wants specific
> > > + /// actions, besides mere deallocation, to happen.
> > > + #[pin]
> > > + data: T,
> > > +}
> > > +
> > > +impl<T: Send + 'static> Callback<T> {
> > > + /// Create a new callback.
> > > + pub fn new(data: impl PinInit<T>, flags: Flags) -> Result<Pin<KBox<Self>>> {
> > > + let cb = try_pin_init!(Self {
> > > + inner: Opaque::uninit(), // Only needed for the C backend, who will initialize it.
> > > + data <- data,
> > > + });
> > > +
> > > + KBox::pin_init(cb, flags)
> > > + }
> > > +
> > > + extern "C" fn callback(rcu_head: *mut bindings::callback_head) {
> > > + let cb_ptr = rcu_head as *mut Self;
> > > +
> > > + // SAFETY: All [`Callback`] objects in this module are always created
> > > + // as `Pin<KBox<Self>>`. `Pin` is a transparent container. The action
> > > + // below merely serves re-creating the KBox so that it can drop properly.
> > > + let _cb = unsafe { KBox::from_raw(cb_ptr) };
> > > +
> > > + // Self::data drops, ensuring the desired cleanup operation.
> > > + }
> > > +
> > > + fn as_raw(&self) -> *mut bindings::callback_head {
> > > + self.inner.get()
> > > + }
> > > +
> > > + /// Arm a [`Callback`]. One grace period after this function was called,
> > > + /// the callback object will be dropped.
> > > + pub fn submit(self: Pin<KBox<Self>>) {
> > > + // SAFETY: The memory is not moved by this code or the C backend.
> > > + let cb = unsafe { Pin::into_inner_unchecked(self) };
> > > + let ptr = KBox::into_raw(cb);
> > > + // SAFETY: `ptr` was just created validly above. `Self::callback` relies
> > > + // on the RCU module / code never being unloaded.
> > > + unsafe { bindings::call_rcu((*ptr).as_raw(), Some(Self::callback)) };
> > > + }
> > > +}
> > > --
> > > 2.49.0
> > >
>
>
next prev parent reply other threads:[~2026-05-26 16:10 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-20 13:17 [PATCH v1] rust: rcu: Add abstraction for call_rcu() Philipp Stanner
2026-05-20 13:59 ` Boqun Feng
2026-05-21 7:25 ` Philipp Stanner
2026-05-26 16:10 ` Boqun Feng [this message]
2026-05-27 5:52 ` Alice Ryhl
2026-05-27 5:59 ` Boqun Feng
2026-05-27 6:31 ` Philipp Stanner
2026-05-28 15:06 ` Boqun Feng
2026-05-27 7:42 ` Alvin Sun
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=ahXF39fwE5helcj6@tardis.local \
--to=boqun@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boris.brezillon@collabora.com \
--cc=dakr@kernel.org \
--cc=frederic@kernel.org \
--cc=gary@garyguo.net \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=joelagnelf@nvidia.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=neeraj.upadhyay@kernel.org \
--cc=ojeda@kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=phasta@kernel.org \
--cc=qiang.zhang@linux.dev \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=urezki@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox