* [PATCH v2 1/4] rust: workqueue: add support for ARef<T>
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
@ 2026-02-04 20:40 ` Daniel Almeida
2026-02-04 20:40 ` [PATCH v2 2/4] rust: drm: dispatch work items to the private data Daniel Almeida
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Daniel Almeida @ 2026-02-04 20:40 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel, Daniel Almeida
Add support for the ARef<T> smart pointer. This allows an instance of
ARef<T> to handle deferred work directly, which can be convenient or even
necessary at times, depending on the specifics of the driver or subsystem.
The implementation is similar to that of Arc<T>, and a subsequent patch
will implement support for drm::Device as the first user. This is notably
important for work items that need access to the drm device, as it was not
possible to enqueue work on a ARef<drm::Device<T>> previously without
failing the orphan rule.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/workqueue.rs | 85 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 79 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 706e833e9702..6ae7f3fb3496 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -192,9 +192,9 @@
sync::Arc,
sync::LockClassKey,
time::Jiffies,
- types::Opaque,
+ types::{ARef, AlwaysRefCounted, Opaque},
};
-use core::marker::PhantomData;
+use core::{marker::PhantomData, ptr::NonNull};
/// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
#[macro_export]
@@ -425,10 +425,11 @@ pub unsafe trait RawDelayedWorkItem<const ID: u64>: RawWorkItem<ID> {}
/// Defines the method that should be called directly when a work item is executed.
///
-/// This trait is implemented by `Pin<KBox<T>>` and [`Arc<T>`], and is mainly intended to be
-/// implemented for smart pointer types. For your own structs, you would implement [`WorkItem`]
-/// instead. The [`run`] method on this trait will usually just perform the appropriate
-/// `container_of` translation and then call into the [`run`][WorkItem::run] method from the
+/// This trait is implemented by `Pin<KBox<T>>`, [`Arc<T>`] and [`ARef<T>`], and
+/// is mainly intended to be implemented for smart pointer types. For your own
+/// structs, you would implement [`WorkItem`] instead. The [`run`] method on
+/// this trait will usually just perform the appropriate `container_of`
+/// translation and then call into the [`run`][WorkItem::run] method from the
/// [`WorkItem`] trait.
///
/// This trait is used when the `work_struct` field is defined using the [`Work`] helper.
@@ -934,6 +935,78 @@ unsafe impl<T, const ID: u64> RawDelayedWorkItem<ID> for Pin<KBox<T>>
{
}
+// SAFETY: Like the `Arc<T>` implementation, the `__enqueue` implementation for
+// `ARef<T>` obtains a `work_struct` from the `Work` field using
+// `T::raw_get_work`, so the same safety reasoning applies:
+//
+// - `__enqueue` gets the `work_struct` from the `Work` field, using `T::raw_get_work`.
+// - The only safe way to create a `Work` object is through `Work::new`.
+// - `Work::new` makes sure that `T::Pointer::run` is passed to `init_work_with_key`.
+// - Finally `Work` and `RawWorkItem` guarantee that the correct `Work` field
+// will be used because of the ID const generic bound. This makes sure that `T::raw_get_work`
+// uses the correct offset for the `Work` field, and `Work::new` picks the correct
+// implementation of `WorkItemPointer` for `ARef<T>`.
+unsafe impl<T, const ID: u64> WorkItemPointer<ID> for ARef<T>
+where
+ T: AlwaysRefCounted,
+ T: WorkItem<ID, Pointer = Self>,
+ T: HasWork<T, ID>,
+{
+ unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
+ // The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
+ let ptr = ptr.cast::<Work<T, ID>>();
+
+ // SAFETY: This computes the pointer that `__enqueue` got from
+ // `ARef::into_raw`.
+ let ptr = unsafe { T::work_container_of(ptr) };
+
+ // SAFETY: The safety contract of `work_container_of` ensures that it
+ // returns a valid non-null pointer.
+ let ptr = unsafe { NonNull::new_unchecked(ptr) };
+
+ // SAFETY: This pointer comes from `ARef::into_raw` and we've been given
+ // back ownership.
+ let aref = unsafe { ARef::from_raw(ptr) };
+
+ T::run(aref)
+ }
+}
+
+// SAFETY: The `work_struct` raw pointer is guaranteed to be valid for the duration of the call to
+// the closure because we get it from an `ARef`, which means that the ref count will be at least 1,
+// and we don't drop the `ARef` ourselves. If `queue_work_on` returns true, it is further guaranteed
+// to be valid until a call to the function pointer in `work_struct` because we leak the memory it
+// points to, and only reclaim it if the closure returns false, or in `WorkItemPointer::run`, which
+// is what the function pointer in the `work_struct` must be pointing to, according to the safety
+// requirements of `WorkItemPointer`.
+unsafe impl<T, const ID: u64> RawWorkItem<ID> for ARef<T>
+where
+ T: AlwaysRefCounted,
+ T: WorkItem<ID, Pointer = Self>,
+ T: HasWork<T, ID>,
+{
+ type EnqueueOutput = Result<(), Self>;
+
+ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
+ where
+ F: FnOnce(*mut bindings::work_struct) -> bool,
+ {
+ let ptr = ARef::into_raw(self);
+
+ // SAFETY: Pointers from ARef::into_raw are valid and non-null.
+ let work_ptr = unsafe { T::raw_get_work(ptr.as_ptr()) };
+ // SAFETY: `raw_get_work` returns a pointer to a valid value.
+ let work_ptr = unsafe { Work::raw_get(work_ptr) };
+
+ if queue_work_on(work_ptr) {
+ Ok(())
+ } else {
+ // SAFETY: The work queue has not taken ownership of the pointer.
+ Err(unsafe { ARef::from_raw(ptr) })
+ }
+ }
+}
+
/// Returns the system work queue (`system_wq`).
///
/// It is the one used by `schedule[_delayed]_work[_on]()`. Multi-CPU multi-threaded. There are
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 2/4] rust: drm: dispatch work items to the private data
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
2026-02-04 20:40 ` [PATCH v2 1/4] rust: workqueue: add support for ARef<T> Daniel Almeida
@ 2026-02-04 20:40 ` Daniel Almeida
2026-02-28 18:40 ` Danilo Krummrich
2026-02-04 20:40 ` [PATCH v2 3/4] rust: workqueue: add delayed work support for ARef<T> Daniel Almeida
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Daniel Almeida @ 2026-02-04 20:40 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel, Daniel Almeida
This implementation dispatches any work enqueued on ARef<drm::Device<T>> to
its driver-provided handler. It does so by building upon the newly-added
ARef<T> support in workqueue.rs in order to call into the driver
implementations for work_container_of and raw_get_work.
This is notably important for work items that need access to the drm
device, as it was not possible to enqueue work on a ARef<drm::Device<T>>
previously without failing the orphan rule.
The current implementation needs T::Data to live inline with drm::Device in
order for work_container_of to function. This restriction is already
captured by the trait bounds. Drivers that need to share their ownership of
T::Data may trivially get around this:
// Lives inline in drm::Device
struct DataWrapper {
work: ...,
// Heap-allocated, shared ownership.
data: Arc<DriverData>,
}
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/drm/device.rs | 54 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 3ce8f62a0056..c760a743e1df 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -6,13 +6,13 @@
use crate::{
alloc::allocator::Kmalloc,
- bindings, device, drm,
- drm::driver::AllocImpl,
- error::from_err_ptr,
- error::Result,
+ bindings, device,
+ drm::{self, driver::AllocImpl},
+ error::{from_err_ptr, Result},
prelude::*,
sync::aref::{ARef, AlwaysRefCounted},
types::Opaque,
+ workqueue::{HasWork, Work, WorkItem},
};
use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull};
@@ -227,3 +227,49 @@ unsafe impl<T: drm::Driver> Send for Device<T> {}
// SAFETY: A `drm::Device` can be shared among threads because all immutable methods are protected
// by the synchronization in `struct drm_device`.
unsafe impl<T: drm::Driver> Sync for Device<T> {}
+
+impl<T, const ID: u64> WorkItem<ID> for Device<T>
+where
+ T: drm::Driver,
+ T::Data: WorkItem<ID, Pointer = ARef<Device<T>>>,
+ T::Data: HasWork<Device<T>, ID>,
+{
+ type Pointer = ARef<Device<T>>;
+
+ fn run(ptr: ARef<Device<T>>) {
+ T::Data::run(ptr);
+ }
+}
+
+// SAFETY:
+//
+// - `raw_get_work` and `work_container_of` return valid pointers by relying on
+// `T::Data::raw_get_work` and `container_of`. In particular, `T::Data` is
+// stored inline in `drm::Device`, so the `container_of` call is valid.
+//
+// - The two methods are true inverses of each other: given `ptr: *mut
+// Device<T>`, `raw_get_work` will return a `*mut Work<Device<T>, ID>` through
+// `T::Data::raw_get_work` and given a `ptr: *mut Work<Device<T>, ID>`,
+// `work_container_of` will return a `*mut Device<T>` through `container_of`.
+unsafe impl<T, const ID: u64> HasWork<Device<T>, ID> for Device<T>
+where
+ T: drm::Driver,
+ T::Data: HasWork<Device<T>, ID>,
+{
+ unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work<Device<T>, ID> {
+ // SAFETY: The caller promises that `ptr` points to a valid `Device<T>`.
+ let data_ptr = unsafe { &raw mut (*ptr).data };
+
+ // SAFETY: `data_ptr` is a valid pointer to `T::Data`.
+ unsafe { T::Data::raw_get_work(data_ptr) }
+ }
+
+ unsafe fn work_container_of(ptr: *mut Work<Device<T>, ID>) -> *mut Self {
+ // SAFETY: The caller promises that `ptr` points at a `Work` field in
+ // `T::Data`.
+ let data_ptr = unsafe { T::Data::work_container_of(ptr) };
+
+ // SAFETY: `T::Data` is stored as the `data` field in `Device<T>`.
+ unsafe { crate::container_of!(data_ptr, Self, data) }
+ }
+}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] rust: drm: dispatch work items to the private data
2026-02-04 20:40 ` [PATCH v2 2/4] rust: drm: dispatch work items to the private data Daniel Almeida
@ 2026-02-28 18:40 ` Danilo Krummrich
2026-03-01 12:06 ` Alice Ryhl
0 siblings, 1 reply; 15+ messages in thread
From: Danilo Krummrich @ 2026-02-28 18:40 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Tejun Heo, Lai Jiangshan,
rust-for-linux, linux-kernel, dri-devel
On Wed Feb 4, 2026 at 9:40 PM CET, Daniel Almeida wrote:
> This implementation dispatches any work enqueued on ARef<drm::Device<T>> to
> its driver-provided handler. It does so by building upon the newly-added
> ARef<T> support in workqueue.rs in order to call into the driver
> implementations for work_container_of and raw_get_work.
>
> This is notably important for work items that need access to the drm
> device, as it was not possible to enqueue work on a ARef<drm::Device<T>>
> previously without failing the orphan rule.
>
> The current implementation needs T::Data to live inline with drm::Device in
> order for work_container_of to function. This restriction is already
> captured by the trait bounds. Drivers that need to share their ownership of
> T::Data may trivially get around this:
>
> // Lives inline in drm::Device
> struct DataWrapper {
> work: ...,
> // Heap-allocated, shared ownership.
> data: Arc<DriverData>,
> }
IIUC, this is how it's supposed to be used:
#[pin_data]
struct MyData {
#[pin]
work: Work<drm::Device<MyDriver>>,
value: u32,
}
impl_has_work! {
impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
}
impl WorkItem for MyData {
type Pointer = ARef<drm::Device<MyDriver>>;
fn run(dev: ARef<drm::Device<MyDriver>>) {
dev_info!(dev, "value = {}\n", dev.value);
}
}
The reason the WorkItem is implemented for MyData, rather than
drm::Device<MyDriver> (which would be a bit more straight forward) is the orphan
rule, I assume.
Now, the whole purpose of this is that a driver can implement WorkItem for
MyData without needing an additional struct (and allocation), such as:
#[pin_data]
struct MyWork {
#[pin]
work: Work<Self>,
dev: drm::Device<MyDriver>,
}
How is this supposed to be done when you want multiple different implementations
of WorkItem that have a drm::Device<MyDriver> as payload?
Fall back to various struct MyWork? Add in an "artificial" type state for MyData
with some phantom data, so you can implement HasWork for MyData<Work0>,
MyData<Work1>, etc.?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] rust: drm: dispatch work items to the private data
2026-02-28 18:40 ` Danilo Krummrich
@ 2026-03-01 12:06 ` Alice Ryhl
2026-03-01 15:18 ` Danilo Krummrich
0 siblings, 1 reply; 15+ messages in thread
From: Alice Ryhl @ 2026-03-01 12:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan, rust-for-linux, linux-kernel, dri-devel
On Sat, Feb 28, 2026 at 07:40:26PM +0100, Danilo Krummrich wrote:
> On Wed Feb 4, 2026 at 9:40 PM CET, Daniel Almeida wrote:
> > This implementation dispatches any work enqueued on ARef<drm::Device<T>> to
> > its driver-provided handler. It does so by building upon the newly-added
> > ARef<T> support in workqueue.rs in order to call into the driver
> > implementations for work_container_of and raw_get_work.
> >
> > This is notably important for work items that need access to the drm
> > device, as it was not possible to enqueue work on a ARef<drm::Device<T>>
> > previously without failing the orphan rule.
> >
> > The current implementation needs T::Data to live inline with drm::Device in
> > order for work_container_of to function. This restriction is already
> > captured by the trait bounds. Drivers that need to share their ownership of
> > T::Data may trivially get around this:
> >
> > // Lives inline in drm::Device
> > struct DataWrapper {
> > work: ...,
> > // Heap-allocated, shared ownership.
> > data: Arc<DriverData>,
> > }
>
> IIUC, this is how it's supposed to be used:
>
> #[pin_data]
> struct MyData {
> #[pin]
> work: Work<drm::Device<MyDriver>>,
> value: u32,
> }
>
> impl_has_work! {
> impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
> }
>
> impl WorkItem for MyData {
> type Pointer = ARef<drm::Device<MyDriver>>;
>
> fn run(dev: ARef<drm::Device<MyDriver>>) {
> dev_info!(dev, "value = {}\n", dev.value);
> }
> }
>
> The reason the WorkItem is implemented for MyData, rather than
> drm::Device<MyDriver> (which would be a bit more straight forward) is the orphan
> rule, I assume.
This characterizes it as a workaround for the orphan rule. I don't think
that's fair. Implementing WorkItem for MyDriver directly is the
idiomatic way to do it, in my opinion.
> Now, the whole purpose of this is that a driver can implement WorkItem for
> MyData without needing an additional struct (and allocation), such as:
>
> #[pin_data]
> struct MyWork {
> #[pin]
> work: Work<Self>,
> dev: drm::Device<MyDriver>,
> }
>
> How is this supposed to be done when you want multiple different implementations
> of WorkItem that have a drm::Device<MyDriver> as payload?
>
> Fall back to various struct MyWork? Add in an "artificial" type state for MyData
> with some phantom data, so you can implement HasWork for MyData<Work0>,
> MyData<Work1>, etc.?
You cannot configure the code that is executed on a per-call basis
because the code called by a work item is a function pointer stored
inside the `struct work_struct`. And it can't be changed after
initialization of the field.
So either you must store that info in a separate field. This is what
Binder does, see drivers/android/binder/process.rs for an example.
impl workqueue::WorkItem for Process {
type Pointer = Arc<Process>;
fn run(me: Arc<Self>) {
let defer;
{
let mut inner = me.inner.lock();
defer = inner.defer_work;
inner.defer_work = 0;
}
if defer & PROC_DEFER_FLUSH != 0 {
me.deferred_flush();
}
if defer & PROC_DEFER_RELEASE != 0 {
me.deferred_release();
}
}
}
Or you must have multiple different fields of type Work, each with a
different function pointer stored inside it.
Note that this is a general workqueue API thing. It's not specific to
ARef<_> or drm::Device<_> based work items, but applies to all users of
the workqueue API.
Alice
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] rust: drm: dispatch work items to the private data
2026-03-01 12:06 ` Alice Ryhl
@ 2026-03-01 15:18 ` Danilo Krummrich
2026-03-01 16:53 ` Alice Ryhl
0 siblings, 1 reply; 15+ messages in thread
From: Danilo Krummrich @ 2026-03-01 15:18 UTC (permalink / raw)
To: Alice Ryhl
Cc: Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan, rust-for-linux, linux-kernel, dri-devel
On Sun Mar 1, 2026 at 1:06 PM CET, Alice Ryhl wrote:
> On Sat, Feb 28, 2026 at 07:40:26PM +0100, Danilo Krummrich wrote:
>> On Wed Feb 4, 2026 at 9:40 PM CET, Daniel Almeida wrote:
>> > This implementation dispatches any work enqueued on ARef<drm::Device<T>> to
>> > its driver-provided handler. It does so by building upon the newly-added
>> > ARef<T> support in workqueue.rs in order to call into the driver
>> > implementations for work_container_of and raw_get_work.
>> >
>> > This is notably important for work items that need access to the drm
>> > device, as it was not possible to enqueue work on a ARef<drm::Device<T>>
>> > previously without failing the orphan rule.
>> >
>> > The current implementation needs T::Data to live inline with drm::Device in
>> > order for work_container_of to function. This restriction is already
>> > captured by the trait bounds. Drivers that need to share their ownership of
>> > T::Data may trivially get around this:
>> >
>> > // Lives inline in drm::Device
>> > struct DataWrapper {
>> > work: ...,
>> > // Heap-allocated, shared ownership.
>> > data: Arc<DriverData>,
>> > }
>>
>> IIUC, this is how it's supposed to be used:
>>
>> #[pin_data]
>> struct MyData {
>> #[pin]
>> work: Work<drm::Device<MyDriver>>,
>> value: u32,
>> }
>>
>> impl_has_work! {
>> impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
>> }
>>
>> impl WorkItem for MyData {
>> type Pointer = ARef<drm::Device<MyDriver>>;
>>
>> fn run(dev: ARef<drm::Device<MyDriver>>) {
>> dev_info!(dev, "value = {}\n", dev.value);
>> }
>> }
>>
>> The reason the WorkItem is implemented for MyData, rather than
>> drm::Device<MyDriver> (which would be a bit more straight forward) is the orphan
>> rule, I assume.
>
> This characterizes it as a workaround for the orphan rule. I don't think
> that's fair. Implementing WorkItem for MyDriver directly is the
> idiomatic way to do it, in my opinion.
The trait bound is T::Data: WorkItem, not T: drm::Driver + WorkItem.
Implementing WorkItem for MyDriver seems more straight forward to me.
>> Now, the whole purpose of this is that a driver can implement WorkItem for
>> MyData without needing an additional struct (and allocation), such as:
>>
>> #[pin_data]
>> struct MyWork {
>> #[pin]
>> work: Work<Self>,
>> dev: drm::Device<MyDriver>,
>> }
>>
>> How is this supposed to be done when you want multiple different implementations
>> of WorkItem that have a drm::Device<MyDriver> as payload?
>>
>> Fall back to various struct MyWork? Add in an "artificial" type state for MyData
>> with some phantom data, so you can implement HasWork for MyData<Work0>,
>> MyData<Work1>, etc.?
>
> You cannot configure the code that is executed on a per-call basis
> because the code called by a work item is a function pointer stored
> inside the `struct work_struct`. And it can't be changed after
> initialization of the field.
>
> So either you must store that info in a separate field. This is what
> Binder does, see drivers/android/binder/process.rs for an example.
>
> impl workqueue::WorkItem for Process {
> type Pointer = Arc<Process>;
>
> fn run(me: Arc<Self>) {
> let defer;
> {
> let mut inner = me.inner.lock();
> defer = inner.defer_work;
> inner.defer_work = 0;
> }
>
> if defer & PROC_DEFER_FLUSH != 0 {
> me.deferred_flush();
> }
> if defer & PROC_DEFER_RELEASE != 0 {
> me.deferred_release();
> }
> }
> }
Ok, so this would be a switch to decide what to do when a single work is run,
i.e. it is not for running multiple work.
> Or you must have multiple different fields of type Work, each with a
> different function pointer stored inside it.
This sounds it works for running multiple work, but I wonder how enqueue() knows
which work should be run in this case? I.e. what do we do with:
impl_has_work! {
impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/4] rust: drm: dispatch work items to the private data
2026-03-01 15:18 ` Danilo Krummrich
@ 2026-03-01 16:53 ` Alice Ryhl
0 siblings, 0 replies; 15+ messages in thread
From: Alice Ryhl @ 2026-03-01 16:53 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan, rust-for-linux, linux-kernel, dri-devel
On Sun, Mar 01, 2026 at 04:18:15PM +0100, Danilo Krummrich wrote:
> On Sun Mar 1, 2026 at 1:06 PM CET, Alice Ryhl wrote:
> > On Sat, Feb 28, 2026 at 07:40:26PM +0100, Danilo Krummrich wrote:
> >> On Wed Feb 4, 2026 at 9:40 PM CET, Daniel Almeida wrote:
> >> > This implementation dispatches any work enqueued on ARef<drm::Device<T>> to
> >> > its driver-provided handler. It does so by building upon the newly-added
> >> > ARef<T> support in workqueue.rs in order to call into the driver
> >> > implementations for work_container_of and raw_get_work.
> >> >
> >> > This is notably important for work items that need access to the drm
> >> > device, as it was not possible to enqueue work on a ARef<drm::Device<T>>
> >> > previously without failing the orphan rule.
> >> >
> >> > The current implementation needs T::Data to live inline with drm::Device in
> >> > order for work_container_of to function. This restriction is already
> >> > captured by the trait bounds. Drivers that need to share their ownership of
> >> > T::Data may trivially get around this:
> >> >
> >> > // Lives inline in drm::Device
> >> > struct DataWrapper {
> >> > work: ...,
> >> > // Heap-allocated, shared ownership.
> >> > data: Arc<DriverData>,
> >> > }
> >>
> >> IIUC, this is how it's supposed to be used:
> >>
> >> #[pin_data]
> >> struct MyData {
> >> #[pin]
> >> work: Work<drm::Device<MyDriver>>,
> >> value: u32,
> >> }
> >>
> >> impl_has_work! {
> >> impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
> >> }
> >>
> >> impl WorkItem for MyData {
> >> type Pointer = ARef<drm::Device<MyDriver>>;
> >>
> >> fn run(dev: ARef<drm::Device<MyDriver>>) {
> >> dev_info!(dev, "value = {}\n", dev.value);
> >> }
> >> }
> >>
> >> The reason the WorkItem is implemented for MyData, rather than
> >> drm::Device<MyDriver> (which would be a bit more straight forward) is the orphan
> >> rule, I assume.
> >
> > This characterizes it as a workaround for the orphan rule. I don't think
> > that's fair. Implementing WorkItem for MyDriver directly is the
> > idiomatic way to do it, in my opinion.
>
> The trait bound is T::Data: WorkItem, not T: drm::Driver + WorkItem.
> Implementing WorkItem for MyDriver seems more straight forward to me.
I missed the part about `for MyData` vs `for MyDriver`. Since you
talked about the orphan rule I assumed you wanted the driver to
implement it for `drm::Device<MyDriver>` directly, which is what the
orphan rule would prohibit, rather than for `MyDriver`.
In any case, I do think it makes sense that you would implement it on
the struct that actually contains the `struct work_struct`.
> >> Now, the whole purpose of this is that a driver can implement WorkItem for
> >> MyData without needing an additional struct (and allocation), such as:
> >>
> >> #[pin_data]
> >> struct MyWork {
> >> #[pin]
> >> work: Work<Self>,
> >> dev: drm::Device<MyDriver>,
> >> }
> >>
> >> How is this supposed to be done when you want multiple different implementations
> >> of WorkItem that have a drm::Device<MyDriver> as payload?
> >>
> >> Fall back to various struct MyWork? Add in an "artificial" type state for MyData
> >> with some phantom data, so you can implement HasWork for MyData<Work0>,
> >> MyData<Work1>, etc.?
> >
> > You cannot configure the code that is executed on a per-call basis
> > because the code called by a work item is a function pointer stored
> > inside the `struct work_struct`. And it can't be changed after
> > initialization of the field.
> >
> > So either you must store that info in a separate field. This is what
> > Binder does, see drivers/android/binder/process.rs for an example.
> >
> > impl workqueue::WorkItem for Process {
> > type Pointer = Arc<Process>;
> >
> > fn run(me: Arc<Self>) {
> > let defer;
> > {
> > let mut inner = me.inner.lock();
> > defer = inner.defer_work;
> > inner.defer_work = 0;
> > }
> >
> > if defer & PROC_DEFER_FLUSH != 0 {
> > me.deferred_flush();
> > }
> > if defer & PROC_DEFER_RELEASE != 0 {
> > me.deferred_release();
> > }
> > }
> > }
>
> Ok, so this would be a switch to decide what to do when a single work is run,
> i.e. it is not for running multiple work.
Yeah.
But in any case, a single `struct work_struct` can't be used to schedule
multiple work items. It only has one prev/next pointer pair.
> > Or you must have multiple different fields of type Work, each with a
> > different function pointer stored inside it.
>
> This sounds it works for running multiple work, but I wonder how enqueue() knows
> which work should be run in this case? I.e. what do we do with:
>
> impl_has_work! {
> impl HasWork<drm::Device<MyDriver>> for MyData { self.work }
> }
Both WorkItem and HasWork are generic over an ID integer. You can
specify it to disambiguiate.
Alice
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] rust: workqueue: add delayed work support for ARef<T>
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
2026-02-04 20:40 ` [PATCH v2 1/4] rust: workqueue: add support for ARef<T> Daniel Almeida
2026-02-04 20:40 ` [PATCH v2 2/4] rust: drm: dispatch work items to the private data Daniel Almeida
@ 2026-02-04 20:40 ` Daniel Almeida
2026-02-04 20:40 ` [PATCH v2 4/4] rust: drm: dispatch delayed work items to the private data Daniel Almeida
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Daniel Almeida @ 2026-02-04 20:40 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel, Daniel Almeida
The preceding patches added support for ARef<T> work items. By the same
token, add support for delayed work items too.
The rationale is the same: it may be convenient or even necessary at times
to implement HasDelayedWork directly on ARef<T>. A follow up patch will
also implement support for drm::Device as the first user.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/workqueue.rs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 6ae7f3fb3496..4ee4ff567197 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -1007,6 +1007,17 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
}
}
+// SAFETY: By the safety requirements of `HasDelayedWork`, the `work_struct` returned by methods in
+// `HasWork` provides a `work_struct` that is the `work` field of a `delayed_work`, and the rest of
+// the `delayed_work` has the same access rules as its `work` field.
+unsafe impl<T, const ID: u64> RawDelayedWorkItem<ID> for ARef<T>
+where
+ T: WorkItem<ID, Pointer = Self>,
+ T: HasDelayedWork<T, ID>,
+ T: AlwaysRefCounted,
+{
+}
+
/// Returns the system work queue (`system_wq`).
///
/// It is the one used by `schedule[_delayed]_work[_on]()`. Multi-CPU multi-threaded. There are
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 4/4] rust: drm: dispatch delayed work items to the private data
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
` (2 preceding siblings ...)
2026-02-04 20:40 ` [PATCH v2 3/4] rust: workqueue: add delayed work support for ARef<T> Daniel Almeida
@ 2026-02-04 20:40 ` Daniel Almeida
2026-02-16 16:20 ` [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Daniel Almeida @ 2026-02-04 20:40 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel, Daniel Almeida
Much like the patch that dispatched (regular) work items, we also need to
dispatch delayed work items in order not to trigger the orphan rule. This
allows a drm::Device<T> to dispatch the delayed work to T::Data.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
rust/kernel/drm/device.rs | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index c760a743e1df..ad0447e8763f 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -12,7 +12,7 @@
prelude::*,
sync::aref::{ARef, AlwaysRefCounted},
types::Opaque,
- workqueue::{HasWork, Work, WorkItem},
+ workqueue::{HasDelayedWork, HasWork, Work, WorkItem},
};
use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull};
@@ -273,3 +273,15 @@ unsafe fn work_container_of(ptr: *mut Work<Device<T>, ID>) -> *mut Self {
unsafe { crate::container_of!(data_ptr, Self, data) }
}
}
+
+// SAFETY: Our `HasWork<T, ID>` implementation returns a `work_struct` that is
+// stored in the `work` field of a `delayed_work` with the same access rules as
+// the `work_struct` owing to the bound on `T::Data: HasDelayedWork<Device<T>,
+// ID>`, which requires that `T::Data::raw_get_work` return a `work_struct` that
+// is inside a `delayed_work`.
+unsafe impl<T, const ID: u64> HasDelayedWork<Device<T>, ID> for Device<T>
+where
+ T: drm::Driver,
+ T::Data: HasDelayedWork<Device<T>, ID>,
+{
+}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
` (3 preceding siblings ...)
2026-02-04 20:40 ` [PATCH v2 4/4] rust: drm: dispatch delayed work items to the private data Daniel Almeida
@ 2026-02-16 16:20 ` Daniel Almeida
2026-02-28 17:35 ` Daniel Almeida
2026-03-01 20:07 ` Danilo Krummrich
2026-03-02 16:47 ` Alice Ryhl
6 siblings, 1 reply; 15+ messages in thread
From: Daniel Almeida @ 2026-02-16 16:20 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel
> On 4 Feb 2026, at 17:40, Daniel Almeida <daniel.almeida@collabora.com> wrote:
>
> This series adds ARef<T> support for both regular and delayed work items.
>
> - Patches 1 and 3 actually implement the support in workqueue.rs
> - Patches 2 and 4 adds a corresponding implementation in drm::Device that
> dispatches the calls to the underlying T::Data.
>
> This was tested on Tyr, and is actually needed in order to obtain a
> &drm::Device when handling work items. This is then needed in order to
> allocate GEM objects inside the work handler that processes the tiler OOM
> (out of memory) events. The current series sets the stage so that the above
> is possible in the future.
>
> This is currently based on v6.19-rc8. I hope we can land all four commits
> on a single tree, but otherwise let me know whether I should split the
> workqueue.rs changes from the drm::Device ones and rebase accordingly.
>
> ---
> Changes in v2:
> - Rebased on v6.19-rc8
> - Cc workqueue maintainers
> - Patch 2 kept the old import style, since drm/device.rs is not yet
> converted.
> - Link to v1: https://lore.kernel.org/r/20260115-aref-workitem-v1-0-9883e00f0509@collabora.com
>
> ---
> Daniel Almeida (4):
> rust: workqueue: add support for ARef<T>
> rust: drm: dispatch work items to the private data
> rust: workqueue: add delayed work support for ARef<T>
> rust: drm: dispatch delayed work items to the private data
>
> rust/kernel/drm/device.rs | 66 ++++++++++++++++++++++++++++++--
> rust/kernel/workqueue.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 152 insertions(+), 10 deletions(-)
> ---
> base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
> change-id: 20260115-aref-workitem-0f57e4fb81ca
>
> Best regards,
> --
> Daniel Almeida <daniel.almeida@collabora.com>
>
Friendly ping for the workqueue maintainers here: any comments?
— Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-02-16 16:20 ` [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
@ 2026-02-28 17:35 ` Daniel Almeida
2026-03-02 16:37 ` Tejun Heo
0 siblings, 1 reply; 15+ messages in thread
From: Daniel Almeida @ 2026-02-28 17:35 UTC (permalink / raw)
To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Tejun Heo,
Lai Jiangshan
Cc: rust-for-linux, linux-kernel, dri-devel
Tejun,
AFAICT, there are no further comments on this series. Can I get your feedback here, please?
— Daniel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-02-28 17:35 ` Daniel Almeida
@ 2026-03-02 16:37 ` Tejun Heo
2026-03-02 16:46 ` Alice Ryhl
0 siblings, 1 reply; 15+ messages in thread
From: Tejun Heo @ 2026-03-02 16:37 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter, Lai Jiangshan,
rust-for-linux, linux-kernel, dri-devel
On Sat, Feb 28, 2026 at 02:35:05PM -0300, Daniel Almeida wrote:
> Tejun,
>
> AFAICT, there are no further comments on this series. Can I get your feedback here, please?
I don't know enough to have an opinion but if it looks alright from rust
side, please go ahead.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-03-02 16:37 ` Tejun Heo
@ 2026-03-02 16:46 ` Alice Ryhl
0 siblings, 0 replies; 15+ messages in thread
From: Alice Ryhl @ 2026-03-02 16:46 UTC (permalink / raw)
To: Tejun Heo
Cc: Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, David Airlie, Simona Vetter,
Lai Jiangshan, rust-for-linux, linux-kernel, dri-devel
On Mon, Mar 02, 2026 at 06:37:40AM -1000, Tejun Heo wrote:
> On Sat, Feb 28, 2026 at 02:35:05PM -0300, Daniel Almeida wrote:
> > Tejun,
> >
> > AFAICT, there are no further comments on this series. Can I get your feedback here, please?
>
> I don't know enough to have an opinion but if it looks alright from rust
> side, please go ahead.
Yes I think it looks alright. This just makes workqueue usable in more
situations.
Alice
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
` (4 preceding siblings ...)
2026-02-16 16:20 ` [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
@ 2026-03-01 20:07 ` Danilo Krummrich
2026-03-02 16:47 ` Alice Ryhl
6 siblings, 0 replies; 15+ messages in thread
From: Danilo Krummrich @ 2026-03-01 20:07 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
David Airlie, Simona Vetter, Tejun Heo, Lai Jiangshan,
rust-for-linux, linux-kernel, dri-devel
On Wed Feb 4, 2026 at 9:40 PM CET, Daniel Almeida wrote:
> rust: workqueue: add support for ARef<T>
> rust: drm: dispatch work items to the private data
> rust: workqueue: add delayed work support for ARef<T>
> rust: drm: dispatch delayed work items to the private data
With the imports fixed
Acked-by: Danilo Krummrich <dakr@kernel.org>
for the DRM bits.
However, we could avoid a (trivial) conflict taking it through the DRM tree. I
assume the Rust workqueue stuff is maintained by Alice and Tejun? (I couldn't
find a dedicated entry for rust/kernel/workqueue.rs.)
In any case, if you agree, I suggest to take it through the DRM tree.
Thanks,
Danilo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 0/4] rust: Add ARef support for work items
2026-02-04 20:40 [PATCH v2 0/4] rust: Add ARef support for work items Daniel Almeida
` (5 preceding siblings ...)
2026-03-01 20:07 ` Danilo Krummrich
@ 2026-03-02 16:47 ` Alice Ryhl
6 siblings, 0 replies; 15+ messages in thread
From: Alice Ryhl @ 2026-03-02 16:47 UTC (permalink / raw)
To: Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter, Tejun Heo, Lai Jiangshan,
rust-for-linux, linux-kernel, dri-devel
On Wed, Feb 04, 2026 at 05:40:38PM -0300, Daniel Almeida wrote:
> This series adds ARef<T> support for both regular and delayed work items.
>
> - Patches 1 and 3 actually implement the support in workqueue.rs
> - Patches 2 and 4 adds a corresponding implementation in drm::Device that
> dispatches the calls to the underlying T::Data.
>
> This was tested on Tyr, and is actually needed in order to obtain a
> &drm::Device when handling work items. This is then needed in order to
> allocate GEM objects inside the work handler that processes the tiler OOM
> (out of memory) events. The current series sets the stage so that the above
> is possible in the future.
>
> This is currently based on v6.19-rc8. I hope we can land all four commits
> on a single tree, but otherwise let me know whether I should split the
> workqueue.rs changes from the drm::Device ones and rebase accordingly.
I noticed my Reviewed-by from v1 was dropped, so here it is again:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 15+ messages in thread