Linux Media Controller development
 help / color / mirror / Atom feed
From: "Onur Özkan" <work@onurozkan.dev>
To: Philipp Stanner <phasta@kernel.org>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@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>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"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>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Igor Korotin" <igor.korotin@linux.dev>,
	"Lorenzo Stoakes" <ljs@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	"Krishna Ketan Rai" <prafulrai522@gmail.com>,
	"Shankari Anand" <shankari.ak0208@gmail.com>,
	manos@pitsidianak.is,
	"Boris Brezillon" <boris.brezillon@collabora.com>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
	rcu@vger.kernel.org
Subject: Re: [PATCH v2 5/6] rust: Add dma_fence abstractions
Date: Tue, 16 Jun 2026 15:47:33 +0300	[thread overview]
Message-ID: <20260616124755.460550-1-work@onurozkan.dev> (raw)
In-Reply-To: <20260616082819.2943886-7-phasta@kernel.org>

On Tue, 16 Jun 2026 10:28:17 +0200
Philipp Stanner <phasta@kernel.org> wrote:

> C's dma_fence's are synchronisation primitives that will be needed by all
> Rust GPU drivers.
> 
> The dma_fence framework sets a number of rules, notably:
>   - fences must only be signalled once
>   - all fences must be signalled at some point
>   - fence error codes must only be set before signalling
>   - every pointer to a fence must be backed by a reference
> 
> All those rules are being addressed by these abstractions.
> 
> To cleanly decouple fence issuers and consumers, two types are provided:
>   - DriverFence: the only fence type that can be signalled and that
>     carries driver-specific data.
>   - Fence: the fence type to be shared with other drivers and / or
>     userspace. The only type callbacks can be registered on.
>     Cannot be signalled.
> 
> Hereby, a Fence lives in the same chunk of memory as a DriverFence. Both
> share the refcount of the underlying C dma_fence. Since this
> implementation does not provide a custom dma_fence_backend_ops.release()
> function, the memory is freed by the dma_fence backend once the refcount
> drops to 0.
> 
> To create a DriverFence, the user must first allocate a
> DriverFenceAllocation, so that the creation of the DriverFence later on
> can always succeed. Otherwise, deadlocks could occur if fences need to
> be created in a GPU job submission path.
> 
> Synchronization is ensured by the dma_fence backend.
> 
> All DriverFence's created through this abstraction must be signalled by
> the creator with an error code. In case a DriverFence drops without
> being signalled beforehand, it is signalled with -ECANCELLED as its
> error and a warning is printed. This allows the Rust abstraction to very
> cleanly decouple fence issuer and consumer by relying on the decoupling
> mechanisms in the C backend, which ensures through RCU and the
> 'signalled' fence-flag that dma_fence_backend_ops functions cannot
> access the potentially unloaded driver code anymore.
> 
> Signalling fences on drop thus grants many advantages. Not signalling
> fences on drop would risk deadlock and does not grant real advantages:
> By definition only the drivers can ensure that a fence always represents
> the hardware's state correctly.
> 
> This implementation models a DmaFenceCtx (fence context) object on which
> fences are to be created, thereby ensuring correct sequence numbering
> according to the timeline.
> 
> dma_fence supports a variety of callbacks. The mandatory callbacks
> (get_timeline_name() and get_driver_name()) are implemented in this
> patch. For convenience, they store those name parameters in the fence
> context, saving the driver from implementing these two callbacks.
> 
> Support for other callbacks (like for hardware signalling) is prepared
> for through the fact that both DriverFence and Fence live in the same
> allocation, allowing for usage of container_of from the callback to
> access the driver-specific data.
> 
> Synchronization for backend_ops callbacks is ensured by only running the
> Rust deconstructor delayed with call_rcu(), which prevents UAF-bugs
> should a DriverFence drop while a Fence callback is currently operating
> on the associated driver data.
> 
> Add abstractions for dma_fence in Rust.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  rust/bindings/bindings_helper.h  |   1 +
>  rust/helpers/dma_fence.c         |  48 ++
>  rust/helpers/helpers.c           |   1 +
>  rust/kernel/dma_buf/dma_fence.rs | 884 +++++++++++++++++++++++++++++++
>  rust/kernel/dma_buf/mod.rs       |  14 +
>  rust/kernel/lib.rs               |   1 +
>  6 files changed, 949 insertions(+)
>  create mode 100644 rust/helpers/dma_fence.c
>  create mode 100644 rust/kernel/dma_buf/dma_fence.rs
>  create mode 100644 rust/kernel/dma_buf/mod.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 446dbeaf0866..814d7740e686 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -52,6 +52,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/device/faux.h>
>  #include <linux/dma-direction.h>
> +#include <linux/dma-fence.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dma-resv.h>
>  #include <linux/errname.h>
> diff --git a/rust/helpers/dma_fence.c b/rust/helpers/dma_fence.c
> new file mode 100644
> index 000000000000..0e08411098fa
> --- /dev/null
> +++ b/rust/helpers/dma_fence.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/dma-fence.h>
> +
> +__rust_helper void rust_helper_dma_fence_get(struct dma_fence *f)
> +{
> +	dma_fence_get(f);
> +}
> +
> +__rust_helper void rust_helper_dma_fence_put(struct dma_fence *f)
> +{
> +	dma_fence_put(f);
> +}

[...]

> +
> +    /// Create a [`FenceCtx`] from an associated [`bindings::dma_fence`].
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must be a valid pointer to a dma_fence which resides within a [`Fence`],
> +    /// which in turn resides in a [`DriverFenceData`].
> +    unsafe fn from_raw_fence<'a>(ptr: *mut bindings::dma_fence) -> &'a Self {
> +        let opaque_fence = Opaque::cast_from(ptr);
> +
> +        // SAFETY: Safe due to the function's overall safety requirements.
> +        let fence_ptr = unsafe { container_of!(opaque_fence, Fence, inner) };
> +
> +        // DriverFenceData is repr(C) and a Fence is its first member.
> +        let fence_data_ptr = fence_ptr as *mut DriverFenceData<T>;

Either the field ordering on the type or this code is wrong because the first
member of DriverFenceData is `rcu_head`.

> +
> +        // SAFETY: Safe because of the safety comment directly above.
> +        let fence_data = unsafe { &*fence_data_ptr };
> +
> +        &fence_data.fctx
> +    }
> +}
> +
> +/// Error type for fence callback registration.
> +///
> +/// Generic over `T` so that `AlreadySignaled` can return the callback to the
> +/// caller, allowing it to reclaim any resources owned by the callback (e.g.,
> +/// a fence handle that needs to be signaled).
> +#[derive(Debug)]
> +pub enum CallbackError<T = ()> {
> +    /// The fence was already signaled. The callback is returned so the caller
> +    /// can extract owned resources without losing them.

[...]

> +        //
> +        // Without this, Drop can race with a concurrent signal:
> +        //   CPU0 (signal, lock held): take() -> signaled(fence_ref) (in progress)
> +        //   CPU1 (drop): sees is_some()==false -> skips lock -> frees struct
> +        //   CPU0: accesses fence_ref -> use-after-free
> +        //
> +        // When the callback has already fired, the signal path detached the
> +        // list node via INIT_LIST_HEAD, so dma_fence_remove_callback just sees
> +        // an empty node and returns false — the lock acquisition is the only
> +        // thing that matters.
> +        //
> +        // SAFETY: The fence pointer is valid and the cb was initialized by
> +        // dma_fence_add_callback during construction.
> +        unsafe {
> +            bindings::dma_fence_remove_callback(self.fence.as_raw(), self.cb.get());
> +        }
> +    }
> +}
> +
> +// SAFETY: FenceCbRegistration can be sent between threads
> +unsafe impl<T: FenceCb> Send for FenceCbRegistration<T> {}
> +
> +// SAFETY: &FenceCbRegistration can be shared between threads if &T can.
> +unsafe impl<T: FenceCb> Sync for FenceCbRegistration<T> where T: Sync {}
> +
> +/// The receiving counterpart of a [`DriverFence`], designed to register callbacks
> +/// on, check the signalled state etc. A [`Fence`] cannot be signalled.
> +/// A [`Fence`] is always refcounted.
> +pub struct Fence {
> +    /// The actual dma_fence passed to C.
> +    inner: Opaque<bindings::dma_fence>,
> +}

I am unsure whether it is safe to cast the pointer in Fence::from_raw without
Fence being #[repr(transparent)] as the layout compatibility is not guaranteed
explicitly.

Regards,
Onur

> +
> +// SAFETY: Fences are literally designed to be shared between threads.
> +unsafe impl Send for Fence {}
> +// SAFETY: Fences are literally designed to be shared between threads.
> +unsafe impl Sync for Fence {}
> +
> +impl Fence {
> +    /// Check whether the fence was signalled at the moment of the function call.
> +    ///
> +    /// Note that this can return `true` for a [`Fence`] whose [`DriverFence`]
> +    /// has not yet been dropped. The reason is that the fence ops callbacks can
> +    /// cause the fence to get signaled by the C backend.
> +    pub fn is_signaled(&self) -> bool {
> +        let fence = self.as_raw();
> +        let mut fence_flags: usize = 0;
> +        let flag_ptr = &raw mut fence_flags;
> +
> +        // We shouuld not use `dma_fence_is_signaled_locked()` here, because
> +        // according to the C backend's recommendations, that function is problematic
> +        // and we should avoid calling that function with a lock held.
> +
> +        // SAFETY: `self` is valid by definition. We take the spinlock above.
> +        let ret = unsafe { bindings::dma_fence_is_signaled(fence) };
> +
> +        // To guarantee that an API caller can 100% rely on the signalling being

[...]

> +    unsafe { bindings::dma_fence_put(fence) };
> +
> +    // The actual memory the data associated with a `DriverFence` lives in
> +    // gets freed by the C dma_fence backend once the fence's refcount reaches 0.
> +}
> diff --git a/rust/kernel/dma_buf/mod.rs b/rust/kernel/dma_buf/mod.rs
> new file mode 100644
> index 000000000000..fb353ce042ce
> --- /dev/null
> +++ b/rust/kernel/dma_buf/mod.rs
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +//! DMA-buf subsystem abstractions.
> +
> +pub mod dma_fence;
> +
> +pub use self::dma_fence::{
> +    DriverFence,
> +    Fence,
> +    FenceCb,
> +    FenceCbRegistration,
> +    FenceCtx,
> +    FenceCtxOps, //
> +};
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index b72b2fbe046d..a05ccaa7598c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -63,6 +63,7 @@
>  pub mod device_id;
>  pub mod devres;
>  pub mod dma;
> +pub mod dma_buf;
>  pub mod driver;
>  #[cfg(CONFIG_DRM = "y")]
>  pub mod drm;
> -- 
> 2.54.0
> 

  reply	other threads:[~2026-06-16 12:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16  8:28 [PATCH v2 0/6] rust / dma_buf: Add abstractions for dma_fence Philipp Stanner
2026-06-16  8:28 ` [PATCH v2 1/6] rust: types: implement ForeignOwnable for ARef<T> Philipp Stanner
2026-06-16  8:28 ` [PATCH v2 2/6] rust: sync: Add abstraction for rcu_barrier() Philipp Stanner
2026-06-16  8:28 ` [PATCH v2 3/6] rust: sync: Add abstraction for synchronize_rcu() Philipp Stanner
2026-06-16  8:28 ` [PATCH v2 4/6] rust: error: Add ECANCELED error code Philipp Stanner
2026-06-16  8:28 ` [PATCH v2 5/6] rust: Add dma_fence abstractions Philipp Stanner
2026-06-16 12:47   ` Onur Özkan [this message]
2026-06-16 14:38     ` Philipp Stanner
2026-06-16 14:51       ` Onur Özkan
2026-06-16 15:28         ` Danilo Krummrich
2026-06-16 16:35           ` Onur Özkan
2026-06-16 15:37       ` Gary Guo
2026-06-16  8:28 ` [PATCH v2 6/6] MAINTAINERS: Add entry for Rust dma-buf Philipp Stanner
2026-06-16 11:02 ` [PATCH v2 0/6] rust / dma_buf: Add abstractions for dma_fence Danilo Krummrich
2026-06-16 11:28   ` Philipp Stanner

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=20260616124755.460550-1-work@onurozkan.dev \
    --to=work@onurozkan.dev \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=boris.brezillon@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frederic@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=igor.korotin@linux.dev \
    --cc=jiangshanlai@gmail.com \
    --cc=joelagnelf@nvidia.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=ljs@kernel.org \
    --cc=lossin@kernel.org \
    --cc=manos@pitsidianak.is \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=neeraj.upadhyay@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=phasta@kernel.org \
    --cc=prafulrai522@gmail.com \
    --cc=qiang.zhang@linux.dev \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=shankari.ak0208@gmail.com \
    --cc=sumit.semwal@linaro.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